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

Do more complete normalization of URIs & headers #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

steven-collins-omega
Copy link

This is an attempt to implement more of AWS's canonicalization rules that are part of their signing algorithm. In particular (among other things) it addresses some specific problems I've run across:

  • /foo//bar (needs to be canonicalized to /foo/bar)
  • /_cat/shards?v (needs to be /_cat/shards?v=)
  • /_plugin/kibana/api/kibana/settings/discover:aggs:terms:size (needs to be /_plugin/kibana/api/kibana/settings/discover%3Aaggs%3Aterms%3Asize)

@@ -288,6 +300,7 @@ function getCanonicalQueryString(url) {
function getCanonicalHeaders(headers) {
var aggregatedHeaders = new Array();
for (var i=0; i<headers.length; i++) {
headers[i].value = headers[i].value.trim().replace(/ +/g, ' ');

Choose a reason for hiding this comment

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

This addresses AWS's rule that consecutive spaces in header values should be collapsed to one space.

@ghost
Copy link

ghost commented May 11, 2017

Thanks for the pull request - sorry I've broken it by merging an earlier one - please hold off on fixing it for now as I'm going to write some tests which may break it further.

@steven-collins-omega
Copy link
Author

Thanks @david-poirier-csn , keep me posted.

@steven-collins-omega
Copy link
Author

I need to make another change anyway; I just realized my canonicalization code leaves off the trailing slash in the request path if there was one, which AWS also does not like.

@steven-collins-omega
Copy link
Author

I hadn't actually looked at the other pull request; I just realized that these 2 PRs actually do literally conflict, as they deal with essentially the same issue. With all due respect, I think mine is more comprehensive and simpler.

@steven-collins-omega steven-collins-omega changed the title Do more complete normalization of URis & headers Do more complete normalization of URIs & headers Jul 21, 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.

1 participant