1448120 - Can't use 0 as minimum or maximum integer in schema

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P3)

Description

[Oriol Brufau [:Oriol]]

In `browser.tabs.query`, the index parameter is defined with

  "index": {
    "type": "integer",
    "optional": true,
    "minimum": 0,
    "description": "The position of the tabs within their windows."
  },

https://searchfox.org/mozilla-central/rev/c217fbde244344fedfd07b57a740c694a456dbca/browser/components/extensions/schemas/tabs.json#715-720

But the minimum restriction is not enforced due to

  return new this(schema, schema.minimum || -Infinity, schema.maximum || Infinity);

https://searchfox.org/mozilla-central/rev/c217fbde244344fedfd07b57a740c694a456dbca/toolkit/components/extensions/Schemas.jsm#1801

Comment 1

[Shane Caraveo (:mixedpuppy)]

nice catch.

Comment 2

[Oriol Brufau [:Oriol]]

Attachment: Bug 1448120 - Allow 0 as minimum or maximum integer in webextension schema

Review commit: https://reviewboard.mozilla.org/r/230298/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/230298/

Comment 3

[Oriol Brufau [:Oriol]]

By the way, StringType's maxLength and ArrayType's maxItems are also affected. But since string and array lengths can't be negative, enforcing a maximum of 0 seems pointless. So they might not be worth fixing.

https://searchfox.org/mozilla-central/rev/c217fbde244344fedfd07b57a740c694a456dbca/toolkit/components/extensions/Schemas.jsm#1391
https://searchfox.org/mozilla-central/rev/c217fbde244344fedfd07b57a740c694a456dbca/toolkit/components/extensions/Schemas.jsm#1860

Comment 4

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8961525 [details]
Bug 1448120 - Allow 0 as minimum or maximum integer in webextension schema

https://reviewboard.mozilla.org/r/230298/#review235908

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75aaafadfe9d
Allow 0 as minimum or maximum integer in webextension schema r=mixedpuppy

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/75aaafadfe9d

Comment 7

[CosminB]

Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!

Comment 8

[Oriol Brufau [:Oriol]]

Attachment: test.xpi

(In reply to CosminB from comment #7)
> Is manual testing required on this bug? If yes, please provide some STR and
> the proper extension(if required) or set the “qe-verify -“ flag.

1. Open browser console
2. Install attached extension in about:debugging

In the console you should see a "TEST PASS - got an error" saying that the index -1 should at least be 0.
Before this bug you would get "TEST FAIL - didn't throw".

Comment 10

[Oriol Brufau [:Oriol]]

I guess bug 1451334 comment 1 belongs here:
> Unnecessary to have QA, it's covered by a test.