Chromad: Policy JSON doesn't accept single quotes |
|||
Issue description
Bad JSON still shows up on chrome://policy page. There's no user-visible error messaging, only a log.
For instance NativePrinters:
Ignoring invalid printer. Invalid JSON object: {'display_name': 'printer', 'uuid': '', 'description': 'printer', 'uri': 'ipp://...', 'model': 'Printer', 'ppd_resource': {'effective_model': 'printer'}, 'manufacturer': 'printer'}
,
Jul 3
Correction - NativePrinters is a somewhat unique policy, and hasn't been fixed by any of the changes so far. Will investigate further.
,
Jul 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48898fe7be280b5a8395e152ed097803bb007e80 commit 48898fe7be280b5a8395e152ed097803bb007e80 Author: A Olsen <olsen@chromium.org> Date: Fri Jul 06 09:49:33 2018 Add validation for NativePrinters policy NativePrinters policy is unique in that is a 'list' policy, but it has a schema that is more complicated than "list of string". There are two problems with this schema - one, it is never used for validation, and two, it is actually wrong. The true schema of the value that we store in the pref store needs to be "list of string", otherwise the code that applies this policy will stop working. See this code at go/nativestringprinters This change makes NativePrinters officially a list of strings, as it needs to be, but adds a validation_schema that will warn if the JSON embedded in each string does not parse or does not match the native printer schema. Reason for this change: 1. Show useful warnings in chrome://policy 2. Having the wrong schema is actually dangerous, since certain code paths might automatically convert a policy value to one that matches the schema - in this case, it would be by automatically parsing each string as JSON so that the value becomes a list of dicts. See the code at go/registry_dict_json. If this code path is ever used (for instance, if support for this policy is added to Chrome for windows) then the policy will have a different schema in Windows than in Chrome OS, for non-obvious reasons. Note that this policy predates LEGACY_EMBEDDED_JSON_WHITELIST so can be added to it, since it contains embedded JSON. New policies should not contain embedded JSON and should not be added there. Bug: 834245 Change-Id: Ic3beea00811ac6194c1ed05aa1bbbfb66c9ce546 Reviewed-on: https://chromium-review.googlesource.com/1125819 Commit-Queue: A Olsen <olsen@chromium.org> Reviewed-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Pavol Marko <pmarko@chromium.org> Cr-Commit-Position: refs/heads/master@{#572929} [modify] https://crrev.com/48898fe7be280b5a8395e152ed097803bb007e80/chrome/browser/policy/configuration_policy_handler_list_factory.cc [modify] https://crrev.com/48898fe7be280b5a8395e152ed097803bb007e80/components/policy/resources/policy_templates.json [modify] https://crrev.com/48898fe7be280b5a8395e152ed097803bb007e80/components/policy/tools/syntax_check_policy_template_json.py
,
Jul 6
Fixed - still doesn't accept single quotes (will not fix this), but at least shows proper errors now.
,
Aug 17
Hi Andrew, In the order to verify this bug, could you please confirm that this is the expected (proper) errors on chrome://policy page (see attached screenshot). Thanks, Ivan
,
Aug 27
From what I can see, it looks like a good error (at least more helpful than "OK"). Unfortunately that page has another problem - it is hard to read the entire error, especially from a screenshot. If you like, you can triple-click on the error to highlight it, then copy and paste it into this bug, so I can verify the entire thing.
,
Sep 6
The entire error: Key "items[0]": Error while parsing JSON value: Line: 1, column: 1, Unexpected token. Also if ONC policy is not correctly configured I see the following error: Network configuration failed to be parsed. Could you please confirm that these errors are expected?
,
Nov 12
I verified if the ONC is valid JSON, there's a reasonable error message. However, if the JSON is invalid, you get "Network configuration failed to be parsed." Maybe it should say something like "Network configuration failed to be parsed. Invalid JSON"? In other cases we set the pref to the bad JSON string if the string is invalid IIRC. Would that be possible here as well?
,
Nov 12
I can easily update the error message to include "Invalid JSON". (I'll create a CL right away) I think we don't display the malformed JSON string for network policies on purpose as those could contain credentials. Those are usually masked in chrome://policy, but if the value is invalid JSON we can't guarantee to properly mask those values anymore.
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1feffde9fb554c3b1c76f9146dc5518554d378f commit d1feffde9fb554c3b1c76f9146dc5518554d378f Author: Alexander Hendrich <hendrich@chromium.org> Date: Tue Nov 13 12:20:14 2018 Extend ONC JSON parse error message Add more detailed reason "invalid JSON" why the parsing failed. Bug: 834245 Change-Id: I9a81f425a2d877fecab7a9da1176e043e154d3f6 Reviewed-on: https://chromium-review.googlesource.com/c/1329977 Reviewed-by: Lutz Justen <ljusten@chromium.org> Commit-Queue: Alexander Hendrich <hendrich@chromium.org> Cr-Commit-Position: refs/heads/master@{#607565} [modify] https://crrev.com/d1feffde9fb554c3b1c76f9146dc5518554d378f/components/policy_strings.grdp |
|||
►
Sign in to add a comment |
|||
Comment 1 by olsen@chromium.org
, Jun 19 2018