Bug 1402341 - openjdk8 on windows updater returns 0 when invalid json is delivered
Summary: openjdk8 on windows updater returns 0 when invalid json is delivered
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: java-1.8.0-openjdk
Version: 7.4
Hardware: Unspecified
OS: Windows
low
low
Target Milestone: rc
: ---
Assignee: Alex Kashchenko
QA Contact: zzambers
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-12-07 09:57 UTC by jiri vanek
Modified: 2021-01-15 07:29 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-01-15 07:29:02 UTC
Target Upstream Version:


Attachments (Terms of Use)

Description jiri vanek 2016-12-07 09:57:40 UTC
When updater.exe:
 - got no file, 404, corurpted file (like "some string") , or network connection error
   - it returns nonzero (and do not write version.json)
 - however when it get unknwoon (but valid) json file (like { unknownKey: someValue })
   - it correctly dont save version.json
   - but returns zero


Imho it should return nonzero to.

Comment 1 Alex Kashchenko 2016-12-07 10:21:47 UTC
The problem here is that version.json parsing is lenient, it tries to parse keys it needs from input json and uses default values for all fields that are not found. Such "well-formed but invalid" json will be parsed into "version descriptor" with all string fields empty and version number field zero.

It is not hard to add strict checks for version.json fields during parsing, but I am not sure that we should do it. It may be good to have at least some leniency with input json - for additional "comment" fields, possible additional localized fields etc.

In any case such checks won't change app behaviour (file is not written) and this issue shouldn't be treated as a release blocker.

Comment 2 jiri vanek 2016-12-07 10:34:34 UTC
(In reply to Alex Kashchenko from comment #1)
> The problem here is that version.json parsing is lenient, it tries to parse
> keys it needs from input json and uses default values for all fields that
> are not found. Such "well-formed but invalid" json will be parsed into
> "version descriptor" with all string fields empty and version number field
> zero.
> 
> It is not hard to add strict checks for version.json fields during parsing,

Few checks for most important keys would be worthy (version, version readable? MAybe some more?
If those are not ofund - exit with nonzero?
> but I am not sure that we should do it. It may be good to have at least some
> leniency with input json - for additional "comment" fields, possible
> additional localized fields etc.
> 

Otherwise I moreover agree.

> In any case such checks won't change app behaviour (file is not written) and
> this issue shouldn't be treated as a release blocker.

It is not.  But still there is plenty of time for few more respins

Comment 4 jiri vanek 2016-12-12 08:23:45 UTC
Is this going to be fixed?

If no please state your reasons and close.

Comment 5 Alex Kashchenko 2016-12-12 09:33:16 UTC
I'd like to add more strict checks, but not in this version. First want to see how the current one will go.

There probably should be different kinds of checks in checker in notifier:

Checker reads version.json from  a "shared between all minor versions" (no NVR) location, so it should have minimal restrictions on required attributes - only those that will be used.

Notifier reads version.json from a location private to this particular installer version (directory with NVR), so it probably should check that only the expected fields are present and that all of them are present.

Comment 8 RHEL Program Management 2021-01-15 07:29:02 UTC
After evaluating this issue, there are no plans to address it further or fix it in an upcoming release.  Therefore, it is being closed.  If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.


Note You need to log in before you can comment on or make changes to this bug.