UnicodeDecodeError on headers with bad characters

[armisael]

Checklist

• I have verified that that issue exists against the master branch of Django REST framework.
• I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• This is not a usage question.
• This cannot be dealt with as a third party library.
• I have reduced the issue to the simplest possible case.
• I have included a failing test as a pull request.

Steps to reproduce

This happened to one of our users; he was accessing our APIs with a browser, and it was sending an HTTP Accept header with a zero width space character in it. This makes django complain about a UnicodeDecodeError (django/http/multipartparser.py:641), which is called by rest framework (rest_framework/utils/mediatypes.py:55).

I did't include a failing test as a pull request, but I did pull the code, wrote a test and verified that it fails; I'm not sending a pull-request because I find it weird to submit a pull-request with only one (failing) test and no actual solution; the test is the following:

    def test_accept_header_non_ascii_chars(self):
        scheme = versioning.AcceptHeaderVersioning
        view = RequestVersionView.as_view(versioning_class=scheme)

        accept_header = b'text/html,application/xhtml+xml,application/xml;' \
                        b'q=0.9,image/webp,\xe2\x80\x8b*/*\xe2\x80\x8b;q=0.8'.decode('latin1')
        request = factory.get('/endpoint/', HTTP_ACCEPT=accept_header)
        view(request)

We're using decode('latin1') there because this is the only way to reproduce the error as we're seeing it in our production environment; using the unicode string u'q=0.9,image/webp,\u200B*/*\u200B;q=0.8' will make rest framework to complain about latin1 (the RFC standard encoding for HTTP headers) not being able to encode \u200b.

Expected behavior

Rest framework should not throw a UnicodeDecodeError, it should correctly parse the Accept header as text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,\u200B*/*\u200B;q=0.8.

Actual behavior

Rest framework throws a UnicodeDecodeError.

[tomchristie]

I'm not 100% sure about this on first pass.

Feel free to stop me if I get any of this wrong, but here's my take...

It's not possible to include a "zero width space" in an HTTP header, because HTTP header values are to be treated as iso-8859-1 encoded bytestrings, which doesn't include a zero wide space codepoint.

Attempting to test the expected behavior with the test client gets awkward because we're suddenly in fuzzy land of being able to pass either unicode or bytes as header values to the test client, which isn't really correct (I assume conferment WSGI clients only pass through bytestrings)

However, if you're seeing a UnicodeDecodeError caused by a client submitting an incorrectly formed Accept header, then that does sound like a bug. We ought to be gracefully handling them in all cases.

Are you able to replicate the issue using curl or python requests?

[xordoquy]

This makes django complain about a UnicodeDecodeError (django/http/multipartparser.py:641), which is called by rest framework (rest_framework/utils/mediatypes.py:55).

I'll tend to forward this one to Django. Is it possible to have the flu traceback ?

[armisael]

@tomchristie here's a requests example to show the exception:

requests.get(
    'http://localhost:8000/v2/collection', 
    headers={'Accept': b'\xe2\x80\x8b*/*\xe2\x80\x8b;q=0.8'}
).ok

@xordoquy full traceback:

Internal Server Error: /v2/collection
Traceback (most recent call last):
  File ".venv/lib/python3.5/site-packages/django/core/handlers/base.py", line 149, in get_response
    response = self.process_exception_by_middleware(e, request)
  File ".venv/lib/python3.5/site-packages/django/core/handlers/base.py", line 147, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File ".venv/lib/python3.5/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File ".venv/lib/python3.5/site-packages/rest_framework/viewsets.py", line 87, in view
    return self.dispatch(request, *args, **kwargs)
  File ".venv/lib/python3.5/site-packages/rest_framework/views.py", line 466, in dispatch
    response = self.handle_exception(exc)
  File ".venv/lib/python3.5/site-packages/rest_framework/views.py", line 454, in dispatch
    self.initial(request, *args, **kwargs)
  File ".venv/lib/python3.5/site-packages/rest_framework/views.py", line 381, in initial
    neg = self.perform_content_negotiation(request)
  File ".venv/lib/python3.5/site-packages/rest_framework/views.py", line 296, in perform_content_negotiation
    return conneg.select_renderer(request, renderers, self.format_kwarg)
  File ".venv/lib/python3.5/site-packages/rest_framework/negotiation.py", line 55, in select_renderer
    for media_type_set in order_by_precedence(accepts):
  File ".venv/lib/python3.5/site-packages/rest_framework/utils/mediatypes.py", line 44, in order_by_precedence
    precedence = _MediaType(media_type).precedence
  File ".venv/lib/python3.5/site-packages/rest_framework/utils/mediatypes.py", line 55, in __init__
    self.full_type, self.params = parse_header(media_type_str.encode(HTTP_HEADER_ENCODING))
  File ".venv/lib/python3.5/site-packages/django/http/multipartparser.py", line 641, in parse_header
    key = plist.pop(0).lower().decode('ascii')
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 0: ordinal not in range(128)

I was also unsure about where to submit this, actually.

[xordoquy]

@armisael thanks.

With the traceback, I'm not sure either where this should be handled. need to think further about.

[tomchristie]

Arguable that django's multipart parser should prefer .decode('iso8859-1') to .decode('ascii') here: https://github.com/django/django/blob/master/django/http/multipartparser.py#L672

RFC-7230

A recipient MUST parse an HTTP message as a sequence of octets in an encoding that is a superset of US-ASCII.

Given iso-8859-1 being historically treated as the header encoding, it'd seem reasonable to use it as a more lenient behavior.

It's a bit fuzzy, as...

