| Summary: | openjdk8 on windows updater returns 0 when invalid json is delivered | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | jiri vanek <jvanek> |
| Component: | java-1.8.0-openjdk | Assignee: | Alex Kashchenko <akashche> |
| Status: | CLOSED WONTFIX | QA Contact: | zzambers |
| Severity: | low | Docs Contact: | |
| Priority: | low | ||
| Version: | 7.4 | CC: | akashche, dbhole, jvanek |
| Target Milestone: | rc | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Windows | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2021-01-15 07:29:02 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
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. (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 Is this going to be fixed? If no please state your reasons and close. 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. 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. |
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.