Full line coverage for AwsV4Signer and also make query parameters single (valid URL) encoded

[sirknightj]

What was changed, why was it changed, and how was it changed?

1. Added comments/explanations for methods of AwsSigv4Signer.java.
2. Add additional unit tests for AwsSigv4Signer.java. The code is functioning fine, however, unit tests can help others when debugging or when attempting to port over the Java code to another programming language and checking that it was ported over correctly. Adding unit tests for each method would essentially give a predefined input along with its expected output. This would make it easier for others to see which methods were returning the wrong output. The newly added unit tests push this class to have 100% line coverage. The previous unit test methods tested several units in each method. This is helpful to see which "group of methods" is not performing correctly, however, it could be improved by narrowing down the issue down to the individual method. Now, each method has at least a single dedicated test, to make it easier to see which "unit" may be malfunctioning.

Time	Photo
Previous
Now

4. In order to test the signer properly, we need to move the dateMilli provider (i.e. new Date().getTime()) from inside of the sign method, to become a parameter for testability.
5. Add an overload for the sign() method. Since wssUri will always be the first part of uri, it's just passing in a redundant extra parameter.

URL Encoding

We now single url-encode the query parameters. The URI constructor encodes non-url-safe characters in the 4th argument (signedCanonicalQueryString). This 4th argument is already URL-encoded, so it will do it a 2nd time. This does not affect the Tyrus Websocket client that we are currently using, as it is able to handle this case. However, other websocket clients, such as the ones in the browser, and wscat cannot.

Previous:

URI uriResult = null;
try {
    uriResult = new URI(wssUri.getScheme(),
            wssUri.getRawAuthority(),
            getCanonicalUri(uri),
            signedCanonicalQueryString,
            null);
} catch (final URISyntaxException e) {
    e.printStackTrace();
}

Now:

URI uriResult = URI.create(wssUri.getScheme() + "://" + wssUri.getHost() + "/?" + getCanonicalUri(uri).substring(1) + signedCanonicalQueryString);

Testing

• Run the unit tests, and check for code coverage. It is at 100% for the AwsSigv4Signer class.
• Run the demo app a few times to ensure that the Websocket url can still be used to connect to Kinesis Video Signaling.
• For each run above, copied the same url to wscat to ensure it can connect there as well.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[sirknightj]

Marking as draft. When copying the changes over to https://github.com/aws-samples/amazon-kinesis-video-streams-demos/tree/master/sigv4-signing, the exact same generated URL results in a 403. Need some time to look into possible issues with the Tyrus websocket client.

Ideas:

• Gradle vs Maven? Version? Perhaps double encoding is required on one of the versions, but not on the other.

[sirknightj]

The issue is most likely a change with the Tyrus WebSocket client in the different versions of Tyrus we are using. In the maven project version, we are using Tyrus 1.15, and that fails every time. Bumping the version to the same as in Android (1.20), it works every time. I also tried with every version in between (1.16, 1.17, 1.18, and 1.19), it does not work.

In the release notes, I see this: https://github.com/eclipse-ee4j/tyrus/releases/tag/1.20, so that's probably what's causing it.