Closed Bug 1739602 Opened 2 years ago Closed 2 years ago

`/developer/url` definition should have `format: url` in JSON schema

Categories

(WebExtensions :: General, defect, P3)

defect

Tracking

(firefox96 fixed)

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: willdurand, Assigned: willdurand)

References

Details

(Whiteboard: [addons-jira])

Attachments

(1 file)

We discovered this bug as part of an addons-linter review: https://github.com/mozilla/addons-linter/pull/4002.

/developer/url, introduced in Bug 1282977, overrides homepage_url when it is present according to MDN. Given that homepage_url has format: url in its definition (added in bug 1244805), /developer/url should definitely have that too.

As noted in the GitHub issue by :rpl, the result of not having format: url is that setting an invalid url in developer.url is ignored, and it does silently override a valid homepage url if one was specified, and the invalid url ends up in about:addons.

This is likely a breaking change because it is going to reject some
invalid URLs: I am not too sure about how we should handle this. Values
other than URLs weren't expected, though, so I don't know.

Assignee: nobody → wdurand
Status: NEW → ASSIGNED
Severity: -- → S3

If there are a significant number of existing extensions that use a non-URL in this key, then we need to add "onError": "warn" to the scheme to prevent them from breaking. Or just not adding a format validation.

:TheOne would you be able to run a code-search to know how many add-ons use developer.url in their manifest? (not taking the value into account as a first step)

Flags: needinfo?(awagner)

This is a little tricky to match precisely just using regular expressions/yara rules. Will suggested to start with searching for /"developer"/, which yielded 15,000 hits for 4,000 add-ons. Let me know if you have ideas for a more precise pattern, I'm happy to run another search.

Flags: needinfo?(awagner)

Is it possible to re-run the query to find all values that are in use? The full set can be reduced (later) to figure out how many invalid URLs are used.

We'd like to know that in order to know

  1. Whether we can need to treat an invalid value as a warning (comment 2).
  2. Whether we can make the linter stricter, per https://github.com/mozilla/addons-linter/issues/4038#issuecomment-966362648

(In reply to Rob Wu [:robwu] from comment #5)

Is it possible to re-run the query to find all values that are in use? The full set can be reduced (later) to figure out how many invalid URLs are used.

We'd like to know that in order to know

  1. Whether we can need to treat an invalid value as a warning (comment 2).
  2. Whether we can make the linter stricter, per https://github.com/mozilla/addons-linter/issues/4038#issuecomment-966362648

We'll definitely want to do that but finding more accurate data than what :TheOne already provided is going to take some time. We decided to use onError: warn in the meantime.

Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b2c9b6344cc
Add '"format": "url"' to 'developer.url' definition in JSON schema. r=rpl
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: