Skip to content

Conversation

patchback[bot]
Copy link
Contributor

@patchback patchback bot commented Jul 10, 2025

This is a backport of PR #11290 as merged into master (16703bb).

What do these changes do?

These changes fix a bug where file uploads fail with HTTP 422 errors when encountering 307/308 redirects. The issue occurs because:

  1. HTTP 307/308 redirects preserve the request body (unlike 301/302/303)
  2. When aiohttp reuses the same file payload for the redirect, the file position is at EOF after the first request
  3. This causes the Content-Length calculation to return 0 for the second request, leading to server rejection

The fix includes:

  • Client change: Preserve the payload instance (not raw data) for 307/308 redirects to prevent recreation of consumed payloads
  • Payload change: Store initial file position on first access and calculate size from that position, ensuring correct Content-Length for reused payloads

Are there changes in behavior for the user?

File uploads that encounter 307/308 redirects will now work correctly instead of failing with HTTP 422 errors. This is a bug fix that restores expected behavior - no breaking changes.

Is it a substantial burden for the maintainers to support this?

No. The changes are minimal and focused:

  • Simple logic to preserve payload instances for specific redirect types
  • File position management that's already part of the payload lifecycle
  • Well-tested with both unit and integration tests
  • No new APIs or complex abstractions

Related issue number

Fixes #11270

Implementation Details

Changes Made

  1. aiohttp/client.py

    • Modified redirect handling to preserve payload instance for 307/308 redirects
    • Changed from data = req.data to data = req._body to avoid payload recreation
  2. aiohttp/payload.py

    • Modified size property to store start position on first access
    • Returns total size from start position instead of remaining bytes
    • Added AttributeError handling for tarfile streams
    • Properly handles unseekable files by returning None for size
  3. tests/test_payload.py

    • Added test for payload size calculation after reading
    • Added test for unseekable file handling
  4. tests/test_client_functional.py

    • Added integration test for file upload with 307 redirect

Technical Explanation

The root cause identified by Cycloctane in the issue: "Two requests share the same BufferedReaderPayload instance, while the second request get zero Content-Length."

This happens because:

  1. First request reads the file, moving position to EOF
  2. For 307/308 redirects, the same payload is reused
  3. Size calculation was file_size - current_position, which equals 0 at EOF
  4. Server receives Content-Length: 0 and returns 422 Unprocessable Entity

The fix stores the initial file position when size is first accessed, then always calculates size from that position. This ensures consistent Content-Length across multiple uses of the same payload instance.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes (no user-facing API changes)
  • Add yourself to CONTRIBUTORS.txt
  • Add a new news fragment 11270.bugfix.rst into the CHANGES/ folder

Copy link

codspeed-hq bot commented Jul 10, 2025

CodSpeed Performance Report

Merging #11297 will not alter performance

Comparing patchback/backports/3.13/16703bb955ae4a11a131cedbbbf3ec7aa55f4bb4/pr-11290 (a136d98) with 3.13 (8790eb0)

🎉 Hooray! pytest-codspeed just leveled up to 4.0.0!

A heads-up, this is a breaking change and it might affect your current performance baseline a bit. But here's the exciting part - it's packed with new, cool features and promises improved result stability 🥳!
Curious about what's new? Visit our releases page to delve into all the awesome details about this new version.

Summary

✅ 59 untouched benchmarks

Copy link

codecov bot commented Jul 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.29%. Comparing base (8790eb0) to head (a136d98).
Report is 2 commits behind head on 3.13.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##             3.13   #11297    +/-   ##
========================================
  Coverage   98.29%   98.29%            
========================================
  Files         132      132            
  Lines       43603    43751   +148     
  Branches     2387     2392     +5     
========================================
+ Hits        42858    43006   +148     
  Misses        565      565            
  Partials      180      180            
Flag Coverage Δ
CI-GHA 98.19% <100.00%> (+<0.01%) ⬆️
OS-Linux 97.94% <100.00%> (+<0.01%) ⬆️
OS-Windows 95.64% <100.00%> (+<0.01%) ⬆️
OS-macOS 97.18% <100.00%> (+<0.01%) ⬆️
Py-3.10.11 96.82% <100.00%> (+0.01%) ⬆️
Py-3.10.18 97.31% <100.00%> (+<0.01%) ⬆️
Py-3.11.13 97.51% <100.00%> (+<0.01%) ⬆️
Py-3.11.9 97.02% <100.00%> (+0.01%) ⬆️
Py-3.12.10 97.11% <100.00%> (+<0.01%) ⬆️
Py-3.12.11 97.60% <100.00%> (+<0.01%) ⬆️
Py-3.13.3 97.83% <100.00%> (+<0.01%) ⬆️
Py-3.9.13 96.72% <100.00%> (+0.01%) ⬆️
Py-3.9.23 97.20% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.16 89.41% <100.00%> (+0.92%) ⬆️
VM-macos 97.18% <100.00%> (+<0.01%) ⬆️
VM-ubuntu 97.94% <100.00%> (+<0.01%) ⬆️
VM-windows 95.64% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Co-authored-by: Michael Berdyshev <mberdyshev@list.ru>
@Dreamsorcerer Dreamsorcerer merged commit ffb9a33 into 3.13 Jul 10, 2025
35 of 36 checks passed
@Dreamsorcerer Dreamsorcerer deleted the patchback/backports/3.13/16703bb955ae4a11a131cedbbbf3ec7aa55f4bb4/pr-11290 branch July 10, 2025 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants