Conversation
…de errors" This reverts commit 1680416.
|
Would you mind explaining briefly the problem in the description of the PR? |
|
OK, I have summarized the test results of the repaired "replace headers" function. |
| } | ||
| } | ||
| let mut dest = self.inner.headers.clone(); | ||
| crate::util::replace_headers(&mut dest, std::mem::take(&mut headers)); |
There was a problem hiding this comment.
I think this changes the behavior (is that what you're trying to do?), because before this would prevent the default header from overwriting a header the user had set, by only checking if Vacant.
Hm, would this mean that if the default had several values for a key, only the first one is added, and the second iteration will see Occupied and not add it?
There was a problem hiding this comment.
I think this changes the behavior (is that what you're trying to do?), because before this would prevent the default header from overwriting a header the user had set, by only checking if
Vacant.Hm, would this mean that if the default had several values for a key, only the first one is added, and the second iteration will see
Occupiedand not add it?
I ran some tests, and it does indeed change the original behavior (it only adds support for handling multiple values under the same header key). Default headers are filled in only when the user hasn’t defined that header key.
Based on the test cases, if a default header contains multiple values for the same key, all of those values will be added. I achieved this by cleverly reusing util::replace_headers.
Line 50 in 74e6f84
There was a problem hiding this comment.
Hm, would this mean that if the default had several values for a key, only the first one is added, and the second iteration will see Occupied and not add it?
The behavior before the change was indeed like that.
Story Background
When sending multiple cookies, people often combine all cookies into a single header. But some servers might flag this as bot-like behavior, because browsers usually split multiple cookies into separate headers.
Solution
This PR fixes the issue by letting reqwest add headers using append instead of just insert (which would overwrite). Now, you can send requests with multiple cookie headers, just like browsers do. The change also keeps the original design logic.
Test Cases
Default Client Configuration
USER_AGENT: "default-agent",cookie: a=b, andcookie: c=dTest Branches and Request Conditions
/1BranchUSER_AGENT: "my-custom-agent"andcookie: a=b, cookie: c=dUSER_AGENTis"my-custom-agent"(overrides the default)a=bandc=d/2BranchUSER_AGENT: "my-custom-agent"andcookie: e=f, cookie: g=hUSER_AGENTis"my-custom-agent"(overrides the default)e=fandg=h(overrides the default)/3BranchUSER_AGENTorcookieUSER_AGENTis"default-agent"(uses the default)a=bandc=d(uses the default)/4BranchUSER_AGENT, but setscookie: e=f, cookie: g=hUSER_AGENTis"default-agent"(uses the default)e=fandg=h(overrides the default)Summary:
This test case checks how the client merges and overrides default headers. It makes sure:
USER_AGENT).