Skip to content

Commit

Permalink
Merge pull request #104 from r00ta/lp-2039105
Browse files Browse the repository at this point in the history
#104

Bugfix for https://bugs.launchpad.net/maas/+bug/2039105. 

The bug is that in case of retry, the oauth headers are duplicated in the requests and MAAS nginx reject the request with 400. The fix is actually to use `http.Header.Set` instead of `http.Header.Add` in order to set or replace the "Authentication" header.
  • Loading branch information
jujubot authored Oct 12, 2023
2 parents 7b1b159 + 06c9fb3 commit 39eac57
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
20 changes: 20 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,26 @@ func (suite *ClientSuite) TestClientDispatchRequestRetries409(c *gc.C) {
c.Check(*server.nbRequests, gc.Equals, NumberOfRetries+1)
}

// See https://bugs.launchpad.net/maas/+bug/2039105
func (suite *ClientSuite) TestClientDispatchRequestRetries409WithoutDuplicatedHeaders(c *gc.C) {
URI := "/some/url/?param1=test"
server := newFlakyServer(URI, 409, NumberOfRetries)
defer server.Close()
client, err := NewAuthenticatedClient(server.URL, "the:api:key")
c.Assert(err, jc.ErrorIsNil)
content := "content"
request, err := http.NewRequest("GET", server.URL+URI, io.NopCloser(strings.NewReader(content)))
c.Assert(err, jc.ErrorIsNil)

_, err = client.dispatchRequest(request)

c.Assert(err, jc.ErrorIsNil)
c.Check(*server.nbRequests, gc.Equals, NumberOfRetries+1)
for _, headers := range *server.headers {
c.Check(len(headers.Values("Authorization")), gc.Equals, 1)
}
}

func (suite *ClientSuite) TestTLSClientDispatchRequestRetries503NilBody(c *gc.C) {
URI := "/some/path"
server := newFlakyTLSServer(URI, 503, NumberOfRetries)
Expand Down
2 changes: 1 addition & 1 deletion oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,6 @@ func (signer plainTextOAuthSigner) OAuthSign(request *http.Request) error {
authHeader = append(authHeader, fmt.Sprintf(`%s="%s"`, key, url.QueryEscape(value)))
}
strHeader := "OAuth " + strings.Join(authHeader, ", ")
request.Header.Add("Authorization", strHeader)
request.Header.Set("Authorization", strHeader)
return nil
}
9 changes: 7 additions & 2 deletions testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,23 @@ type flakyServer struct {
*httptest.Server
nbRequests *int
requests *[][]byte
headers *[]http.Header
}

// newFlakyServer creates a "flaky" test http server which will
// return `nbFlakyResponses` responses with the given code and then a 200 response.
func newFlakyServer(uri string, code int, nbFlakyResponses int) *flakyServer {
nbRequests := 0
requests := make([][]byte, nbFlakyResponses+1)
headers := make([]http.Header, nbFlakyResponses+1)
handler := func(writer http.ResponseWriter, request *http.Request) {
nbRequests += 1
body, err := readAndClose(request.Body)
if err != nil {
panic(err)
}
requests[nbRequests-1] = body
headers[nbRequests-1] = request.Header
if request.URL.String() != uri {
errorMsg := fmt.Sprintf("Error 404: page not found (expected '%v', got '%v').", uri, request.URL.String())
http.Error(writer, errorMsg, http.StatusNotFound)
Expand All @@ -81,12 +84,13 @@ func newFlakyServer(uri string, code int, nbFlakyResponses int) *flakyServer {

}
server := httptest.NewServer(http.HandlerFunc(handler))
return &flakyServer{server, &nbRequests, &requests}
return &flakyServer{server, &nbRequests, &requests, &headers}
}

func newFlakyTLSServer(uri string, code int, nbFlakyResponses int) *flakyServer {
nbRequests := 0
requests := make([][]byte, nbFlakyResponses+1)
headers := make([]http.Header, nbFlakyResponses+1)
var server *httptest.Server

handler := func(writer http.ResponseWriter, request *http.Request) {
Expand All @@ -96,6 +100,7 @@ func newFlakyTLSServer(uri string, code int, nbFlakyResponses int) *flakyServer
panic(err)
}
requests[nbRequests-1] = body
headers[nbRequests-1] = request.Header
if request.URL.String() != uri {
errorMsg := fmt.Sprintf("Error 404: page not found (expected '%v', got '%v').", uri, request.URL.String())
http.Error(writer, errorMsg, http.StatusNotFound)
Expand All @@ -111,7 +116,7 @@ func newFlakyTLSServer(uri string, code int, nbFlakyResponses int) *flakyServer
}
}
server = httptest.NewTLSServer(http.HandlerFunc(handler))
return &flakyServer{server, &nbRequests, &requests}
return &flakyServer{server, &nbRequests, &requests, &headers}
}

type simpleResponse struct {
Expand Down

0 comments on commit 39eac57

Please sign in to comment.