Commit f8d1ab946205440ed3c44511ef42e5ad13fb9e5e
1 parent
b3e6d445
JSON schema -- accept single item in place of array
When the schema wants a variable-length array, allow a single item as well as allowing an array.
Showing
7 changed files
with
46 additions
and
28 deletions
ChangeLog
| 1 | 2022-07-24 Jay Berkenbilt <ejb@ql.org> | 1 | 2022-07-24 Jay Berkenbilt <ejb@ql.org> |
| 2 | 2 | ||
| 3 | + * include/qpdf/JSON.hh: Schema validation: allow a single item to | ||
| 4 | + appear anywhere that the schema has an array of a single item. | ||
| 5 | + This makes it possible to change an element of the schema from an | ||
| 6 | + item to an array to allow the data to accept an array where a | ||
| 7 | + single value was previously required. This change is needed to | ||
| 8 | + allow QPDFJob JSON to start accepting multiple items where a | ||
| 9 | + single item used to be expected without breaking backward | ||
| 10 | + compatibility. Without this change, the earlier fix to | ||
| 11 | + removeAttachment would be a breaking change. | ||
| 12 | + | ||
| 3 | * QPDFObjectHandle: for the methods insertItem, appendItem, | 13 | * QPDFObjectHandle: for the methods insertItem, appendItem, |
| 4 | eraseItem, replaceKey, and removeKey, add a corresponding | 14 | eraseItem, replaceKey, and removeKey, add a corresponding |
| 5 | "AndGetNew" and/or "AndGetOld" methods. The ones that end with | 15 | "AndGetNew" and/or "AndGetOld" methods. The ones that end with |
TODO
| @@ -19,10 +19,6 @@ Pending changes: | @@ -19,10 +19,6 @@ Pending changes: | ||
| 19 | appimage build specifically is setting the runpath, which is | 19 | appimage build specifically is setting the runpath, which is |
| 20 | actually desirable in this case. Make sure to understand and | 20 | actually desirable in this case. Make sure to understand and |
| 21 | document this. Maybe add a check for it in the build. | 21 | document this. Maybe add a check for it in the build. |
| 22 | -* Make job JSON accept a single element and treat as an array of one | ||
| 23 | - when an array is expected. This allows for making things repeatable | ||
| 24 | - in the future without breaking compatibility and is needed for the | ||
| 25 | - remote-attachment fix to be backward-compatible. | ||
| 26 | * Decide what to do about #664 (get*Box) | 22 | * Decide what to do about #664 (get*Box) |
| 27 | * Add an option --ignore-encryption to ignore encryption information | 23 | * Add an option --ignore-encryption to ignore encryption information |
| 28 | and treat encrypted files as if they weren't encrypted. This should | 24 | and treat encrypted files as if they weren't encrypted. This should |
include/qpdf/JSON.hh
| @@ -184,6 +184,14 @@ class JSON | @@ -184,6 +184,14 @@ class JSON | ||
| 184 | // * The schema is a nested structure containing dictionaries, | 184 | // * The schema is a nested structure containing dictionaries, |
| 185 | // single-element arrays, and strings only. | 185 | // single-element arrays, and strings only. |
| 186 | // * Recursively walk the schema. | 186 | // * Recursively walk the schema. |
| 187 | + // * Whenever the schema has an array of length 1 and the object | ||
| 188 | + // does not have an array in the corresponding location, | ||
| 189 | + // validate the object against the array's single element. | ||
| 190 | + // This effectively enables a single element to appear in | ||
| 191 | + // place of an array and be treated as if it were an array of | ||
| 192 | + // one element. This makes it possible to decide later that | ||
| 193 | + // something that used to contain a single element now allows | ||
| 194 | + // an array without invalidating any old data. | ||
| 187 | // * If the current value is a dictionary, this object must have | 195 | // * If the current value is a dictionary, this object must have |
| 188 | // a dictionary in the same place with the same keys. If flags | 196 | // a dictionary in the same place with the same keys. If flags |
| 189 | // contains f_optional, a key in the schema does not have to | 197 | // contains f_optional, a key in the schema does not have to |
libqpdf/JSON.cc
| @@ -538,26 +538,27 @@ JSON::checkSchemaInternal( | @@ -538,26 +538,27 @@ JSON::checkSchemaInternal( | ||
| 538 | } | 538 | } |
| 539 | } | 539 | } |
| 540 | } else if (sch_arr) { | 540 | } else if (sch_arr) { |
| 541 | - if (!this_arr) { | ||
| 542 | - QTC::TC("libtests", "JSON wanted array"); | ||
| 543 | - errors.push_back(err_prefix + " is supposed to be an array"); | ||
| 544 | - return false; | ||
| 545 | - } | ||
| 546 | if (sch_arr->elements.size() != 1) { | 541 | if (sch_arr->elements.size() != 1) { |
| 547 | QTC::TC("libtests", "JSON schema array error"); | 542 | QTC::TC("libtests", "JSON schema array error"); |
| 548 | errors.push_back( | 543 | errors.push_back( |
| 549 | err_prefix + " schema array contains other than one item"); | 544 | err_prefix + " schema array contains other than one item"); |
| 550 | return false; | 545 | return false; |
| 551 | } | 546 | } |
| 552 | - int i = 0; | ||
| 553 | - for (auto const& element: this_arr->elements) { | 547 | + if (this_arr) { |
| 548 | + int i = 0; | ||
| 549 | + for (auto const& element: this_arr->elements) { | ||
| 550 | + checkSchemaInternal( | ||
| 551 | + element.get(), | ||
| 552 | + sch_arr->elements.at(0).get(), | ||
| 553 | + flags, | ||
| 554 | + errors, | ||
| 555 | + prefix + "." + QUtil::int_to_string(i)); | ||
| 556 | + ++i; | ||
| 557 | + } | ||
| 558 | + } else { | ||
| 559 | + QTC::TC("libtests", "JSON schema array for single item"); | ||
| 554 | checkSchemaInternal( | 560 | checkSchemaInternal( |
| 555 | - element.get(), | ||
| 556 | - sch_arr->elements.at(0).get(), | ||
| 557 | - flags, | ||
| 558 | - errors, | ||
| 559 | - prefix + "." + QUtil::int_to_string(i)); | ||
| 560 | - ++i; | 561 | + this_v, sch_arr->elements.at(0).get(), flags, errors, prefix); |
| 561 | } | 562 | } |
| 562 | } else if (!sch_str) { | 563 | } else if (!sch_str) { |
| 563 | QTC::TC("libtests", "JSON schema other type"); | 564 | QTC::TC("libtests", "JSON schema other type"); |
libtests/json.cc
| @@ -162,7 +162,9 @@ test_schema() | @@ -162,7 +162,9 @@ test_schema() | ||
| 162 | "x": "ecks" | 162 | "x": "ecks" |
| 163 | }, | 163 | }, |
| 164 | "s": [ | 164 | "s": [ |
| 165 | - "esses" | 165 | + { |
| 166 | + "ss": "esses" | ||
| 167 | + } | ||
| 166 | ] | 168 | ] |
| 167 | } | 169 | } |
| 168 | }, | 170 | }, |
| @@ -236,6 +238,8 @@ test_schema() | @@ -236,6 +238,8 @@ test_schema() | ||
| 236 | JSON bad_schema = JSON::parse(R"({"a": true, "b": "potato?"})"); | 238 | JSON bad_schema = JSON::parse(R"({"a": true, "b": "potato?"})"); |
| 237 | check_schema(bad_schema, bad_schema, 0, false, "bad schema field type"); | 239 | check_schema(bad_schema, bad_schema, 0, false, "bad schema field type"); |
| 238 | 240 | ||
| 241 | + // "two" exercises the case of the JSON containing a single | ||
| 242 | + // element where the schema has an array. | ||
| 239 | JSON good = JSON::parse(R"( | 243 | JSON good = JSON::parse(R"( |
| 240 | { | 244 | { |
| 241 | "one": { | 245 | "one": { |
| @@ -245,17 +249,15 @@ test_schema() | @@ -245,17 +249,15 @@ test_schema() | ||
| 245 | "x": [1, null] | 249 | "x": [1, null] |
| 246 | }, | 250 | }, |
| 247 | "s": [ | 251 | "s": [ |
| 248 | - null, | ||
| 249 | - "anything" | 252 | + {"ss": null}, |
| 253 | + {"ss": "anything"} | ||
| 250 | ] | 254 | ] |
| 251 | } | 255 | } |
| 252 | }, | 256 | }, |
| 253 | - "two": [ | ||
| 254 | - { | ||
| 255 | - "glarp": "enspliel", | ||
| 256 | - "goose": 3.14 | ||
| 257 | - } | ||
| 258 | - ], | 257 | + "two": { |
| 258 | + "glarp": "enspliel", | ||
| 259 | + "goose": 3.14 | ||
| 260 | + }, | ||
| 259 | "three": { | 261 | "three": { |
| 260 | "<objid>": { | 262 | "<objid>": { |
| 261 | "z": "ebra" | 263 | "z": "ebra" |
libtests/libtests.testcov
| @@ -36,7 +36,6 @@ Pl_PNGFilter decodePaeth 0 | @@ -36,7 +36,6 @@ Pl_PNGFilter decodePaeth 0 | ||
| 36 | Pl_TIFFPredictor processRow 1 | 36 | Pl_TIFFPredictor processRow 1 |
| 37 | JSON wanted dictionary 0 | 37 | JSON wanted dictionary 0 |
| 38 | JSON key missing in object 0 | 38 | JSON key missing in object 0 |
| 39 | -JSON wanted array 0 | ||
| 40 | JSON schema array error 0 | 39 | JSON schema array error 0 |
| 41 | JSON key extra in object 0 | 40 | JSON key extra in object 0 |
| 42 | QPDFArgParser read args from stdin 0 | 41 | QPDFArgParser read args from stdin 0 |
| @@ -93,3 +92,4 @@ JSON 16 high high 0 | @@ -93,3 +92,4 @@ JSON 16 high high 0 | ||
| 93 | JSON 16 low not after high 0 | 92 | JSON 16 low not after high 0 |
| 94 | JSON 16 dangling high 0 | 93 | JSON 16 dangling high 0 |
| 95 | JSON parse duplicate key 0 | 94 | JSON parse duplicate key 0 |
| 95 | +JSON schema array for single item 0 |
libtests/qtest/json/json.out
| @@ -4,7 +4,8 @@ top-level object is supposed to be a dictionary | @@ -4,7 +4,8 @@ top-level object is supposed to be a dictionary | ||
| 4 | --- missing items | 4 | --- missing items |
| 5 | json key ".one.a": key "q" is present in schema but missing in object | 5 | json key ".one.a": key "q" is present in schema but missing in object |
| 6 | json key ".one.a.r" is supposed to be a dictionary | 6 | json key ".one.a.r" is supposed to be a dictionary |
| 7 | -json key ".one.a.s" is supposed to be an array | 7 | +json key ".one.a.s": key "ss" is present in schema but missing in object |
| 8 | +json key ".one.a.s": key "z" is not present in schema but appears in object | ||
| 8 | json key ".one.a": key "t" is not present in schema but appears in object | 9 | json key ".one.a": key "t" is not present in schema but appears in object |
| 9 | json key ".three.anything": key "z" is present in schema but missing in object | 10 | json key ".three.anything": key "z" is present in schema but missing in object |
| 10 | json key ".three.anything": key "x" is not present in schema but appears in object | 11 | json key ".three.anything": key "x" is not present in schema but appears in object |