tool_cb_hdr: with -J, use the redirect name as a backup - #20430
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances the -J/--remote-header-name option to use the filename from redirect Location headers as a fallback when no Content-Disposition header is provided. This improves user experience by providing a more meaningful output filename based on the redirect path when the server doesn't explicitly set a filename via Content-Disposition.
Changes:
- Modified
content_disposition()function to parse and store filenames from Location headers as a temporary measure - Added test1641 to verify Content-Disposition takes precedence over Location header filename
- Added test1642 to verify Location header filename is used when Content-Disposition is absent
- Updated documentation to describe the new fallback behavior starting in version 8.19.0
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tool_cb_hdr.c | Added Location header parsing logic to extract and store filename as fallback before Content-Disposition processing |
| tests/data/test1641 | Test case verifying Content-Disposition filename is used when present after redirect |
| tests/data/test1642 | Test case verifying Location header filename is used when Content-Disposition is absent |
| tests/data/Makefile.am | Added test1641 to the test file list |
| docs/cmdline-opts/remote-header-name.md | Documented the new fallback behavior for version 8.19.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
augment review |
|
Ref #1241 |
🤖 Augment PR SummarySummary: Improve Changes:
Technical Notes: The redirect fallback uses the last 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The -J / --remote-header-name logic now records the file name part used in the redirects so that it can use the last one as a name if no Content-Disposition header arrives. Add tests to verify: 1641: -J with a redirect and extract the CD contents in the second response 1642: -J with a redirect but no Content-Disposition, use the name from the Location: header 1643: -J with two redirects, using the last file name and also use queries and fragments to verify them stripped off Closes #20430
f73bf77 to
58f2529
Compare
|
augment review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The -J / --remote-header-name logic now records the file name part used in the redirects so that it can use the last one as a name if no Content-Disposition header arrives. Add tests to verify: 1641: -J with a redirect and extract the CD contents in the second response 1642: -J with a redirect but no Content-Disposition, use the name from the Location: header 1643: -J with two redirects, using the last file name and also use queries and fragments to verify them stripped off Closes #20430
f526c8e to
bb7ec56
Compare
This comment was marked as resolved.
This comment was marked as resolved.
🔒 Aisle Security Analysis✅ We scanned this PR and did not find any security vulnerabilities. Analyzed PR: #20430 at commit |
The -J / --remote-header-name logic now records the file name part used in the redirects so that it can use the last one as a name if no Content-Disposition header arrives.
Add three tests to verify.