fix: restore URL buffer size to 2048 bytes#475
Conversation
The zero-copy refactor (879acbc) inadvertently halved HTTP_URL_BUFFER_SIZE from 2048 to 1024 when moving it from connection.h to service.h. This causes truncation for users with long upstream URLs containing many query parameters. Restore all pipeline-synchronized URL buffer constants to 2048 and adjust the merged-URL overflow test accordingly.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://thankful-water-0a297bf00-475.eastasia.1.azurestaticapps.net |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 408c992504
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Restores the URL buffer ceilings across the HTTP→service→RTSP pipeline back to 2048 bytes (matching the pre–zero-copy refactor behavior) and updates the e2e coverage to validate merged-URL overflow behavior at the new limit.
Changes:
- Bump
HTTP_URL_BUFFER_SIZEand RTSP URL/path/copy buffer sizes from 1024 → 2048. - Increase
http_request_t.urlstorage from 1024 → 2048. - Adjust the merged-URL overflow e2e test padding so it still reliably exceeds the ceiling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/service.h | Restores HTTP_URL_BUFFER_SIZE to 2048 to raise the service-layer URL working buffer ceiling. |
| src/rtsp.h | Restores RTSP URL/path/copy buffer sizes to 2048 to keep the RTSP layer aligned with the service-layer ceiling. |
| src/http.h | Expands request URL storage and updates API documentation for URL parsing expectations. |
| e2e/test_m3u.py | Updates overflow test padding so the merged URL reliably exceeds the restored 2048-byte ceiling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move HTTP_URL_BUFFER_SIZE definition from service.h to http.h so the macro is the single source of truth; use it for http_request_t.url instead of a hard-coded literal. - Raise config file MAX_LINE from 1024 to 4096 so inline M3U URLs up to 2048 bytes are not silently truncated by fgets. - Fix http_parse_url_components path copy limit from 1023 to 2047 to match the restored 2048-byte ceiling.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://thankful-water-0a297bf00-475.eastasia.1.azurestaticapps.net |
Summary
HTTP_URL_BUFFER_SIZE,RTSP_SERVER_URL_SIZE,RTSP_SERVER_PATH_SIZE,RTSP_URL_COPY_SIZE, andhttp_request_t.urlfrom 1024 back to 2048 bytes — the original value before the zero-copy refactor (879acbc) inadvertently halved them.HTTP_URL_BUFFER_SIZEdefinition fromservice.htohttp.has the single source of truth;http_request_t.urluses the macro instead of a hard-coded literal.MAX_LINEfrom 1024 to 4096 so inline M3U URLs up to 2048 bytes are not silently truncated byfgets.http_parse_url_componentspath copy limit from 1023 to 2047 to match the restored 2048-byte ceiling.Test plan
cmake --build buildcompiles cleanlytest_error.py— 17 passed (includingtest_very_long_url)test_m3u.py— 33 passed (includingtest_merged_url_overflow_returns_500)test_url_template.py— 32 passedtest_rtsp_seek_mode.py— 32 passed (including r2h-token stripping)test_http_proxy.py— 18 passed🤖 Generated with Claude Code