Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES/12948.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed ``IndexError: string index out of range`` in ``parse_content_disposition``
when a header parameter has an empty value (e.g. ``filename=``).
-- by :user:`JSap0914`.
6 changes: 6 additions & 0 deletions CHANGES/12953.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Fixed the ``sock_read`` timeout being re-armed on a keep-alive connection after
it had been returned to the pool. An idle pooled connection could be left with a
pending read timeout that fired and poisoned it, so the next request reusing the
connection failed immediately with :exc:`aiohttp.SocketTimeoutError`. The read
timeout is now only rescheduled when resuming a transport that was actually
paused -- by :user:`daragok`.
1 change: 1 addition & 0 deletions CHANGES/12954.bugfix.rst
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ Sebastian Hanula
Sebastian Hüther
Sebastien Geffroy
SeongSoo Cho
Sergei Grachev
Sergey Ninua
Sergey Skripnick
Serhii Charykov
Expand Down
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ global-exclude *.lib
global-exclude *.dll
global-exclude *.a
global-exclude *.obj
global-exclude *.hash
exclude aiohttp/*.html
prune docs/_build
4 changes: 3 additions & 1 deletion aiohttp/client_proto.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,10 @@ def pause_reading(self) -> None:
self._drop_timeout()

def resume_reading(self, resume_parser: bool = True) -> None:
was_paused = self._reading_paused
super().resume_reading(resume_parser)
self._reschedule_timeout()
if was_paused:
self._reschedule_timeout()

def set_exception(
self,
Expand Down
2 changes: 1 addition & 1 deletion aiohttp/multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def is_token(string: str) -> bool:
return bool(string) and TOKEN >= set(string)

def is_quoted(string: str) -> bool:
return string[0] == string[-1] == '"'
return len(string) >= 2 and string[0] == string[-1] == '"'

def is_rfc5987(string: str) -> bool:
return is_token(string) and string.count("'") == 2
Expand Down
37 changes: 37 additions & 0 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,43 @@ async def handler(request: web.Request) -> web.Response:
assert result == b"foo"


async def test_sock_read_timeout_not_rearmed_on_pooled_connection(
aiohttp_client: AiohttpClient,
) -> None:
# Reading the buffered body of a completed response must not re-arm the
# sock_read timeout on a connection that has already been released to the
# keep-alive pool. Otherwise the timer fires while the connection sits idle
# in the pool, stamps SocketTimeoutError on it, and the next request that
# reuses it fails immediately (with no real read having stalled).
async def handler(request: web.Request) -> web.Response:
return web.json_response({"ok": True})

app = web.Application()
app.router.add_get("/", handler)

timeout = aiohttp.ClientTimeout(total=30, sock_read=0.1)
client = await aiohttp_client(app, timeout=timeout)

async with client.get("/") as resp:
assert resp.status == 200
await resp.read()

assert client.session.connector is not None
pooled = next(iter(client.session.connector._conns.values()))
proto = pooled[0][0]
# The pooled connection must carry no read-timeout handle, otherwise
# it could trigger an exception on the next request.
assert proto._read_timeout_handle is None
assert proto.exception() is None

# The connection is still reusable.
async with client.get("/") as resp:
assert resp.status == 200
assert await resp.json() == {"ok": True}

assert next(iter(client.session.connector._conns.values()))[0][0] is proto


async def test_request_exception_cleanup_with_no_total_timeout(
aiohttp_client: AiohttpClient,
) -> None:
Expand Down
16 changes: 16 additions & 0 deletions tests/test_multipart_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,22 @@ def test_bad_continuous_param(self) -> None:
assert "attachment" == disptype
assert {} == params

def test_empty_param_value_no_crash(self) -> None:
"""Empty param value (e.g. filename=) must not raise IndexError."""
with pytest.warns(aiohttp.BadContentDispositionHeader):
disptype, params = parse_content_disposition("attachment; filename=")
assert disptype is None
assert {} == params

def test_empty_param_value_multiple(self) -> None:
"""Multiple params where one has empty value must not raise IndexError."""
with pytest.warns(aiohttp.BadContentDispositionHeader):
disptype, params = parse_content_disposition(
"attachment; name=foo; filename="
)
assert disptype is None
assert {} == params


class TestContentDispositionFilename:
# http://greenbytes.de/tech/tc2231/
Expand Down
Loading