Historically, HTTP has allowed field content with text in the ISO-8859-1 charset [ISO-8859-1], supporting other charsets only through use of [RFC2047] encoding. In practice, most HTTP header field values use only a subset of the US-ASCII charset [USASCII]. Newly defined header fields SHOULD limit their field values to US-ASCII octets. A recipient SHOULD treat other octets in field content (obs-text) as opaque data.

But having Django use iso-8859-1 would be more tolerant there (cannot raise a UnicodeDecodeError as all 256 codes are valid), which seems like marginally better behavior than 500'ing.

[tomchristie]

Similarly this behavior here...

https://github.com/django/django/blob/master/django/http/request.py#L388-L392

Seems unnecessary. The request headers are being decoded as ascii, and falling back to the (superset) of iso-8859-1, when we could just decode as 8859-1 in the first place.

[tomchristie]

This one is tangential, but might also be worth exploring...

That Django uses settings.DEFAULT_CHARSET for QueryDict https://github.com/django/django/blob/master/django/http/request.py#L377 seems odd too - the server settings shouldn't be affecting how we parse incoming requests, nor does it match up with the documented meaning...

Default charset to use for all HttpResponse objects, if a MIME type isn’t manually specified. Used with DEFAULT_CONTENT_TYPE to construct the Content-Type header.

Which is perfectly reasonable, but isn't at all how it's being used in the QueryDict context.

---

In summary: I believe there may be 2 or 3 independent PRs/issues to work through with the Django team here.

1. Simplify querystring decoding, to always just use iso-8859-1, rather than using treating it as a fallback.
2. Use more lenient iso-8859-1 in multipart parser and possible other sections of code that currently use ascii.
3. Dig into if settings.DEFAULT_CHARSET is being improperly applied when parsing incoming requests.

[tomchristie]

Alternate...

Use .decode('ascii', errors='ignore') for HTTP header decoding.

Eg:

>>> b'image/webp,\xe2\x80\x8b*/*\xe2\x80\x8b;q=0.8'.decode('ascii', errors='ignore')
'image/webp,*/*;q=0.8'

[raphendyr]

This might be related. So _MediaType doesn't produce similar result when converted to string as given input was. This is because self.params contains byte values:

>>> str(_MediaType('test/test; foobar=2; cat=dog'))
"test/test; cat=b'dog'; foobar=b'2'"

Not sure if parse_header works as supposed to.

This in init seems to fix my case:

self.params = dict(((k, v.decode(HTTP_HEADER_ENCODING)) for k, v in self.params.items()))

[raphendyr]

To add to my previous comment, my point is kind of fixed in default content negotiation class locally: https://github.com/tomchristie/django-rest-framework/blob/10a080d395b8f1fc5cef0778cd2a40c3d81dfe19/rest_framework/negotiation.py#L70

[carltongibson]

I'm going to de-milestone this for now. We can reassess after v3.7

[bitcity]

Came here searching for a similar issue. I'm using rest framework to return JSON response. If I use a unicode character in HTTP_ACCEPT header e.g. :

curl -H "Accept: */*, ♣" https://example.com/

I get a similar error. Here's the traceback:

Traceback (most recent call last):
File "./django/core/handlers/base.py", line 132, in get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "./django/views/decorators/csrf.py", line 58, in wrapped_view
return view_func(*args, **kwargs)
File "./django/views/generic/base.py", line 71, in view
return self.dispatch(request, *args, **kwargs)
File "./rest_framework/views.py", line 466, in dispatch
response = self.handle_exception(exc)
File "./rest_framework/views.py", line 454, in dispatch
self.initial(request, *args, **kwargs)
File "./rest_framework/views.py", line 381, in initial
neg = self.perform_content_negotiation(request)
File "./rest_framework/views.py", line 296, in perform_content_negotiation
return conneg.select_renderer(request, renderers, self.format_kwarg)
File "./rest_framework/negotiation.py", line 49, in select_renderer
accepts = self.get_accept_list(request)
File "./rest_framework/negotiation.py", line 97, in get_accept_list
return [token.strip() for token in header.split(',')]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 30: ordinal not in range(128)

[TheProdigyFilippo]

Have the same problem as above on ancient Django 1.7.1 with DRF 3.2.4 running on Python 2.7.12. Unfortunately didn't test on newer versions.

[bitcity]

Related : https://code.djangoproject.com/ticket/30737

Thanks for this report, however parse_header is a part of internal API that expects an ASCII-encoded bytes (see ​an example). You can raise an issue in DRF if you think that there is any in the way how they handle/use it.

Is it possible to override is_form_media_type in rest_framework/request.py with something like :

def is_form_media_type(media_type):
    try:
        base_media_type, params = parse_header(media_type.encode(HTTP_HEADER_ENCODING))
    except UnicodeDecodeError:
        # handle the error gracefully here

@raphendyr Can you share your fix please

[raphendyr]

@bitcity There were a hack here, but that was removed later when the code was rewrote to use ASCII values.

I guess if errors raise on media_type.encode(...), then invalid characters should be skipped or such. Maybe media_type.encode('latin1', 'ignore') would be ok here?

[peterthomassen]

I just observed users sending such invalid headers to our service, causing 500.

How about @bitcity's suggestion above?

[auvipy]

yup we can gracefully handle this for now

[peterthomassen]

What do you mean? - It seems like it isn't being handled gracefully at the moment.

[auvipy]

I meant, a PR to gracefully handle this is feasible for now

[peterthomassen]

heads-up: I looked into this, but don't feel competent to do a PR as I have not sufficiently comprehensive understanding of the flow and interplay of headers and charset settings etc. through Django and DRF. Would have liked to help -- sorry!

[auvipy]

@samims you can check this out