941480 - Port urllib2 manifest fetching code to requests

Status: VERIFIED FIXED

(Marketplace Graveyard :: General, defect, P3)

Description

[Matt Basta [:basta]]

I'm unsure of why this is happening, and doing some basic debugging on my end hasn't yielded anything. The following manifest URL doesn't work when submitted to the devhub:

https://coinbase.com/mainfest.webapp

Note that it *is* SSL, but I have no reason to assume it's SNI again, since there appears to be no errors when I test it on my VPS.

It's also worth noting that the site uses Cloudflare as a proxy, though I don't believe that would cause any issues (and if it does, we should definitely either fix it or work with them to make this work well).

Comment 1

[Mathieu Pillard [:mat]]

It's on their end, they are blacklisting urllib user-agent :

curl -si https://coinbase.com/mainfest.webapp -H 'User-Agent: Whatever'
HTTP/1.1 200 OK

curl -i https://coinbase.com/mainfest.webapp -H 'User-Agent: Python-urllib/2.7' 
HTTP/1.1 403 Forbidden

curl -i https://coinbase.com/mainfest.webapp -H 'User-Agent: Python-urllib/2.6' 
HTTP/1.1 403 Forbidden


(Why we use urllib2.urlopen() (see mkt.developers.tasks._fetch_content) when fetching the manifest I have no idea, especially since we use requests in the validator itself ?!)

Comment 2

[Christopher Van Wiemeersch [:cvan]]

This reminds me of bug 888085 and bug 891654.

Comment 3

[Matt Basta [:basta]]

We should just port our code over to requests, which isn't blocked anyway.

Comment 4

[Fred Wenzel [:wenzel]]

I'd suggest specifying a meaningful custom user agent too. No reason not to identify the Marketplace when fetching things.

Comment 5

[Matt Basta [:basta]]

Yes and no. It's better to have a generic user agent because it prevents nefarious developers from having an easy way to subvert the review process. If the marketplace is easily distinguished, it's easier to serve a phony manifest that avoids triggering our rereview process.

Comment 6

[Fred Wenzel [:wenzel]]

They can already do that by IP address...

Comment 7

[Matt Basta [:basta]]

Our IP address(es) aren't in our source code.

Comment 9

[Christopher Van Wiemeersch [:cvan]]

https://github.com/mozilla/zamboni/commit/cabf11a

Comment 10

[Victor Carciu]

Can you please add some STRs to this bug or mark it as [qa-]?

Comment 11

[Christopher Van Wiemeersch [:cvan]]

Please test that submitting a manifest works OK. Thanks.

Comment 12

[Matt Basta [:basta]]

Also, please test that submitting apps via HTTPS works. See the bug this blocks for more information on that.

Comment 13

[Victor Carciu]

Verified as fixed, I was able to validate both https://coinbase.com/manifest.webapp and http://www.ntfusion.net/Content/annie/manifest.webapp

Comment 14

[Matt Basta [:basta]]

This broke some stuff. If a UnicodeDecodeError happens, everything fails.

Dec 19 20:09:39 cel1.mktweb.services.phx1.mozilla.com: [][] celery.worker.job:ERROR Task mkt.developers.tasks.fetch_manifest[] raised exception: UnicodeDecodeError('ascii', '\x00\x00\xfe\xff', 2, 3, 'ordinal not in range(128)') :/data/mkt.prod/www/marketplace.firefox.com/current/venv/lib/python2.6/site-packages/celery/utils/log.py:246#012Traceback (most recent call last):#012  File "/data/mkt.prod/www/marketplace.firefox.com/current/venv/lib/python2.6/site-packages/celery/task/trace.py", line 233, in trace_task#012    R = retval = fun(*args, **kwargs)#012  File "/data/mkt.prod/www/marketplace.firefox.com/current/venv/lib/python2.6/site-packages/celery/task/trace.py", line 420, in __protected_call__#012    return self.run(*args, **kwargs)#012  File "/data/mkt.prod/www/marketplace.firefox.com/current/venv/src/nuggets/celeryutils.py", line 35, in wrapped#012    return fun(*args, **kw)#012  File "/data/mkt.prod/www/marketplace.firefox.com/current/zamboni/apps/amo/decorators.py", line 157, in wrapper#012    return f(*args, **kw)#012  File "/data/mkt.prod/www/marketplace.firefox.com/current/zamboni/apps/amo/decorators.py", line 149, in wrapper#012    return f(*args, **kw)#012  File "/data/mkt.prod/www/marketplace.firefox.com/deploy-zamboni-prod-20131219004549-b9ab679923/zamboni/mkt/developers/tasks.py", line 397, in fetch_manifest#012    content = _fetch_manifest(url, upload)#012  File "/data/mkt.prod/www/marketplace.firefox.com/deploy-zamboni-prod-20131219004549-b9ab679923/zamboni/mkt/developers/tasks.py", line 387, in _fetch_manifest#012    content = strip_bom(content)#012  File "/data/mkt.prod/www/marketplace.firefox.com/current/zamboni/apps/amo/utils.py", line 961, in strip_bom#012    if data.startswith(bom):#012UnicodeDecodeError: 'ascii' codec can't decode byte 0xfe in position 2: ordinal not in range(128)

Comment 15

[Christopher Van Wiemeersch [:cvan]]

https://github.com/mozilla/zamboni/commit/f19f64d

Thanks, Basta. Sorry, y'all.

Comment 16

[Christopher Van Wiemeersch [:cvan]]

Manifest in question that was causing the `UnicodeDecodeError`:
http://www.yelp.com/firefox_manifest

Comment 17

[Victor Carciu]

(In reply to Christopher Van Wiemeersch [:cvan] from comment #16)
> Manifest in question that was causing the `UnicodeDecodeError`:
> http://www.yelp.com/firefox_manifest

I want to be sure that I will correctly verify this bug so please give some specific STRs .

Comment 18

[Christopher Van Wiemeersch [:cvan]]

(In reply to Victor Carciu from comment #17)
> (In reply to Christopher Van Wiemeersch [:cvan] from comment #16)
> > Manifest in question that was causing the `UnicodeDecodeError`:
> > http://www.yelp.com/firefox_manifest
> 
> I want to be sure that I will correctly verify this bug so please give some
> specific STRs .

Just validate that manifest in the validator during the submission flow.

Comment 19

[Victor Carciu]

App validation passed. Marking bug as verified...