800342 - [traceback] product and product_key can be Falsey

Status: RESOLVED WONTFIX

(support.mozilla.org :: Questions, task, P4)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

Got this in a support error email:

"""
Traceback (most recent call last):

  File "/data/www/support.mozilla.org/kitsune/vendor/src/django/django/core/handlers/base.py", line 111, in get_response
    response = callback(request, *callback_args, **callback_kwargs)

  File "/data/www/support.mozilla.org/kitsune/vendor/src/django-mobility/mobility/decorators.py", line 23, in wrapper
    return f(request, *args, **kw)

  File "/data/www/support.mozilla.org/kitsune/vendor/src/django-session-csrf/session_csrf/__init__.py", line 127, in wrapper
    response = f(request, *args, **kw)

  File "/data/www/support.mozilla.org/kitsune/apps/questions/views.py", line 247, in aaq
    product.get('tags'),

AttributeError: 'NoneType' object has no attribute 'get'


<WSGIRequest
GET:<QueryDict: {u'category': [u'd2'], u'showform': [u'1'], u'product': [u''], u'search': [u'Firefox crashes'], u'ns:expression(netsparker(0x0026A8))': [u'']}>,
"""

Url is this:

http://127.0.0.1:8000/en-US/questions/new?product=;ns:expression%28netsparker%280x0026A8%29%29;&category=d2&search=Firefox+crashes&showform=1



The code at that point looks like this:

    if product_key is None:
        product_key = request.GET.get('product')
    product = products.get(product_key)
    if product_key and not product:
        raise Http404


The problem with that conditional in this case is that product_key is Falsey, so it doesn't trigger the Http404 condition.

I don't entirely understand what's going on here or whether it's a requirement that we have a valid product for AAQ to continue, but later in the code it expects that product is not None. So I suspect we really want:

    if not product:
        raise Http404

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

Making this a P4. It's something one of the fuzzers picked up, but I'm hardpressed to figure out how a user could ever see this. Still, it's meh code and it sends us error email. Less of that is better.

Assuming my analysis and suggested fix are correct, it's either a 0 or a 1 pointer. Need to run this by Ricky to see if he agrees with the suggested fix first, though.

Comment 2

[Ricky Rosario [:rrosario, :r1cky]]

(In reply to Will Kahn-Greene [:willkg] from comment #0)
>     if not product:
>         raise Http404

We can't really do this because having no product is a valid case if there is no product_key specified yet. So it probably should be:

    if product_key is not None and not product:
        raise Http404

Comment 3

[Ricky Rosario [:rrosario, :r1cky]]

Also, since it isn't clear what is going on we should add a comment :)

Comment 4

[Will Kahn-Greene [:willkg] ET needinfo? me]

Sounds good to me. bd

Comment 5

[Ricky Rosario [:rrosario, :r1cky]]

2012Q4 is in the past. Moving on to the future!

Comment 6

[madalina]

Hate to do this but.. is this still valid?

Comment 7

[Will Kahn-Greene [:willkg] ET needinfo? me]

This url still kicks up an error:

https://support.mozilla.org/en-US/questions/new?product=;ns:expression%28netsparker%280x0026A8%29%29;&category=d2&search=Firefox+crashes&showform=1

The code is still the same:

498:    product_config = config.products.get(product_key)
        if product_key and not product_config:
            raise Http404


Seems like this is still valid.

Having said that, I doubt it has real impact on users since the url is obviously a fuzzed url.