Need encodeURIComponent instead of encodeURI

[satb]

We need to use encodeURIComponent instead of encodeURI to account for special characters like '+' in the file name. This is especially so for those that directly upload to s3 from the browser where they have no control over filenames

[domenic]

I am almost certain this was changed to encodeURI in the past in order to fix another bug. Have you ensured that all the tests pass with your change?

[domenic]

Also, I will not merge this without a new test, which must fail previously, and succeed afterward.

[satb]

No I haven't run the tests - it just made more sense that encodeURIComponent is the right thing to use. encodeURIComponent is used to encode the filename and special characters (other than single quote) is taken care of. Try it out with some special characters (say, a + in the file name) in the file name it will most certainly fail with encodeURI. If there was another bug that caused the change to encodeURI, I'll try to run the tests to see if everything passes.

[domenic]

Amazon's encoding rules are not normal URL encoding rules.

[satb]

You may be right that Amazon has some exceptions, but encodeURI will just not work in some conditions. As I said, simply upload a file with a special character in it directly to S3 and try with encodeURI - it will fail. So encodeURI is not the right thing in all cases, yet I cannot be sure if encodeURIComponent is the right thing either that covers every case. Unless I know all of Amazon's rules, I can't be sure about what the exception cases are. Do you know of any documentation that tells what Amazon encoding won't work with standard encoding rules?

[domenic]

Please produce a failing test. I am not convinced by your theoretical concerns.

[domenic]

Do you know of any documentation that tells what Amazon encoding won't work with standard encoding rules?

Yes, the Amazon AWS documentation discusses the rules at length, and we implement them---using encodeURI.

[satb]

The pull request is a direct result of a test that failed in production for us :). So you can rest assured its not "theoretical"

[domenic]

I am very sorry for forgetting about this and not merging it.

Can you rebase your commits so that (a) they make no formatting changes at all, and in general don't change areas of Knox that are unrelated to the relevant bug; and (b) there is only one of them?