Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve sts header parsing #3013

Merged
merged 3 commits into from
Jul 18, 2017

Conversation

joelthompson
Copy link
Contributor

I'm not sure if this is the best way to do so in go, so please let me know if there's a better way of accomplishing this!

Fixes #2810

@@ -1407,7 +1406,38 @@ func parseGetCallerIdentityResponse(response string) (GetCallerIdentityResponse,
return result, err
}

func submitCallerIdentityRequest(method, endpoint string, parsedUrl *url.URL, body string, headers http.Header) (*GetCallerIdentityResult, error) {
func parseIamRequestHeaders(headersB64 string) (*http.Header, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http.Header is a typedef for map[string][]string and as such it's a reference type -- return it directly, not as a pointer.

}
}
default:
return nil, fmt.Errorf("Header %q value %q has type %s, not string or []interface", k, typedValue, reflect.TypeOf(v))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Header/header -- same above

Copy link
Member

@jefferai jefferai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except for the header being a pointer. Unfortunately fixing this will require backing out half the changes you made in the first place...sorry!

@jefferai jefferai added this to the 0.7.4 milestone Jul 17, 2017
This refactors the parsing of iam_request_headers parameter into a
separate method to make it easy to add unit tests around the parsing, in
preparation for relaxing the header requirements (issue 2810).
This allows headers to be of both map[string][]string (which is
http.Header -- the existing format) and map[string]string to be more
interoperable with other languages and SDKs.
@joelthompson joelthompson force-pushed the improve_sts_header_parsing branch from f7c1389 to 03fdfb2 Compare July 18, 2017 00:28
@joelthompson
Copy link
Contributor Author

Done.

Unfortunately fixing this will require backing out half the changes you made in the first place...sorry!

No, thank you, I learned something new about go so it's great! :)

@jefferai
Copy link
Member

Looks good -- thanks!

@jefferai jefferai merged commit 88910d0 into hashicorp:master Jul 18, 2017
@joelthompson joelthompson deleted the improve_sts_header_parsing branch July 18, 2017 15:23
@jefferai jefferai modified the milestones: 0.7.4, 0.8.0 Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants