fix(http-proxy): drain large m3u rewrite bodies#488
Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://thankful-water-0a297bf00-488.eastasia.1.azurestaticapps.net |
There was a problem hiding this comment.
Pull request overview
This PR aims to fix HTTP proxy M3U rewrite buffering so large playlists continue draining upstream reads under edge-triggered polling.
Changes:
- Reports buffered M3U body reads as progress during rewrite mode.
- Reports initial body bytes received with headers as progress.
- Adds an E2E regression test for large playlist rewrite buffering.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/http_proxy.c |
Adjusts M3U rewrite receive progress signaling. |
e2e/test_http_proxy_m3u_rewrite.py |
Adds a large playlist rewrite regression test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return http_proxy_finalize_rewrite(session); | ||
| } | ||
|
|
||
| bytes_forwarded = (int)initial_size; |
There was a problem hiding this comment.
Addressed by the second commit (f5b918d) which added socket_progress fallback — the header-only case now returns socket_progress (the full recv byte count) instead of 0, so the drain loop stays alive. Additionally updated the doc comment and renamed the accumulator to progress in 5a1a0d9 to reflect the actual semantics.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://thankful-water-0a297bf00-488.eastasia.1.azurestaticapps.net |
| } | ||
|
|
||
| return bytes_forwarded; | ||
| return bytes_forwarded > 0 ? bytes_forwarded : socket_progress; |
There was a problem hiding this comment.
Went with updating the contract — renamed the accumulator to progress and updated the doc to say "positive value if progress was made (data received/forwarded)". No caller uses the positive value semantically, so separating the signals would be unnecessary complexity.
| } | ||
|
|
||
| return 0; /* Keep buffering */ | ||
| return (int)received; /* Progress: keep draining edge-triggered sockets */ |
There was a problem hiding this comment.
Updated the doc comment to say "positive value if progress was made" instead of "bytes forwarded", and renamed the accumulator in http_proxy_handle_socket_event from bytes_forwarded to progress. The only caller (stream.c) only checks < 0, so no functional change.
…andler The M3U rewrite drain fix changed the return semantics from "bytes forwarded to client" to "progress made" (including buffered-but-not-yet- forwarded bytes). Update the doc comment and rename the accumulator variable to reflect this.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://thankful-water-0a297bf00-488.eastasia.1.azurestaticapps.net |
Summary
Fix HTTP proxy M3U rewrite buffering so large playlists keep draining upstream reads under edge-triggered polling.
Related: #486
Root Cause
The M3U rewrite path buffered upstream body bytes but returned 0 before the body was complete. The outer read loop treats 0 as no progress, so it could stop before reaching EAGAIN and miss remaining buffered bytes.
Validation