From f5312936e332c2e1e24bba05fbe256ec46f89fce Mon Sep 17 00:00:00 2001 From: Martin Giger Date: Fri, 23 Nov 2018 21:33:23 +0100 Subject: [PATCH] feat: detect API and manifest key incompatibilities with strict_min_version (#1493) --- README.md | 2 +- docs/import-firefox-schema.md | 4 +- docs/rules.md | 150 +++++------ package.json | 1 + src/const.js | 2 + src/messages/javascript.js | 24 ++ src/messages/manifestjson.js | 88 +++++++ src/parsers/manifestjson.js | 105 +++++++- src/rules/javascript/index.js | 3 + .../webextension-api-compat-android.js | 49 ++++ .../javascript/webextension-api-compat.js | 49 ++++ src/utils.js | 37 +++ tests/unit/helpers.js | 2 +- tests/unit/parsers/test.manifestjson.js | 232 +++++++++++++++++- .../test.incompatible_browser_api.js | 66 +++++ tests/unit/test.utils.js | 129 ++++++++++ 16 files changed, 865 insertions(+), 78 deletions(-) create mode 100644 src/rules/javascript/webextension-api-compat-android.js create mode 100644 src/rules/javascript/webextension-api-compat.js create mode 100644 tests/unit/rules/javascript/test.incompatible_browser_api.js diff --git a/README.md b/README.md index 7803b033a11..640d40f7dac 100644 --- a/README.md +++ b/README.md @@ -151,7 +151,7 @@ Dependencies are automatically kept up-to-date using [greenkeeper](http://greenk | npm run test-coverage-once | Runs the tests once with coverage | | npm run test-integration-linter | Runs our integration test-suite | | npm run prettier | Automatically format the whole code-base with Prettier | -| npm run prettier-dev | Automatically compare and format modified source files against the master branch | +| npm run prettier-dev | Automatically compare and format modified source files against the master branch | ### Building diff --git a/docs/import-firefox-schema.md b/docs/import-firefox-schema.md index 66b72f65286..36a1060bb8e 100644 --- a/docs/import-firefox-schema.md +++ b/docs/import-firefox-schema.md @@ -29,5 +29,5 @@ And import the schema. ## Things to check for further updates -* Review the schema update carefully and see if there are any updates that require additional linting / warning from the linter (e.g properties that are meant for internal add-ons and shouldn't be used by regular add-ons, ask around if unsure) -* Check for custom format validations in ``src/schema/formats.js`` and update accordingly with upstream code (e.g ``manifestShortcutKey``) +- Review the schema update carefully and see if there are any updates that require additional linting / warning from the linter (e.g properties that are meant for internal add-ons and shouldn't be used by regular add-ons, ask around if unsure) +- Check for custom format validations in `src/schema/formats.js` and update accordingly with upstream code (e.g `manifestShortcutKey`) diff --git a/docs/rules.md b/docs/rules.md index fdb37a7fe42..a41d3132da2 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -6,30 +6,32 @@ Rules are sorted by severity. ## JavaScript -| Message code | Severity | Description | -| ------------------------- | -------- | ---------------------------------------------------------------- | -| `KNOWN_LIBRARY` | notice | This is version of a JS library is known and generally accepted. | -| `OPENDIALOG_NONLIT_URI` | notice | openDialog called with non-literal parameter. | -| `EVENT_LISTENER_FOURTH` | notice | `addEventListener` called with truthy fourth argument. | -| `UNEXPECTED_GLOGAL_ARG` | warning | Unexpected global passed as an argument. | -| `NO_IMPLIED_EVAL` | warning | disallow the use of `eval()`-like methods. | -| `OPENDIALOG_REMOTE_URI` | warning | openDialog called with non-local URI. | -| `NO_DOCUMENT_WRITE` | warning | Use of `document.write` strongly discouraged. | -| `JS_SYNTAX_ERROR` | warning | JavaScript compile-time error. | -| `UNADVISED_LIBRARY` | warning | This version of a JS library is not recommended. | -| `TABS_GETSELECTED` | warning | Deprecated API `tabs.getSelected`. | -| `TABS_SENDREQUEST` | warning | Deprecated API `tabs.sendRequest`. | -| `TABS_GETALLINWINDOW` | warning | Deprecated API `tabs.getAllInWindow`. | -| `TABS_ONSELECTIONCHANGED` | warning | Deprecated API `tabs.onSelectionChanged`. | -| `TABS_ONACTIVECHANGED` | warning | Deprecated API `tabs.onActiveChanged`. | -| `EXT_SENDREQUEST` | warning | Deprecated API `extension.sendRequest`. | -| `EXT_ONREQUESTEXTERNAL` | warning | Deprecated API `extension.onRequestExternal`. | -| `EXT_ONREQUEST` | warning | Deprecated API `extension.onRequest`. | -| `APP_GETDETAILS` | warning | Deprecated API `app.getDetails`. | -| `STORAGE_LOCAL` | warning | Temporary IDs can cause issues with `storage.local`. | -| `STORAGE_SYNC` | warning | Temporary IDs can cause issues with `storage.sync`. | -| `IDENTITY_GETREDIRECTURL` | warning | Temporary IDs can cause issues with `identity.getRedirectURL`. | -| `BANNED_LIBRARY` | error | This version of a JS library is banned for security reasons. | +| Message code | Severity | Description | +| -------------------------- | -------- | -------------------------------------------------------------------------------------- | +| `KNOWN_LIBRARY` | notice | This is version of a JS library is known and generally accepted. | +| `OPENDIALOG_NONLIT_URI` | notice | openDialog called with non-literal parameter. | +| `EVENT_LISTENER_FOURTH` | notice | `addEventListener` called with truthy fourth argument. | +| `UNEXPECTED_GLOGAL_ARG` | warning | Unexpected global passed as an argument. | +| `NO_IMPLIED_EVAL` | warning | disallow the use of `eval()`-like methods. | +| `OPENDIALOG_REMOTE_URI` | warning | openDialog called with non-local URI. | +| `NO_DOCUMENT_WRITE` | warning | Use of `document.write` strongly discouraged. | +| `JS_SYNTAX_ERROR` | warning | JavaScript compile-time error. | +| `UNADVISED_LIBRARY` | warning | This version of a JS library is not recommended. | +| `TABS_GETSELECTED` | warning | Deprecated API `tabs.getSelected`. | +| `TABS_SENDREQUEST` | warning | Deprecated API `tabs.sendRequest`. | +| `TABS_GETALLINWINDOW` | warning | Deprecated API `tabs.getAllInWindow`. | +| `TABS_ONSELECTIONCHANGED` | warning | Deprecated API `tabs.onSelectionChanged`. | +| `TABS_ONACTIVECHANGED` | warning | Deprecated API `tabs.onActiveChanged`. | +| `EXT_SENDREQUEST` | warning | Deprecated API `extension.sendRequest`. | +| `EXT_ONREQUESTEXTERNAL` | warning | Deprecated API `extension.onRequestExternal`. | +| `EXT_ONREQUEST` | warning | Deprecated API `extension.onRequest`. | +| `APP_GETDETAILS` | warning | Deprecated API `app.getDetails`. | +| `STORAGE_LOCAL` | warning | Temporary IDs can cause issues with `storage.local`. | +| `STORAGE_SYNC` | warning | Temporary IDs can cause issues with `storage.sync`. | +| `IDENTITY_GETREDIRECTURL` | warning | Temporary IDs can cause issues with `identity.getRedirectURL`. | +| `BANNED_LIBRARY` | error | This version of a JS library is banned for security reasons. | +| `INCOMPATIBLE_API` | warning | API not compatible with `applications.gecko.strict_min_version` | +| `ANDROID_INCOMPATIBLE_API` | warning | API not compatible with Firefox for Android at `applications.gecko.strict_min_version` | ## Markup @@ -49,9 +51,9 @@ Rules are sorted by severity. ## Content -| Message code | Severity | Description | -| -------------- | -------- | ---------------------- | -| `HIDDEN_FILE` | warning | Hidden file flagged. | +| Message code | Severity | Description | +| -------------- | -------- | ----------------------- | +| `HIDDEN_FILE` | warning | Hidden file flagged. | | `FLAGGED_FILE` | warning | Flagged filename found. | ## Package layout @@ -59,12 +61,12 @@ Rules are sorted by severity. | Message code | Severity | Description | | -------------------------- | -------- | --------------------------------------------------- | | `MOZILLA_COND_OF_USE` | notice | Mozilla conditions of use violation. | -| `FLAGGED_FILE_TYPE` | notice | (Binary) Flagged file type found. | -| `FLAGGED_FILE_EXTENSION` | warning | Flagged file extensions found | +| `FLAGGED_FILE_TYPE` | notice | (Binary) Flagged file type found. | +| `FLAGGED_FILE_EXTENSION` | warning | Flagged file extensions found | | `DUPLICATE_XPI_ENTRY` | warning | Package contains duplicate entries | | `ALREADY_SIGNED` | warning | Already signed | | `COINMINER_USAGE_DETECTED` | warning | Firefox add-ons are not allowed to run coin miners. | -| `BAD_ZIPFILE` | error | Bad zip file | +| `BAD_ZIPFILE` | error | Bad zip file | | `FILE_TOO_LARGE` | error | File is too large to parse | ## Type detection @@ -75,52 +77,56 @@ Rules are sorted by severity. ## Language packs -| Message code | Severity | Description | -| -------------- | -------- | --------------------------- | +| Message code | Severity | Description | +| -------------- | -------- | ----------------------------- | | FLUENT_INVALID | warning | Invalid fluent template file. | ## Web Extensions / manifest.json -| Message code | Severity | Description | -| --------------------------- | -------- | -------------------------------------------------------------------- | -| `MANIFEST_UNUSED_UPDATE` | notice | update_url ignored in manifest.json | -| `PROP_VERSION_TOOLKIT_ONLY` | notice | version is in the toolkit format in manifest.json | -| `CORRUPT_ICON_FILE` | warning | Icons must not be corrupt | -| `MANIFEST_CSP` | warning | content_security_policy in manifest.json means more review | -| `MANIFEST_CSP_UNSAFE_EVAL` | warning | usage of 'unsafe-eval' is strongly discouraged | -| `MANIFEST_PERMISSIONS` | warning | Unknown permission | -| `NO_MESSAGES_FILE` | warning | When default_locale is specified a matching messages.json must exist | -| `NO_DEFAULT_LOCALE` | warning | When \_locales directory exists, default_locale must exist | -| `UNSAFE_VAR_ASSIGNMENT` | warning | Assignment using dynamic, unsanitized values | -| `UNSUPPORTED_API` | warning | Unsupported or unknown browser API | -| `DANGEROUS_EVAL` | warning | `eval` and the `Function` constructor are discouraged | -| `STRICT_MAX_VERSION` | warning | strict_max_version not required | -| `PREDEFINED_MESSAGE_NAME` | warning | String name is reserved for a predefined | -| `MISSING_PLACEHOLDER` | warning | Placeholder for message is not | -| `WRONG_ICON_EXTENSION` | error | Icons must have valid extension | -| `MANIFEST_UPDATE_URL` | error | update_url not allowed in manifest.json | -| `MANIFEST_FIELD_REQUIRED` | error | A required field is missing | -| `MANIFEST_FIELD_INVALID` | error | A field is invalid | -| `MANIFEST_BAD_PERMISSION` | error | Bad permission | -| `JSON_BLOCK_COMMENTS` | error | Block Comments are not allowed in JSON | -| `MANIFEST_INVALID_CONTENT` | error | This add-on contains forbidden content | -| `CONTENT_SCRIPT_NOT_FOUND` | error | Content script file could not be found | -| `CONTENT_SCRIPT_EMPTY` | error | Content script file name should not be empty | -| `NO_MESSAGE` | error | Translation string is missing the message | -| `INVALID_MESSAGE_NAME` | error | String name contains invalid characters | -| `INVALID_PLACEHOLDER_NAME` | error | Placeholder name contains invalid characters | -| `NO_PLACEHOLDER_CONTENT` | error | Placeholder is missing the content | -| `JSON_INVALID` | error | JSON is not well formed | -| `JSON_DUPLICATE_KEY` | error | Duplicate key in JSON | -| `MANIFEST_VERSION_INVALID` | error | manifest_version in manifest.json is not valid. | -| `PROP_NAME_MISSING` | error | name property missing from manifest.json | -| `PROP_NAME_INVALID` | error | name property is invalid in manifest.json | -| `PROP_VERSION_MISSING` | error | version property missing from manifest.json | -| `PROP_VERSION_INVALID` | error | version is invalid in manifest.json | -| `MANIFEST_DICT_NOT_FOUND` | error | A dictionary file defined in the manifest could not be found | -| `MANIFEST_MULTIPLE_DICTS` | error | Multiple dictionaries found | -| `MANIFEST_EMPTY_DICTS` | error | Empty `dictionaries` object | -| `MANIFEST_DICT_MISSING_ID` | error | Missing `applications.gecko.id` property for a dictionary | +| Message code | Severity | Description | +| ------------------------------------------------------- | -------- | ----------------------------------------------------------------------------------------------- | +| `MANIFEST_UNUSED_UPDATE` | notice | update_url ignored in manifest.json | +| `PROP_VERSION_TOOLKIT_ONLY` | notice | version is in the toolkit format in manifest.json | +| `CORRUPT_ICON_FILE` | warning | Icons must not be corrupt | +| `MANIFEST_CSP` | warning | content_security_policy in manifest.json means more review | +| `MANIFEST_CSP_UNSAFE_EVAL` | warning | usage of 'unsafe-eval' is strongly discouraged | +| `MANIFEST_PERMISSIONS` | warning | Unknown permission | +| `NO_MESSAGES_FILE` | warning | When default_locale is specified a matching messages.json must exist | +| `NO_DEFAULT_LOCALE` | warning | When \_locales directory exists, default_locale must exist | +| `UNSAFE_VAR_ASSIGNMENT` | warning | Assignment using dynamic, unsanitized values | +| `UNSUPPORTED_API` | warning | Unsupported or unknown browser API | +| `DANGEROUS_EVAL` | warning | `eval` and the `Function` constructor are discouraged | +| `STRICT_MAX_VERSION` | warning | strict_max_version not required | +| `PREDEFINED_MESSAGE_NAME` | warning | String name is reserved for a predefined | +| `MISSING_PLACEHOLDER` | warning | Placeholder for message is not | +| `WRONG_ICON_EXTENSION` | error | Icons must have valid extension | +| `MANIFEST_UPDATE_URL` | error | update_url not allowed in manifest.json | +| `MANIFEST_FIELD_REQUIRED` | error | A required field is missing | +| `MANIFEST_FIELD_INVALID` | error | A field is invalid | +| `MANIFEST_BAD_PERMISSION` | error | Bad permission | +| `JSON_BLOCK_COMMENTS` | error | Block Comments are not allowed in JSON | +| `MANIFEST_INVALID_CONTENT` | error | This add-on contains forbidden content | +| `CONTENT_SCRIPT_NOT_FOUND` | error | Content script file could not be found | +| `CONTENT_SCRIPT_EMPTY` | error | Content script file name should not be empty | +| `NO_MESSAGE` | error | Translation string is missing the message | +| `INVALID_MESSAGE_NAME` | error | String name contains invalid characters | +| `INVALID_PLACEHOLDER_NAME` | error | Placeholder name contains invalid characters | +| `NO_PLACEHOLDER_CONTENT` | error | Placeholder is missing the content | +| `JSON_INVALID` | error | JSON is not well formed | +| `JSON_DUPLICATE_KEY` | error | Duplicate key in JSON | +| `MANIFEST_VERSION_INVALID` | error | manifest_version in manifest.json is not valid. | +| `PROP_NAME_MISSING` | error | name property missing from manifest.json | +| `PROP_NAME_INVALID` | error | name property is invalid in manifest.json | +| `PROP_VERSION_MISSING` | error | version property missing from manifest.json | +| `PROP_VERSION_INVALID` | error | version is invalid in manifest.json | +| `MANIFEST_DICT_NOT_FOUND` | error | A dictionary file defined in the manifest could not be found | +| `MANIFEST_MULTIPLE_DICTS` | error | Multiple dictionaries found | +| `MANIFEST_EMPTY_DICTS` | error | Empty `dictionaries` object | +| `MANIFEST_DICT_MISSING_ID` | error | Missing `applications.gecko.id` property for a dictionary | +| `KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION` | warning | Manifest key not compatible with `applications.gecko.strict_min_version` | +| `KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION` | warning | Manifest key not compatible with Firefox for Android at `applications.gecko.strict_min_version` | +| `PERMISSION_FIREFOX_UNSUPPORTED_BY_MIN_VERSION` | notice | Permission not compatible with `applications.gecko.strict_min_version` | +| `PERMISSION_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION` | notice | Permission not compatible with Firefox for Android at `applications.gecko.strict_min_version` | ### Static Theme / manifest.json diff --git a/package.json b/package.json index 6dc698341a8..e7350ad0762 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "glob": "7.1.3", "is-mergeable-object": "1.1.0", "jed": "1.1.1", + "mdn-browser-compat-data": "0.0.58", "os-locale": "3.0.1", "pino": "5.9.0", "po2json": "0.4.5", diff --git a/src/const.js b/src/const.js index bd0fb052438..d0eaff9c4be 100644 --- a/src/const.js +++ b/src/const.js @@ -25,6 +25,8 @@ export const ESLINT_RULE_MAPPING = Object.assign( 'webextension-api': ESLINT_WARNING, 'webextension-unsupported-api': ESLINT_WARNING, 'content-scripts-file-absent': ESLINT_ERROR, + 'webextension-api-compat': ESLINT_WARNING, + 'webextension-api-compat-android': ESLINT_WARNING, }, EXTERNAL_RULE_MAPPING ); diff --git a/src/messages/javascript.js b/src/messages/javascript.js index 4d95efde92e..f06a145a0cf 100644 --- a/src/messages/javascript.js +++ b/src/messages/javascript.js @@ -160,10 +160,34 @@ export const STORAGE_LOCAL = temporaryAPI('storage.local'); export const STORAGE_SYNC = temporaryAPI('storage.sync'); export const IDENTITY_GETREDIRECTURL = temporaryAPI('identity.getRedirectURL'); +export const INCOMPATIBLE_API = { + code: 'INCOMPATIBLE_API', + message: null, + messageFormat: i18n._( + '{{api}} is not supported in Firefox version {{minVersion}}' + ), + description: i18n._( + 'This API is not implemented by the given minimum Firefox version' + ), +}; + +export const ANDROID_INCOMPATIBLE_API = { + code: 'ANDROID_INCOMPATIBLE_API', + message: null, + messageFormat: i18n._( + '{{api}} is not supported in Firefox for Android version {{minVersion}}' + ), + description: i18n._( + 'This API is not implemented by the given minimum Firefox for Android version' + ), +}; + export const ESLINT_OVERWRITE_MESSAGE = { 'no-eval': DANGEROUS_EVAL, 'no-implied-eval': NO_IMPLIED_EVAL, 'no-new-func': DANGEROUS_EVAL, 'no-unsafe-innerhtml/no-unsafe-innerhtml': UNSAFE_DYNAMIC_VARIABLE_ASSIGNMENT, 'webextension-unsupported-api': UNSUPPORTED_API, + 'webextension-api-compat': INCOMPATIBLE_API, + 'webextension-api-compat-android': ANDROID_INCOMPATIBLE_API, }; diff --git a/src/messages/manifestjson.js b/src/messages/manifestjson.js index 0136e98d75f..81343efa311 100644 --- a/src/messages/manifestjson.js +++ b/src/messages/manifestjson.js @@ -418,3 +418,91 @@ export function noMessagesFileInLocales(path) { file: MANIFEST_JSON, }; } + +export const KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION = + 'KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION'; +export function keyFirefoxUnsupportedByMinVersion( + key, + minVersion, + versionAdded +) { + return { + code: KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION, + message: i18n._( + 'Manifest key not supported by the specified minimum Firefox version' + ), + description: i18n.sprintf( + i18n._(oneLine`"strict_min_version" requires Firefox %(minVersion)s, which + was released before version %(versionAdded)s introduced support for + "%(key)s".`), + { key, minVersion, versionAdded } + ), + file: MANIFEST_JSON, + }; +} + +export const PERMISSION_FIREFOX_UNSUPPORTED_BY_MIN_VERSION = + 'èERMISSION_FIREFOX_UNSUPPORTED_BY_MIN_VERSION'; +export function permissionFirefoxUnsupportedByMinVersion( + key, + minVersion, + versionAdded +) { + return { + code: PERMISSION_FIREFOX_UNSUPPORTED_BY_MIN_VERSION, + message: i18n._( + 'Permission not supported by the specified minimum Firefox version' + ), + description: i18n.sprintf( + i18n._(oneLine`"strict_min_version" requires Firefox %(minVersion)s, which + was released before version %(versionAdded)s introduced support for + "%(key)s".`), + { key, minVersion, versionAdded } + ), + file: MANIFEST_JSON, + }; +} + +export const KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION = + 'KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION'; +export function keyFirefoxAndroidUnsupportedByMinVersion( + key, + minVersion, + versionAdded +) { + return { + code: KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION, + message: i18n._( + 'Manifest key not supported by the specified minimum Firefox for Android version' + ), + description: i18n.sprintf( + i18n._(oneLine`"strict_min_version" requires Firefox for Android + %(minVersion)s, which was released before version %(versionAdded)s + introduced support for "%(key)s".`), + { key, minVersion, versionAdded } + ), + file: MANIFEST_JSON, + }; +} + +export const PERMISSION_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION = + 'PERMISSION_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION'; +export function permissionFirefoxAndroidUnsupportedByMinVersion( + key, + minVersion, + versionAdded +) { + return { + code: PERMISSION_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION, + message: i18n._( + 'Permission not supported by the specified minimum Firefox for Android version' + ), + description: i18n.sprintf( + i18n._(oneLine`"strict_min_version" requires Firefox for Android + %(minVersion)s, which was released before version %(versionAdded)s + introduced support for "%(key)s".`), + { key, minVersion, versionAdded } + ), + file: MANIFEST_JSON, + }; +} diff --git a/src/parsers/manifestjson.js b/src/parsers/manifestjson.js index 0ce50b6f345..9996fd353b7 100644 --- a/src/parsers/manifestjson.js +++ b/src/parsers/manifestjson.js @@ -6,6 +6,7 @@ import RJSON from 'relaxed-json'; import { oneLine } from 'common-tags'; import probeImageSize from 'probe-image-size'; import upath from 'upath'; +import bcd from 'mdn-browser-compat-data'; import { getDefaultConfigValue } from 'yargs-options'; import { @@ -28,7 +29,12 @@ import log from 'logger'; import * as messages from 'messages'; import JSONParser from 'parsers/json'; import { isToolkitVersionString } from 'schema/formats'; -import { parseCspPolicy, normalizePath } from 'utils'; +import { + parseCspPolicy, + normalizePath, + firefoxStrictMinVersion, + basicCompatVersionComparison, +} from 'utils'; import BLOCKED_CONTENT_SCRIPT_HOSTS from 'blocked_content_script_hosts.txt'; async function getImageMetadata(io, iconPath) { @@ -88,6 +94,84 @@ export default class ManifestJSONParser extends JSONParser { } } + checkKeySupport(support, minVersion, key, isPermission = false) { + if (support.firefox) { + const versionAdded = support.firefox.version_added; + if (basicCompatVersionComparison(versionAdded, minVersion)) { + if (!isPermission) { + this.collector.addWarning( + messages.keyFirefoxUnsupportedByMinVersion( + key, + this.parsedJSON.applications.gecko.strict_min_version, + versionAdded + ) + ); + } else { + this.collector.addNotice( + messages.permissionFirefoxUnsupportedByMinVersion( + key, + this.parsedJSON.applications.gecko.strict_min_version, + versionAdded + ) + ); + } + } + } + if (support.firefox_android) { + const versionAddedAndroid = support.firefox_android.version_added; + if (basicCompatVersionComparison(versionAddedAndroid, minVersion)) { + if (!isPermission) { + this.collector.addWarning( + messages.keyFirefoxAndroidUnsupportedByMinVersion( + key, + this.parsedJSON.applications.gecko.strict_min_version, + versionAddedAndroid + ) + ); + } else { + this.collector.addNotice( + messages.permissionFirefoxAndroidUnsupportedByMinVersion( + key, + this.parsedJSON.applications.gecko.strict_min_version, + versionAddedAndroid + ) + ); + } + } + } + } + + checkCompatInfo(compatInfo, minVersion, key, manifestKeyValue) { + for (const subkey in compatInfo) { + if (Object.prototype.hasOwnProperty.call(compatInfo, subkey)) { + const subkeyInfo = compatInfo[subkey]; + if (subkey === '__compat') { + this.checkKeySupport(subkeyInfo.support, minVersion, key); + } else if ( + typeof manifestKeyValue === 'object' && + Object.prototype.hasOwnProperty.call(manifestKeyValue, subkey) + ) { + this.checkCompatInfo( + subkeyInfo, + minVersion, + `${key}.${subkey}`, + manifestKeyValue[subkey] + ); + } else if ( + (key === 'permissions' || key === 'optional_permissions') && + manifestKeyValue.includes(subkey) + ) { + this.checkKeySupport( + subkeyInfo.__compat.support, + minVersion, + `${key}:${subkey}`, + true + ); + } + } + } + } + errorLookup(error) { // This is the default message. let baseObject = messages.JSON_INVALID; @@ -287,6 +371,21 @@ export default class ManifestJSONParser extends JSONParser { } } + const minVersion = firefoxStrictMinVersion(this.parsedJSON); + if (!this.isLanguagePack && !this.isDictionary && minVersion) { + for (const key in bcd.webextensions.manifest) { + if (Object.prototype.hasOwnProperty.call(this.parsedJSON, key)) { + const compatInfo = bcd.webextensions.manifest[key]; + this.checkCompatInfo( + compatInfo, + minVersion, + key, + this.parsedJSON[key] + ); + } + } + } + if (isToolkitVersionString(this.parsedJSON.version)) { this.collector.addNotice(messages.PROP_VERSION_TOOLKIT_ONLY); } @@ -605,6 +704,10 @@ export default class ManifestJSONParser extends JSONParser { name: this.parsedJSON.name, type: PACKAGE_EXTENSION, version: this.parsedJSON.version, + firefoxMinVersion: + this.parsedJSON.applications && + this.parsedJSON.applications.gecko && + this.parsedJSON.applications.gecko.strict_min_version, }; } } diff --git a/src/rules/javascript/index.js b/src/rules/javascript/index.js index 5eb5212a0cd..758922be927 100644 --- a/src/rules/javascript/index.js +++ b/src/rules/javascript/index.js @@ -14,5 +14,8 @@ module.exports = { .default, 'webextension-unsupported-api': require('./webextension-unsupported-api') .default, + 'webextension-api-compat': require('./webextension-api-compat').default, + 'webextension-api-compat-android': require('./webextension-api-compat-android') + .default, }, }; diff --git a/src/rules/javascript/webextension-api-compat-android.js b/src/rules/javascript/webextension-api-compat-android.js new file mode 100644 index 00000000000..e270b1fb928 --- /dev/null +++ b/src/rules/javascript/webextension-api-compat-android.js @@ -0,0 +1,49 @@ +import bcd from 'mdn-browser-compat-data'; + +import { ANDROID_INCOMPATIBLE_API } from 'messages/javascript'; +import { + isBrowserNamespace, + firefoxStrictMinVersion, + isCompatible, +} from 'utils'; +import { hasBrowserApi } from 'schema/browser-apis'; + +export default { + create(context) { + const minVersion = + context.settings.addonMetadata && + firefoxStrictMinVersion({ + applications: { + gecko: { + strict_min_version: + context.settings.addonMetadata.firefoxMinVersion, + }, + }, + }); + if (minVersion) { + return { + MemberExpression(node) { + if ( + !node.computed && + node.object.object && + isBrowserNamespace(node.object.object.name) + ) { + const namespace = node.object.property.name; + const property = node.property.name; + const api = `${namespace}.${property}`; + if ( + hasBrowserApi(namespace, property) && + !isCompatible(bcd, api, minVersion, 'firefox_android') + ) { + context.report(node, ANDROID_INCOMPATIBLE_API.messageFormat, { + api, + minVersion: context.settings.addonMetadata.firefoxMinVersion, + }); + } + } + }, + }; + } + return {}; + }, +}; diff --git a/src/rules/javascript/webextension-api-compat.js b/src/rules/javascript/webextension-api-compat.js new file mode 100644 index 00000000000..4d918fc37fe --- /dev/null +++ b/src/rules/javascript/webextension-api-compat.js @@ -0,0 +1,49 @@ +import bcd from 'mdn-browser-compat-data'; + +import { INCOMPATIBLE_API } from 'messages/javascript'; +import { + isBrowserNamespace, + firefoxStrictMinVersion, + isCompatible, +} from 'utils'; +import { hasBrowserApi } from 'schema/browser-apis'; + +export default { + create(context) { + const minVersion = + context.settings.addonMetadata && + firefoxStrictMinVersion({ + applications: { + gecko: { + strict_min_version: + context.settings.addonMetadata.firefoxMinVersion, + }, + }, + }); + if (minVersion) { + return { + MemberExpression(node) { + if ( + !node.computed && + node.object.object && + isBrowserNamespace(node.object.object.name) + ) { + const namespace = node.object.property.name; + const property = node.property.name; + const api = `${namespace}.${property}`; + if ( + hasBrowserApi(namespace, property) && + !isCompatible(bcd, api, minVersion, 'firefox') + ) { + context.report(node, INCOMPATIBLE_API.messageFormat, { + api, + minVersion: context.settings.addonMetadata.firefoxMinVersion, + }); + } + } + }, + }; + } + return {}; + }, +}; diff --git a/src/utils.js b/src/utils.js index 8b8ad054619..154bb1173e0 100644 --- a/src/utils.js +++ b/src/utils.js @@ -365,3 +365,40 @@ export function couldBeMinifiedCode(code) { hugeLinesCount > hugeLinesThreshold ); } + +export function firefoxStrictMinVersion(manifestJson) { + if ( + manifestJson.applications && + manifestJson.applications.gecko && + manifestJson.applications.gecko.strict_min_version + ) { + return parseInt( + manifestJson.applications.gecko.strict_min_version.split('.')[0], + 10 + ); + } + return null; +} + +export function basicCompatVersionComparison(versionAdded, minVersion) { + const asNumber = parseInt(versionAdded, 10); + return !Number.isNaN(asNumber) && asNumber > minVersion; +} + +export function isCompatible(bcd, path, minVersion, application) { + const steps = path.split('.'); + let { api } = bcd.webextensions; + for (const step of steps) { + if (Object.prototype.hasOwnProperty.call(api, step)) { + api = api[step]; + } else { + break; + } + } + // API namespace may be undocumented or not implemented, ignore in that case. + if (api.__compat) { + const versionAdded = api.__compat.support[application].version_added; + return !basicCompatVersionComparison(versionAdded, minVersion); + } + return true; +} diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js index 73aec3ea20a..1baca377cec 100644 --- a/tests/unit/helpers.js +++ b/tests/unit/helpers.js @@ -132,7 +132,7 @@ export function validManifestJSON(extra) { applications: { gecko: { id: '{daf44bf7-a45e-4450-979c-91cf07434c3d}', - strict_min_version: '40.0.0', + strict_min_version: '48.0.0', }, }, version: '0.1', diff --git a/tests/unit/parsers/test.manifestjson.js b/tests/unit/parsers/test.manifestjson.js index bb3ad6435c9..43221d3666f 100644 --- a/tests/unit/parsers/test.manifestjson.js +++ b/tests/unit/parsers/test.manifestjson.js @@ -289,7 +289,7 @@ describe('ManifestJSONParser', () => { describe('enum', () => { it('should only return one message', () => { const addonLinter = new Linter({ _: ['bar'] }); - const json = validManifestJSON({ permissions: ['tabs', 'wat'] }); + const json = validManifestJSON({ permissions: ['alarms', 'wat'] }); const manifestJSONParser = new ManifestJSONParser( json, addonLinter.collector @@ -457,6 +457,236 @@ describe('ManifestJSONParser', () => { }); }); + describe('strict_min_version', () => { + it('should warn when using a manifest key before Firefox marks it as supported', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON({ + applications: { + gecko: { + strict_min_version: '47.0', + }, + }, + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(6); + let fxIncompatCount = 0; + for (const warning of addonLinter.collector.warnings) { + if ( + warning.code !== + messages.KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION + ) { + expect(warning.code).toEqual( + messages.KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION + ); + ++fxIncompatCount; + } + } + expect(fxIncompatCount).toBeGreaterThan(0); + }); + + it('should warn when using a manifest key before Firefox for Android marks it as supported', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON({ + applications: { + gecko: { + strict_min_version: '47.0', + }, + }, + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(6); + let fxaIncompatCount = 0; + for (const warning of addonLinter.collector.warnings) { + if (warning.code !== messages.KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION) { + expect(warning.code).toEqual( + messages.KEY_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION + ); + ++fxaIncompatCount; + } + } + expect(fxaIncompatCount).toBeGreaterThan(0); + }); + + it('should not warn when using an unsupported manifest key without strict_min_version', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON(); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(0); + }); + + it('should not warn when all manifest keys are supported in Firefox and Firefox for Android with the given strict_min_version', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON({ + applications: { + gecko: { + strict_min_version: '48.0a1', + }, + }, + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(0); + }); + + it('should ignore manifest key version support for dictionaries', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validDictionaryManifestJSON({ + applications: { + gecko: { + id: '@my-dictionary', + strict_min_version: '47.0', + }, + }, + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector, + { + io: { files: { 'path/to/fr.dic': '', 'path/to/fr.aff': '' } }, + } + ); + + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(0); + }); + + it('should ignore manifest key version support for langpacks', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validLangpackManifestJSON({ + applications: { + gecko: { + strict_min_version: '47.0', + }, + }, + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(0); + }); + + it('should warn on unsupported subkeys', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON({ + applications: { + gecko: { + strict_min_version: '54.0', + }, + }, + chrome_settings_overrides: { + homepage: 'https://example.com', + }, + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + + expect(manifestJSONParser.isValid).toEqual(true); + const { warnings } = addonLinter.collector; + expect(warnings.length).toEqual(2); + for (const warning of warnings) { + expect(warning.code).toEqual( + messages.KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION + ); + } + }); + + it('should warn on unsupported subsubkeys', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON({ + applications: { + gecko: { + strict_min_version: '56.0', + }, + }, + chrome_settings_overrides: { + search_provider: { + name: 'test', + search_url: 'https://example.com', + is_default: true, + }, + }, + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + + expect(manifestJSONParser.isValid).toEqual(true); + const { warnings } = addonLinter.collector; + expect(warnings.length).toEqual(1); + expect(warnings[0].code).toEqual( + messages.KEY_FIREFOX_UNSUPPORTED_BY_MIN_VERSION + ); + }); + + it('should add a notice on unsupported permissions', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON({ + applications: { + gecko: { + strict_min_version: '56.0', + }, + }, + optional_permissions: ['find'], + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(0); + expect(addonLinter.collector.notices.length).toEqual(1); + expect(addonLinter.collector.notices[0].code).toEqual( + messages.PERMISSION_FIREFOX_UNSUPPORTED_BY_MIN_VERSION + ); + }); + it('should add a notice on unsupported permissions on android', () => { + const addonLinter = new Linter({ _: ['bar'] }); + const json = validManifestJSON({ + applications: { + gecko: { + strict_min_version: '55.0', + }, + }, + permissions: ['browsingData'], + }); + const manifestJSONParser = new ManifestJSONParser( + json, + addonLinter.collector + ); + + expect(manifestJSONParser.isValid).toEqual(true); + expect(addonLinter.collector.warnings.length).toEqual(0); + expect(addonLinter.collector.notices.length).toEqual(1); + expect(addonLinter.collector.notices[0].code).toEqual( + messages.PERMISSION_FIREFOX_ANDROID_UNSUPPORTED_BY_MIN_VERSION + ); + }); + }); + describe('content security policy', () => { it('should warn on rules allowing remote code execution', () => { const addonLinter = new Linter({ _: ['bar'] }); diff --git a/tests/unit/rules/javascript/test.incompatible_browser_api.js b/tests/unit/rules/javascript/test.incompatible_browser_api.js new file mode 100644 index 00000000000..a0309b3f466 --- /dev/null +++ b/tests/unit/rules/javascript/test.incompatible_browser_api.js @@ -0,0 +1,66 @@ +import { VALIDATION_WARNING } from 'const'; +import JavaScriptScanner from 'scanners/javascript'; + +describe('incompatible browser APIs', () => { + it('flags event that is not yet implemented at strict_min_version', async () => { + const code = 'browser.bookmarks.onChanged.addListener(() => {});'; + const jsScanner = new JavaScriptScanner(code, 'badcode.js', { + addonMetadata: { firefoxMinVersion: '50.0' }, + }); + + const { linterMessages } = await jsScanner.scan(); + expect(linterMessages.length).toEqual(1); + expect(linterMessages[0].type).toEqual(VALIDATION_WARNING); + expect(linterMessages[0].message).toEqual( + 'bookmarks.onChanged is not supported in Firefox version 50.0' + ); + }); + + it('flags method that is not yet implemented at strict_min_version', async () => { + const code = 'browser.clipboard.setImageData({}, "png");'; + const jsScanner = new JavaScriptScanner(code, 'badcode.js', { + addonMetadata: { firefoxMinVersion: '50.0' }, + }); + + const { linterMessages } = await jsScanner.scan(); + expect(linterMessages.length).toEqual(2); + for (const msg of linterMessages) { + expect(msg.type).toEqual(VALIDATION_WARNING); + expect(msg.message).toEqual( + expect.stringMatching( + /clipboard\.setImageData is not supported in Firefox(?: for Android)? version 50\.0/ + ) + ); + } + }); + + it('does not flag APIs that are not implemented on Android', async () => { + const code = 'browser.browserAction.setBadgeText({ text: "lorem ipsum" });'; + const jsScanner = new JavaScriptScanner(code, 'badcode.js', { + addonMetadata: { firefoxMinVersion: '57.0a1' }, + }); + + const { linterMessages } = await jsScanner.scan(); + expect(linterMessages.length).toEqual(0); + }); + + it('does not flag method that is implemented in the strict_min_version', async () => { + const code = 'browser.clipboard.setImageData({}, "png");'; + const jsScanner = new JavaScriptScanner(code, 'badcode.js', { + addonMetadata: { firefoxMinVersion: '57.0a1' }, + }); + + const { linterMessages } = await jsScanner.scan(); + expect(linterMessages.length).toEqual(0); + }); + + it('does not flag method that is not supported without strict_min_version', async () => { + const code = 'browser.clipboard.setImageData({}, "png");'; + const jsScanner = new JavaScriptScanner(code, 'badcode.js', { + addonMetadata: {}, + }); + + const { linterMessages } = await jsScanner.scan(); + expect(linterMessages.length).toEqual(0); + }); +}); diff --git a/tests/unit/test.utils.js b/tests/unit/test.utils.js index c28a144ab20..af878e39c4b 100644 --- a/tests/unit/test.utils.js +++ b/tests/unit/test.utils.js @@ -14,6 +14,9 @@ import { isLocalUrl, normalizePath, parseCspPolicy, + firefoxStrictMinVersion, + basicCompatVersionComparison, + isCompatible, } from 'utils'; describe('getRootExpression()', () => { @@ -478,3 +481,129 @@ describe('normalizePath', () => { expect(normalizePath('foo/bar baz/qux')).toEqual('foo/bar baz/qux'); }); }); + +describe('firefoxStrictMinVersion', () => { + it('should return null without applications key', () => { + expect(firefoxStrictMinVersion({})).toEqual(null); + }); + + it('should return null without applications.gecko key', () => { + expect(firefoxStrictMinVersion({ applications: null })).toEqual(null); + }); + + it('should return null without applications.gecko.strict_min_version key', () => { + expect(firefoxStrictMinVersion({ applications: { gecko: null } })).toEqual( + null + ); + }); + + it('should return the first number in applications.gecko.strict_min_version', () => { + expect( + firefoxStrictMinVersion({ + applications: { gecko: { strict_min_version: '6' } }, + }) + ).toEqual(6); + expect( + firefoxStrictMinVersion({ + applications: { gecko: { strict_min_version: '60.0a1' } }, + }) + ).toEqual(60); + }); +}); + +describe('basicCompatVersionComparison', () => { + it('should return false when version added is a boolean', () => { + expect(basicCompatVersionComparison(false, 60)).toBe(false); + }); + + it('should return false when version added is undefined', () => { + expect(basicCompatVersionComparison(undefined, 60)).toBe(false); + }); + + it('should return true when version added is bigger than min version', () => { + expect(basicCompatVersionComparison('61', 60)).toBe(true); + }); + + it('should return false when version added is smaller than min version', () => { + expect(basicCompatVersionComparison('59', 60)).toBe(false); + }); +}); + +describe('isCompatible', () => { + const getBCD = (data) => ({ + webextensions: { + api: data, + }, + }); + + it('should be true if the given key path is not in the object', () => { + expect(isCompatible(getBCD({}), 'foo.bar', 60, 'firefox')).toBe(true); + }); + + it('should be true if the given key path has a compatibility of false', () => { + expect( + isCompatible( + getBCD({ + foo: { __compat: { support: { firefox: { version_added: false } } } }, + }), + 'foo', + 60, + 'firefox' + ) + ).toBe(true); + }); + + it('should be true if the given key path is compatible with the minVersion', () => { + expect( + isCompatible( + getBCD({ + foo: { __compat: { support: { firefox: { version_added: '60' } } } }, + }), + 'foo', + 60, + 'firefox' + ) + ).toBe(true); + }); + + it('should be false if the given key path is incompatible with the minVersion', () => { + expect( + isCompatible( + getBCD({ + foo: { + __compat: { support: { firefox_android: { version_added: '61' } } }, + }, + }), + 'foo', + 60, + 'firefox_android' + ) + ).toBe(false); + }); + + it('should fall back to deepest matching compat info', () => { + expect( + isCompatible( + getBCD({ + foo: { __compat: { support: { firefox: { version_added: '61' } } } }, + }), + 'foo.bar', + 60, + 'firefox' + ) + ).toBe(false); + }); + + it('should be compatible if no specific version is specified', () => { + expect( + isCompatible( + getBCD({ + foo: { __compat: { support: { firefox: { version_added: true } } } }, + }), + 'foo.bar', + 60, + 'firefox' + ) + ).toBe(true); + }); +});