Allow the Request to override the Authorization header set by the Client #121
Conversation
Codecov Report
@@ Coverage Diff @@
## master #121 +/- ##
==========================================
+ Coverage 96.32% 96.32% +<.01%
==========================================
Files 10 10
Lines 1033 1035 +2
==========================================
+ Hits 995 997 +2
Misses 21 21
Partials 17 17
Continue to review full report at Codecov.
|
|
@arun251 Thank you for the PR. I will verify this behaviour between v0.13 and latest. I good to ensure any other singleton header also is not affected. |
|
@arun251 If there is an issue with singleton headers, that needs to be fixed across not just Authorization. Thanks for your findings. I will analyze and get back to you. |
| @@ -472,3 +472,20 @@ func TestSetLogPrefix(t *testing.T) { | |||
| assertEqual(t, "CUSTOM ", c.logPrefix) | |||
| assertEqual(t, "CUSTOM ", c.Log.Prefix()) | |||
| } | |||
|
|
|||
| func TestRequestOverridesClientAuthorizationHeader(t *testing.T) { | |||
There was a problem hiding this comment.
Please move this test case to request_test.go file.
| @@ -76,6 +76,11 @@ func parseRequestHeader(c *Client, r *Request) error { | |||
| hdr[k] = append(hdr[k], r.Header[k]...) | |||
| } | |||
|
|
|||
| // Allow the Request to override the Authorization header set by the Client | |||
There was a problem hiding this comment.
I have analyzed the v0.13 and latest version. Fix for across headers.
Remove this if condition and update your PR as follows.
Update Line #75 to #77 to following-
for k := range r.Header {
// remove header from client level by key
// since overrides happens for that key in the request
hdr.Del(k)
hdr[k] = append(hdr[k], r.Header[k]...)
}
|
@arun251 Awaiting for your PR changes or let me know I will do it. |
|
Please go ahead with the changes. Thanks! |
|
@arun251 Thank you, I will take your test case and create new PR. |
We have code that sets the
Authorizationheader on theClientobject and then overrides the header on theRequestobject. That previously worked with Resty v0.13 but no longer worked after upgrading to Resty v1.2. The root cause of this issue is inparseRequestHeaderwhere client and request headers are concatenated.This PR fixes the issue for
Authorization, but I suspect there are other singleton headers that could be affected.