565688 - int(GET['addons-author-addons-select']) needs a try-except

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Jeff Balogh (:jbalogh)]

File "/data/virtualenvs/zamboni/lib/python2.6/site-packages/django/core/handlers/base.py", line 101, in get_response
   response = callback(request, *callback_args, **callback_kwargs)

 File "/data/amo_python/www/prod/zamboni/apps/addons/views.py", line 23, in decorated
   target_id = int(request.GET.get('addons-author-addons-select'))

ValueError: invalid literal for int() with base 10: "'271"

I've also seen a UnicodeError here, so it might be appropriate to catch and drop Exception here.


Oh, here's the UnicodeError, because I enjoy the GET params:

Traceback (most recent call last):

 File "/data/virtualenvs/zamboni/lib/python2.6/site-packages/django/core/handlers/base.py", line 101, in get_response
   response = callback(request, *callback_args, **callback_kwargs)

 File "/data/amo_python/www/prod/zamboni/apps/addons/views.py", line 23, in decorated
   target_id = int(request.GET.get('addons-author-addons-select'))

UnicodeEncodeError: 'decimal' codec can't encode character u'\x00' in position 14: invalid decimal Unicode string


<WSGIRequest
GET:<QueryDict: {u'addons-author-addons-select': [u'suckmydick.php\x00271']}>,

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Related to bug 565581, or is there no shared code here at all?

Comment 2

[James Socol [:jsocol, :james]]

(In reply to comment #1)
> Related to bug 565581, or is there no shared code here at all?

Related in that we both need to catch ValueErrors. No shared code, though.

Comment 3

[Fred Wenzel [:wenzel]]

Fix and test: http://github.com/jbalogh/zamboni/commits/c4c2f18

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Jeff/Fred: have a URL that trigger(s/ed) this exception?

Comment 5

[Stephen Donner [:stephend] Not actively reading bugmail]

https://preview.addons.mozilla.org/en-US/firefox/addon/1865/?addons-author-addons-select=stephend3000 is now a 404, rather than triggering a traceback (I'm assuming, since I still don't yet get them).

Verified?

Comment 6

[Fred Wenzel [:wenzel]]

(In reply to comment #5)
> Verified?

Yes, I implemented it as a 404 when you provide bad data. Come to think of it, perhaps that should be a 400 though.

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #6)
> (In reply to comment #5)
> > Verified?
> 
> Yes, I implemented it as a 404 when you provide bad data. Come to think of it,
> perhaps that should be a 400 though.

One line fix; do it now, while no-one's looking!

Comment 8

[Fred Wenzel [:wenzel]]

I'm on it :) waiting for our test suite to slug its way to success.

Comment 9

[Fred Wenzel [:wenzel]]

s/404/400/

http://github.com/jbalogh/zamboni/commits/5bd6838

Please verify this once it's landed: Bad input should now throw an error 400. If, however, you feed it a number that doesn't correspond to a valid add-on, that's a 404.

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

https://preview.addons.mozilla.org/en-US/firefox/addon/1865/?addons-author-addons-select=stephend3000 now returns a 400 error when it returns "Invalid add-on ID."

https://preview.addons.mozilla.org/en-US/firefox/addon/yourmom/?addons-author-addons-select=stephend3000 now returns a 404 :-)

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

https://preview.addons.mozilla.org/en-US/firefox/addon/83478437894278978943/ is a better test, actually (verified, all the same).

Comment 12

[Fred Wenzel [:wenzel]]

For compleness' sake:

https://preview.addons.mozilla.org/en-US/firefox/addon/1865/?addons-author-addons-select=348293472938

redirects, but then returns a 404, both as expected.

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #12)
> For compleness' sake:
> 
> https://preview.addons.mozilla.org/en-US/firefox/addon/1865/?addons-author-addons-select=348293472938
> 
> redirects, but then returns a 404, both as expected.

Aha; thanks, Fred.  Didn't feel quite right; appreciate the link for posterity.

Comment 15

[Jeff Balogh (:jbalogh)]

(In reply to comment #10)
> https://preview.addons.mozilla.org/en-US/firefox/addon/1865/?addons-author-addons-select=stephend3000
> now returns a 400 error when it returns "Invalid add-on ID."

I don't think we should error out like that here.  From the logs, I don't think people are typing in these wonky links, they're following bad links from somewhere else.  We should ignore the bad query string or redirect to the same page with the bad part dropped.