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

VERIFIED FIXED in 5.11

Status

addons.mozilla.org Graveyard
Public Pages
VERIFIED FIXED
8 years ago
2 years ago

People

(Reporter: jbalogh, Assigned: wenzel)

Tracking

Details

(Whiteboard: [z][crash], URL)

(Reporter)

Description

8 years ago
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']}>,
Related to bug 565581, or is there no shared code here at all?
(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.
(Assignee)

Comment 3

8 years ago
Fix and test: http://github.com/jbalogh/zamboni/commits/c4c2f18
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Jeff/Fred: have a URL that trigger(s/ed) this exception?
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?
(Assignee)

Comment 6

8 years ago
(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.
(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!
(Assignee)

Comment 8

8 years ago
I'm on it :) waiting for our test suite to slug its way to success.
(Assignee)

Comment 9

8 years ago
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.
(Assignee)

Comment 12

8 years ago
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.
(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.
(Reporter)

Updated

8 years ago
Duplicate of this bug: 567760
(Reporter)

Comment 15

8 years ago
(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.
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.