Skip to content

Conversation

@YomesInc
Copy link
Contributor

No description provided.

return Long.toString(System.currentTimeMillis() / 1000L);
}

public static byte[] getUTF8Bytes(String string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this was made public we need documentation. Same goes for produceSignature below.
Please also document all new methods/classes (ApiResponseSignatureVerifier, NotificationRequestSignatureVerifier, etc).

import com.cloudinary.utils.StringUtils;

class SigningUtils {
static String safeString(String str) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a method in StringUtils instead of a new SigningUtils file, as it's not really a signing issue (And may be used by other utils).


@Test
public void testVerifySignatureFailWhenSignatureDoesntMatch() {
NotificationRequestSignatureVerifier impl = new NotificationRequestSignatureVerifier("someApiSecret");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why impl? should be called something more specific (e.g. verifier), there's no abstract/interface vs impl involved.

@nitzanj nitzanj merged commit 50099c3 into master Feb 17, 2020
@nitzanj nitzanj deleted the feature/add-notification-signature-checking branch March 19, 2020 10:08
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.

3 participants