-
Notifications
You must be signed in to change notification settings - Fork 3
feat: sigva #30
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
feat: sigva #30
Conversation
| override fun equals(other: Any?): Boolean { | ||
| if (this === other) return true | ||
| if (other == null || this::class != other::class) return false | ||
|
|
||
| other as AwsSigningResult | ||
|
|
||
| if (signedRequest != other.signedRequest) return false | ||
| if (!signature.contentEquals(other.signature)) return false | ||
|
|
||
| return true | ||
| } | ||
|
|
||
| override fun hashCode(): Int { | ||
| var result = signedRequest?.hashCode() ?: 0 | ||
| result = 31 * result + signature.contentHashCode() | ||
| return result | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Why are these custom equals/hashCode implementations necessary? They seem identical to what I'd expect from the default implementations in a data class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh because Kotlin's data classes don't handle arrays as expected. Nevermind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^ yup that's why
| public actual suspend fun signRequest(request: HttpRequest, config: AwsSigningConfig): HttpRequest = | ||
| checkNotNull(sign(request, config).signedRequest) { "AwsSigningResult request must not be null" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: What circumstances could lead to the signing result's request being null? Looking through the code I can't see how that would occur.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the comments from crt-java:
https://github.com/awslabs/aws-crt-java/blob/main/src/main/java/software/amazon/awssdk/crt/auth/signing/AwsSigningResult.java#L15-L19
I think it's due to the flexibility in the signing implementation but in practice it shouldn't be null the way we use it. Or at least that's my understanding.
Issue #, if available:
N/A
Description of changes:
sign()method that returns anAwsSigningResultBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.