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

Running web-ext sign results in a timeout at "Validating add-on" if name in manifest includes "Firefox" #690

Closed
lmorchard opened this issue Apr 5, 2018 · 13 comments · Fixed by mozilla/addons-server#12761
Assignees
Labels
component:api priority:p3 This priority level reflects our backlog. state:verified_fixed
Milestone

Comments

@lmorchard
Copy link

We recently changed the name of our add-on from "Themer" to "Firefox Color". The ID also changed from themer-local@mozilla.com to firefoxcolor-local@mozilla.com

After that change, I get a timeout at the "Validating add-on" step when running web-ext sign. When I revert the name change, the signing works without a timeout.

Attached are two versions of the add-on, one before and one after the name change:

@lmorchard
Copy link
Author

Trying to run diffs between the files in the two versions. Seeing no difference in background.js or anything in images/, just the name change in contentScript.js, and these changes between manifest.json:

lmorchard@preciso:~/devel/ThemesRFun/build$ diff themer-extension/extension/manifest.json firefoxcolor-extension/exten
sion/manifest.json
2c2
<   "name": "Themer (local)",
---
>   "name": "Firefox Color (local)",
10c10
<     "default_title": "Themer",
---
>     "default_title": "Firefox Color",
39c39
<       "id": "themer-local@mozilla.com",
---
>       "id": "firefoxcolor-local@mozilla.com",
44,45c44,45
<   "version": "0.0.36",
<   "description": "Theming demo for Firefox Quantum and beyond.",
---
>   "version": "0.0.34",
>   "description": "Theming experiment for Firefox Quantum and beyond.",
47c47
<   "homepage_url": "https://mozilla.github.io/Themer/"
---
>   "homepage_url": "https://mozilla.github.io/FirefoxColor/"

@lmorchard
Copy link
Author

lmorchard commented Apr 9, 2018

To be more clear about steps to reproduce - the zip files I attached are extracts from my build directory. In a nutshell, skipping my build process, the zip files could be used like so:

unzip firefoxcolor-extension.zip
export WEB_EXT_API_KEY={my user ID here}
export WEB_EXT_API_SECRET={my secret here}
web-ext sign --source-dir=extension

This gets to the "Validating add-on" stage, populates the brackets with dots (e.g. [. ... . . .. ]) but then hangs on the kind of barber-pole progress stage when the dots scroll through the brackets.

If I instead use themer-extension.zip and ensure the version in manifest.json is updated and unique, the above steps complete successfully.

Also FWIW, I'm trying to sign this for self-hosting and not a listed release on AMO. My sole interaction with AMO with this project has been getting API keys and using web-ext sign

@lmorchard
Copy link
Author

lmorchard commented Apr 9, 2018

One more thing I just discovered in trying to reduce the test case down:

If I change the name in manifest.json from "Firefox Color (local)" to "Themer Color (local)", web-ext sign completes successfully.

If I change it back to "Firefox Color (local)", the timeout occurs again.

It seems like just having the word "Firefox" in the name is causing an issue?

@lmorchard lmorchard changed the title Running web-ext sign results in a timeout at "Validating add-on" Running web-ext sign results in a timeout at "Validating add-on" if name in manifest includes "Firefox" Apr 9, 2018
@kumar303
Copy link
Contributor

kumar303 commented Apr 9, 2018

Hi @lmorchard. I'm away on parental leave but maybe @rpl can help if it's a web-ext issue (it might be). Could you attach a verbose log? That will show some of the HTTP requests to the API.

@wagnerand
Copy link
Member

Good catch @lmorchard. We do have a trademark check in our validation tools. However, of course the validation should not timeout when that rule is triggered.

@lmorchard
Copy link
Author

lmorchard commented Apr 10, 2018

Could you attach a verbose log? That will show some of the HTTP requests to the API.

Captured verbose log output - log.txt

Pretty long, but most of it looks like the repeated requests at the end of the process leading up to the timeout.

@muffinresearch
Copy link
Contributor

@EnTeQuAk would you be able to look into this?

@muffinresearch muffinresearch added the priority:p3 This priority level reflects our backlog. label May 2, 2018
@philip-lamb
Copy link

philip-lamb commented Oct 30, 2019

I've also just run into this testing signing for add-ons for Firefox Reality. "name": "Firefox Reality WebCompat" fails with timeout. While this is actually a Mozilla-authored extension, I tried testing the official policy published at https://extensionworkshop.com/documentation/publish/add-on-policies/#content (which states its OK to include "for Firefox" in the name) with the same results, i.e. "name": "WebCompat for Firefox Reality" fails with timeout. Changing this to "name": "WebCompat for FxR" succeeds.

@diox
Copy link
Member

diox commented Oct 30, 2019

https://sentry.prod.mozaws.net/operations/olympia-prod/issues/6565855/?query=is:unresolved

This should have resulted in a proper validation error, not an uncaught exception...

@diox diox assigned diox and unassigned EnTeQuAk Oct 30, 2019
@diox
Copy link
Member

diox commented Oct 30, 2019

We've identified the issue. It's limited to users with a mozilla email using the API.

The check for Mozilla Trademark is made twice, but the second time, if using the API, is made without the user that had submitted the add-on, and the validation error it raises is then uncaught. If using an user with a non mozilla email to submit, the error is caught during the initial call, and we never reach the second one so it works as expected, showing a validation error to the user.

@diox
Copy link
Member

diox commented Oct 30, 2019

This will be fixed in prod next week.

QA: make sure submitting an add-on with "Firefox" in the name works for people with a mozilla email, and raises a validation error otherwise. This should apply to both devhub and API submissions.

@AlexandraMoga
Copy link

Using an add-on that has the "mozilla" or "firefox" trademark in its name or guid, I was able to reproduce the issue when running web-ext sign on stage with a mozilla user.

After that, I've run the same test on -dev and the issue didn't reproduced anymore => the add-on is successfully signed.
When I'm authenticating with a regular user, the trademark violation error is raised (status 400).

Additionally, I've submitted the same type of extensions in devhub and the results were as expected - i.e 'mozilla' user sees no error, while a regular user sees the trademark violation error.

@diox something I've noticed and thought it was worth sharing: when I've uploaded an add-on on stage through the signing API directly, i.e not through web-ext sign , there was no validation timeout. Just for the record, I'm using Postman for manual API testing.

@diox
Copy link
Member

diox commented Nov 4, 2019

@diox something I've noticed and thought it was worth sharing: when I've uploaded an add-on on stage through the signing API directly, i.e not through web-ext sign , there was no validation timeout. Just for the record, I'm using Postman for manual API testing.

Yes, this "timeout" is purely on web-ext side, because the addons-server bug I fixed prevented validation to finish. You would indeed never see the error using the API alone, but it would never be considered valid/invalid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api priority:p3 This priority level reflects our backlog. state:verified_fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants