feat: add to_dict() and get_headers() methods to Headers#1376
feat: add to_dict() and get_headers() methods to Headers#1376
Conversation
Headers.to_dict() returns a flat dict with comma-joined values for duplicate headers. Headers.get_headers() returns a dict of lists preserving all values per header name. Supersedes #1184 Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughTwo new accessor methods are added to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/types/headers.rs`:
- Around line 101-105: to_dict() currently unconditionally joins multiple header
values with commas (joined_value), which corrupts non-combinable headers like
"set-cookie"; change the construction so that when the header name is in a small
constant set (e.g., NON_COMBINABLE_HEADERS containing "set-cookie",
"set-cookie2", etc.) you do NOT comma-merge but instead preserve a single value
(e.g., values[0].clone()) or otherwise avoid joining, and leave a note that
callers should use get_headers() for full-fidelity multi-value headers; add the
NON_COMBINABLE_HEADERS constant and use it inside to_dict() where joined_value
is computed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b2e2577-dcac-4021-a3e5-f825944dbc5e
📒 Files selected for processing (3)
robyn/robyn.pyisrc/types/headers.rsunit_tests/test_request_object.py
| let joined_value = if values.len() == 1 { | ||
| values[0].clone() | ||
| } else { | ||
| values.join(",") | ||
| }; |
There was a problem hiding this comment.
to_dict() should not comma-merge non-combinable headers (e.g., set-cookie).
At Line 104, duplicate values are always joined with commas. That is not valid for all HTTP headers and can corrupt semantics for headers like set-cookie. Prefer special-casing non-combinable names (or documenting to_dict() as lossy and directing callers to get_headers() for full fidelity).
Suggested adjustment
pub fn to_dict(&self, py: Python) -> Py<PyDict> {
let dict = PyDict::new(py);
for iter in self.headers.iter() {
let (key, values) = iter.pair();
- let joined_value = if values.len() == 1 {
+ let joined_value = if values.len() == 1 {
values[0].clone()
+ } else if key.eq_ignore_ascii_case("set-cookie") {
+ // Non-combinable header; keep a deterministic single value in flattened view.
+ // Callers needing all values should use `get_headers()`.
+ values.last().cloned().unwrap_or_default()
} else {
values.join(",")
};
dict.set_item(key, joined_value).unwrap();
}
dict.into()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let joined_value = if values.len() == 1 { | |
| values[0].clone() | |
| } else { | |
| values.join(",") | |
| }; | |
| pub fn to_dict(&self, py: Python) -> Py<PyDict> { | |
| let dict = PyDict::new(py); | |
| for iter in self.headers.iter() { | |
| let (key, values) = iter.pair(); | |
| let joined_value = if values.len() == 1 { | |
| values[0].clone() | |
| } else if key.eq_ignore_ascii_case("set-cookie") { | |
| // Non-combinable header; keep a deterministic single value in flattened view. | |
| // Callers needing all values should use `get_headers()`. | |
| values.last().cloned().unwrap_or_default() | |
| } else { | |
| values.join(",") | |
| }; | |
| dict.set_item(key, joined_value).unwrap(); | |
| } | |
| dict.into() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/headers.rs` around lines 101 - 105, to_dict() currently
unconditionally joins multiple header values with commas (joined_value), which
corrupts non-combinable headers like "set-cookie"; change the construction so
that when the header name is in a small constant set (e.g.,
NON_COMBINABLE_HEADERS containing "set-cookie", "set-cookie2", etc.) you do NOT
comma-merge but instead preserve a single value (e.g., values[0].clone()) or
otherwise avoid joining, and leave a note that callers should use get_headers()
for full-fidelity multi-value headers; add the NON_COMBINABLE_HEADERS constant
and use it inside to_dict() where joined_value is computed.
There was a problem hiding this comment.
Resolve — by design. to_dict() is a convenience flattened view. get_headers() preserves full multi-value fidelity for headers like set-cookie.
There was a problem hiding this comment.
Rate Limit Exceeded
@sansyrox have exceeded the limit for the number of chat messages per hour. Please wait 2 minutes and 1 seconds before sending another message.
Summary
Headers.to_dict()which returns a flatdict[str, str]with comma-joined values for duplicate headers, matching standard HTTP semantics.to_dict()andget_headers()inrobyn.pyi.get_headers()list preservation.Supersedes #1184
Test plan
test_headers_to_dict— basic single-value headerstest_headers_to_dict_with_duplicates— comma-joining of multi-value headerstest_headers_get_headers— list preservation viaget_headers()test_headers_to_dict_empty— empty headers edge caseMade with Cursor
Summary by CodeRabbit
New Features
get_headers()method to retrieve all headers grouped by name with values as lists.to_dict()method to retrieve headers as a flattened dictionary, with multiple values concatenated by commas.Tests