-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Using extended headers with sign URL #2422
Conversation
This reverts commit bd5f566.
Conflicts: google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java
if (canonicalizedExtensionHeaders != null) { | ||
|
||
List<String> orderedKeys = new ArrayList<>(canonicalizedExtensionHeaders.keySet()); | ||
Collections.sort(orderedKeys); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
* Creates an {@code SignatureInfo} object from this builder. | ||
*/ | ||
public SignatureInfo build() { | ||
return new SignatureInfo(this); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
* @param path the resource URI | ||
* @return signature info | ||
*/ | ||
private SignatureInfo buildSignarueInfo(Map<SignUrlOption.Option, Object> optionMap, |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@frantello do you still want to get this PR submitted? |
Hi @garrettjonesgoogle, for sure. I think it is an interesting feature. Just let me apply the @andreamlin revisions. I was very busy these weeks. I thank you both for the support! |
…source, expiration) are non-null in the build() method
@garrettjonesgoogle @andreamlin, it is done |
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.
LGTM
@andreamlin Is this pull request ready to merge? |
@frantello Would you mind fixing the java 8 Travis build error? It might just be that the URL on CanonicalExtensionHeadersSerializer.java:28 is broken up into two lines and the doc linter would rather have the URL on one line. |
@andreamlin done! |
Thanks @frantello ! |
Special features over sign URLs require extension headers.
Added headers must match specification about canonicalized extension headers.
I discover that headers must be ordered before sign. Otherwise server never will match signatures.