[server] Unhandled exception when None in header values

[panagiks]

Long story short

When a dictionary is passed to the headers kwarg (in any response, either returned or raised) and a value in this dictionary is None, an unhandled exception is raised that results in the client getting no response at all.

Expected behavior

The expected behavior would be to handle it in the same way an empty string header is handled (i.e. discard it) or to catch the exception so that a 500 error can be returned.

Actual behavior

No response arrives at the client.

Steps to reproduce

import asyncio

from aiohttp import web


def view_func(request):
    raise web.HTTPUnauthorized(headers={'a':''})


def view_raise_fail(request):
    raise web.HTTPUnauthorized(headers={'a': None})


def view_return_fail(request):
    return web.Response(status=200, headers={'a': None})


def get_app():
    app = web.Application()
    app.router.add_route('GET', '/', view_func)
    app.router.add_route('GET', '/raise/fail/', view_raise_fail)
    app.router.add_route('GET', '/return/fail/', view_return_fail)
    return app


if __name__ == '__main__':
    loop = asyncio.get_event_loop()
    app = get_app()
    web.run_app(app, port=8080, host='127.0.0.1')

wget http://127.0.0.1:8080/
--2018-02-16 08:47:40--  http://127.0.0.1:8080/
Connecting to 127.0.0.1:8080... connected.
HTTP request sent, awaiting response... 401 Unauthorized

wget http://127.0.0.1:8080/raise/fail/
--2018-02-16 08:48:31--  http://127.0.0.1:8080/raise/fail/
Connecting to 127.0.0.1:8080... connected.
HTTP request sent, awaiting response... No data received.
Retrying.

wget http://127.0.0.1:8080/return/fail/
--2018-02-16 08:48:43--  http://127.0.0.1:8080/return/fail/
Connecting to 127.0.0.1:8080... connected.
HTTP request sent, awaiting response... No data received.
Retrying.

The Traceback:

Unhandled exception
Traceback (most recent call last):
  File "<PATH>/lib/python3.6/site-packages/aiohttp/web_protocol.py", line 396, in start
    await resp.prepare(request)
  File "<PATH>/lib/python3.6/site-packages/aiohttp/web_response.py", line 300, in prepare
    return self._start(request)
  File "<PATH>/lib/python3.6/site-packages/aiohttp/web_response.py", line 605, in _start
    return super()._start(request)
  File "<PATH>/lib/python3.6/site-packages/aiohttp/web_response.py", line 367, in _start
    writer.write_headers(status_line, headers)
  File "<PATH>/lib/python3.6/site-packages/aiohttp/http_writer.py", line 98, in write_headers
    [k + SEP + v + END for k, v in headers.items()])
  File "<PATH>/lib/python3.6/site-packages/aiohttp/http_writer.py", line 98, in <listcomp>
    [k + SEP + v + END for k, v in headers.items()])
TypeError: must be str, not NoneType

Your environment

OS version : Linux deb9 4.9.0-3-amd64 #1 SMP Debian 4.9.30-2+deb9u5 (2017-09-19) x86_64 GNU/Linux

Python version : 3.6.4

aiohttp server version: 3.0.1

[panagiks]

I could open a PR to add a check inside the list comprehension (with tests) if you agree.

[webknjaz]

For json response proper fix is likely to be a custom JSONEncoder, which understands that None must be turned into null.

[panagiks]

The problem is not on the JSONEncoder it's on k + SEP + v + END where string concatenation fails if v is None.

[webknjaz]

Oh, I misread the second handler. In this case, it should be 500 Internal Server Error on the HTTP layer + maybe better exception raised from header conversion part

[asvetlov]

A bad type for header value should produce 500 with exception info anyway.
Changing exception type doesnt improve situation a lot: user should get 500 Internal Server Error

[panagiks]

Ok, since there seems to be a consensus that the server shouldn't handle it silently I'll look into adding a 500 Error for this case during the weekend :)

[webknjaz]

Andrew, I think raising exception is for developer (and ops/qa) anyway :) Not sure why you mentioned web-app's end-user. End-user of the framework is a developer and good UX for them is comprehensive exception + error message.

[asvetlov]

We had a bug in http parser that sometimes emitted None. Fixed