Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addon Validation Error: value equal null #2834

Closed
lidel opened this issue Sep 19, 2019 · 7 comments
Closed

Addon Validation Error: value equal null #2834

lidel opened this issue Sep 19, 2019 · 7 comments

Comments

@lidel
Copy link

lidel commented Sep 19, 2019

Information

Tried to upload fennec version for android:
ipfs_companion_beta_aeecbb5_-2.8.4.824_fennec.zip

Got error without any explanation:

Screenshot_2019-09-19 Validation Results for ipfs_companion_beta_aeecbb5_-2 8 4 824_fennec zip Developer Hub Add-ons for Fi

Bug?

I think it is because of "incognito": null in manifest.json. Addon loads fine into Firefox, however addons-linter does not support null keys and returns an error:

$ web-ext lint
Applying config files: ./package.json, ./web-ext-config.js
Cannot convert undefined or null to object

TypeError: Cannot convert undefined or null to object
    at hasOwnProperty (<anonymous>)
    at ManifestJSONParser.call [as checkCompatInfo] (/home/lidel/project/ipfs-companion/node_modules/addons-linter/dist/webpack:/src/parsers/manifestjson.js:153:43)
    at ManifestJSONParser.checkCompatInfo [as _validate] (/home/lidel/project/ipfs-companion/node_modules/addons-linter/dist/webpack:/src/parsers/manifestjson.js:401:16)
    at new _validate (/home/lidel/project/ipfs-companion/node_modules/addons-linter/dist/webpack:/src/parsers/manifestjson.js:94:12)
    at Linter._callee$ (/home/lidel/project/ipfs-companion/node_modules/addons-linter/dist/webpack:/src/linter.js:255:30)
    at tryCatch (/home/lidel/project/ipfs-companion/node_modules/regenerator-runtime/runtime.js:45:40)
    at Generator.invoke [as _invoke] (/home/lidel/project/ipfs-companion/node_modules/regenerator-runtime/runtime.js:271:22)
    at Generator.prototype.(anonymous function) [as next] (/home/lidel/project/ipfs-companion/node_modules/regenerator-runtime/runtime.js:97:21)
    at asyncGeneratorStep (/home/lidel/project/ipfs-companion/node_modules/addons-linter/dist/webpack:/node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:1)
    at _next (/home/lidel/project/ipfs-companion/node_modules/addons-linter/dist/webpack:/node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:1)

Solution?

I think null values should be allowed, it is a valid JSON.

@muffinresearch muffinresearch transferred this issue from mozilla/addons Oct 7, 2019
@stale
Copy link

stale bot commented Apr 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

@stale stale bot added the state:stale label Apr 4, 2020
@Rob--W Rob--W removed the state:stale label Apr 5, 2020
@Rob--W
Copy link
Member

Rob--W commented Apr 5, 2020

Still reproduced with latest web-ext and the latest AMO: https://addons-dev.allizom.org/en-US/developers/addon/submit/upload-listed

TypeError: Cannot convert undefined or null to object
    at hasOwnProperty (<anonymous>)
    at ManifestJSONParser.checkCompatInfo (/path/to/web-ext/node_modules/addons-linter/dist/webpack:/src/parsers/manifestjson.js:160:43)
    at ManifestJSONParser._validate (/path/to/web-ext/node_modules/addons-linter/dist/webpack:/src/parsers/manifestjson.js:417:16)
    at new ManifestJSONParser (/path/to/web-ext/node_modules/addons-linter/dist/webpack:/src/parsers/manifestjson.js:101:12)
    at Linter._callee$ (/path/to/web-ext/node_modules/addons-linter/dist/webpack:/src/linter.js:255:30)
    at tryCatch (/path/to/web-ext/node_modules/regenerator-runtime/runtime.js:45:40)
    at Generator.invoke [as _invoke] (/path/to/web-ext/node_modules/regenerator-runtime/runtime.js:274:22)
    at Generator.prototype.<computed> [as next] (/path/to/web-ext/node_modules/regenerator-runtime/runtime.js:97:21)
    at asyncGeneratorStep (/path/to/web-ext/node_modules/addons-linter/node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
    at _next (/path/to/web-ext/node_modules/addons-linter/node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)

@stale
Copy link

stale bot commented Oct 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

@stale stale bot added the state:stale label Oct 3, 2020
@stale stale bot closed this as completed Oct 17, 2020
@Rob--W Rob--W reopened this Oct 17, 2020
@thamimemel
Copy link
Contributor

thamimemel commented Oct 26, 2020

Should null be accepted ?
According to this incognito is an enum of ["spanning", "split", "not allowed"]
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/incognito

While looking this up i found another potential bug, the MDN docs say that it should accept ["spanning", "split", "not allowed"] but the current validation accepts only ["spanning", "not allowed"]

Edit: my bad "split" isn't supported in firefox 😭

@willdurand
Copy link
Member

@Rob--W what do you think we should do here? According to the comment above, null isn't valid.

@rpl
Copy link
Member

rpl commented Jan 15, 2021

@Rob--W what do you think we should do here? According to the comment above, null isn't valid.

@willdurand I just tried to reproduce this locally and I got a nicer validation result with incognito set to null:

 ./bin/addons-linter tmp/test-incognito-null
Validation Summary:

errors          2
notices         0
warnings        0

ERRORS:

Code                     Message                                  Description                                                   File            Line   Column
MANIFEST_FIELD_INVALID   "/incognito" should be string            See https://mzl.la/1ZOhoEN (MDN Docs) for more information.   manifest.json
JSON_INVALID             "/incognito" should be equal to one of   Your JSON file could not be parsed.
                         the allowed values

This validation result seems nice enough and so if the original error can't be reproduced anymore I would keep this behavior.

@Rob--W
Copy link
Member

Rob--W commented Jan 16, 2021

@rpl That warning looks good enough to me.

@Rob--W Rob--W closed this as completed Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants