You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Zhao Jin <no...@github.com> on 2015/02/11 04:01:11 UTC
[jclouds] add aws s3 signature v4 (#678)
AWS S3 signature v4 impl
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/678
-- Commit Summary --
* add aws s3 signature v4
-- File Changes --
M apis/s3/src/main/java/org/jclouds/s3/config/S3HttpApiModule.java (308)
A apis/s3/src/main/java/org/jclouds/s3/filters/AwsHostNameUtils.java (185)
M apis/s3/src/main/java/org/jclouds/s3/filters/RequestAuthorizeSignature.java (319)
A apis/s3/src/main/java/org/jclouds/s3/filters/RequestAuthorizeSignatureV4.java (461)
A apis/s3/src/main/java/org/jclouds/s3/filters/S3RequestAuthorizeSignatureV4.java (70)
M apis/s3/src/test/java/org/jclouds/s3/S3ClientMockTest.java (33)
A apis/s3/src/test/java/org/jclouds/s3/filters/AwsHostNameUtilsTest.java (61)
D apis/s3/src/test/java/org/jclouds/s3/filters/RequestAuthorizeSignatureTest.java (157)
A apis/s3/src/test/java/org/jclouds/s3/filters/RequestAuthorizeSignatureV2Test.java (157)
A apis/s3/src/test/java/org/jclouds/s3/filters/RequestAuthorizeSignatureV4Test.java (118)
M apis/s3/src/test/java/org/jclouds/s3/internal/BaseS3ClientTest.java (10)
M providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/AWSS3BlobRequestSigner.java (6)
M providers/aws-s3/src/main/java/org/jclouds/aws/s3/filters/AWSRequestAuthorizeSignature.java (5)
A providers/aws-s3/src/main/java/org/jclouds/aws/s3/filters/AWSRequestAuthorizeSignatureV4.java (58)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/678.patch
https://github.com/jclouds/jclouds/pull/678.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> + }
> +
> + byte[] hmacSHA256(String toSign, byte[] key) {
> + try {
> + ByteProcessor<byte[]> hmacSHA256 = asByteProcessor(crypto.hmacSHA256(key));
> + return readBytes(toInputStream(toSign), hmacSHA256);
> + } catch (IOException e) {
> + throw new HttpException("read bytes error", e);
> + } catch (InvalidKeyException e) {
> + throw new HttpException("invalid key", e);
> + }
> + }
> +
> + protected byte[] hash(InputStream input) throws HTTPException {
> + try {
> + MessageDigest md = MessageDigest.getInstance("SHA-256");
Can you use Guava `Hashing.sha256` instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24491587
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
@pswvg This PR still needs a thorough review -- S3 is our most popular provider and I want to ensure we do not introduce any regressions for our users.
@zhaojin0 Sorry about the delay; I will try to get to this soon.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-114307152
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by pswvg <no...@github.com>.
I thought about the content-length again. The old V2 implementation wasn't able to handle unknown content-lengths. So does the V4. But I think it wouldn't be so difficult to add this. Because chunking is already implemented nicely and instead of sending the content-length of the complete payload, one could switch to transfer-encoding instead - like "Note"d here:
http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-streaming.html
That would mean:
Transfer-Encoding: chunked
should do the trick and so omitting Content-Length completely.
That would be really nice, since in our project we aren't always be able to get the complete
content length in advance (live streaming from browser clients etc.). So in the worst-case, we have
temporarily store the whole stream to disk (to get the content length) before we can send it to s3.
What do you guys think?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-103791927
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> @@ -29,6 +25,10 @@
> import org.testng.annotations.BeforeClass;
> import org.testng.annotations.Test;
>
> +import java.io.IOException;
> +
> +import static org.testng.Assert.assertEquals;
> +
Spurious code movement?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r28619932
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
AWS Sign V4 use sha256 content hash. If payload can not be reset, aws supported chunked uploads.
http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-streaming.html
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-88369850
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.s3.filters;
> +
> +import org.testng.Assert;
> +import org.testng.annotations.Test;
> +
> +import java.net.URI;
> +
> +/**
> + * Created by ZhaoJin on 15-2-3.
> + */
Please remove useless Javadoc.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24491611
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> + try {
> + String encoded = URLEncoder.encode(value, DEFAULT_ENCODING);
> +
> + Matcher matcher = ENCODED_CHARACTERS_PATTERN.matcher(encoded);
> + StringBuffer buffer = new StringBuffer(encoded.length());
> +
> + while (matcher.find()) {
> + String replacement = matcher.group(0);
> +
> + if ("+".equals(replacement)) {
> + replacement = "%20";
> + } else if ("*".equals(replacement)) {
> + replacement = "%2A";
> + } else if ("%7E".equals(replacement)) {
> + replacement = "~";
> + }
Does Guava `UrlEscapers` do the same thing?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24491973
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
@zhaojin0 I made some stylistic modifications, squashed the commits, and pushed to master as 8bddbb496a7b1a52f851e3e22626eaf644072973. Thank you for your contribution and for your patience with this extraordinarily long review!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-171471644
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
Hi, I impl aws s3 signer v4 chunked upload, use when put object, payload cannot repeatable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-98595506
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
>
> - private static final Set<String> SIGNED_PARAMETERS = ImmutableSet.of("acl", "torrent", "logging", "location", "policy",
> + private static final Set<String> SIGNED_PARAMETERS = ImmutableSet.of("acl", "torrent", "logging", "location", "policy",
Right, but can we move this code into its own source file instead of it having a inner class in the interface?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r28619901
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> + SortedMap<String, String> sorted = new TreeMap<String, String>();
> + if (params == null) {
> + return "";
> + }
> + Iterator<Map.Entry<String, String>> pairs = params.entries().iterator();
> + while (pairs.hasNext()) {
> + Map.Entry<String, String> pair = pairs.next();
> + String key = pair.getKey();
> + String value = pair.getValue();
> + sorted.put(urlEncode(key), urlEncode(value));
> + }
> +
> + StringBuilder builder = new StringBuilder();
> + pairs = sorted.entrySet().iterator();
> + while (pairs.hasNext()) {
> + Map.Entry<String, String> pair = pairs.next();
ok
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24555334
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
add aws s3 signature v4, plz review.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-73835221
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
It's seem as region eu-central-1 doesn't supported AWS sign V2.
在 2015/4/18 2:46, Andrew Gaul 写道:
>
> @zhaojin0 <https://github.com/zhaojin0> I am testing this and see many
> errors of the form:
>
> |org.jclouds.aws.AWSResponseException: request GET https://gaul-blobstore3760643340725640912-v4-only.s3-eu-central-1.amazonaws.com/ HTTP/1.1 failed with code 400, error: AWSError{requestId='0F84681CE6013127', requestToken='THXHcOkpHKTdN7DedbIb8qCj7/MuvvvGyu31O42h6BjkTFSvHdKNggDL/aQ6Mm1IGROwsAO58fw=', code='InvalidRequest', message='The authorization mechanism you have provided is not supported. Please use AWS4-HMAC-SHA256.', context='{HostId=THXHcOkpHKTdN7DedbIb8qCj7/MuvvvGyu31O42h6BjkTFSvHdKNggDL/aQ6Mm1IGROwsAO58fw=}'}
> |
>
> when deleting items in the container between runs. Any suggestions on
> this? Also do all the integration tests pass for you? I see a few errors:
>
> | AWSS3ContainerIntegrationLiveTest>BaseContainerIntegrationTest.deleteContainerIfEmptyWithoutContents:315 expected [true] but found [false]
> AWSS3ContainerLiveTest>BaseContainerLiveTest.testPublicAccessInNonDefaultLocationWithBigBlob:112->BaseContainerLiveTest.runCreateContainerInLocation:124->BaseBlobStoreIntegrationTest.assertConsistencyAwareContainerExists:361->BaseBlobStoreIntegrationTest.assertConsistencyAware:248->BaseBlobStoreIntegrationTest.assertConsistencyAware:235 » HttpResponse
> AWSS3ServiceIntegrationLiveTest>BaseServiceIntegrationTest.testAllLocations:52->BaseBlobStoreIntegrationTest.assertConsistencyAware:248->BaseBlobStoreIntegrationTest.assertConsistencyAware:235 » HttpResponse
> AWSS3ServiceIntegrationLiveTest>BaseServiceIntegrationTest.testGetAssignableLocations:93 {scope=REGION, id=eu-central-1, description=eu-central-1, parent=aws-s3, iso3166Codes=[DE-HE]} ||{scope=PROVIDER, id=aws-s3, description=https://s3.amazonaws.com, iso3166Codes=[US, US-CA, US-OR, BR-SP, IE, SG, AU-NSW, JP-13]}
>
> Tests run: 99, Failures: 4, Errors: 0, Skipped: 1
> |
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/jclouds/jclouds/pull/678#issuecomment-94050351>.
>
--
赵金
Zhao Jin
18610722868
北京优创联动科技有限公司
北京市 海淀区 学清路38号 金码大厦16层
100083
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-94248352
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> + ChunkedInputStreamEnumeration(InputStream inputStream, int chunkedBlockSize) {
> + this.inputStream = new BufferedInputStream(inputStream, chunkedBlockSize);
> + buffer = new byte[chunkedBlockSize];
> + lastChunked = false;
> + }
> +
> + @Override
> + public boolean hasMoreElements() {
> + return !lastChunked;
> + }
> +
> + @Override
> + public InputStream nextElement() {
> + int bytesRead;
> + try {
> + bytesRead = inputStream.read(buffer, 0, buffer.length);
Can also call `ByteStreams.readFully(InputStream, byte[])`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r30610288
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
Closed #678.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#event-514370147
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
I'm sorry for my lazy...
i add temporary access signature code, but i dont known how to test it. It could work in aws region cn-north-1.
```java
public class AWSS3BlobStoreContextModule extends S3BlobStoreContextModule {
//...
@Override
protected void bindRequestSigner() {
bind(BlobRequestSigner.class).to(AWSS3BlobRequestSignerV4.class);
}
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-75247425
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> @@ -56,7 +56,7 @@
> public AWSS3BlobRequestSigner(RestAnnotationProcessor processor, BlobToObject blobToObject,
> BlobToHttpGetOptions blob2HttpGetOptions, Class<AWSS3Client> interfaceClass,
> @org.jclouds.location.Provider Supplier<Credentials> credentials,
> - RequestAuthorizeSignature authSigner, @TimeStamp Provider<String> timeStampProvider,
> + RequestAuthorizeSignatureV2 authSigner, @TimeStamp Provider<String> timeStampProvider,
Why does this take the concrete class `RequestAuthorizeSignatureV2` instead of the interface `RequestAuthorizeSignature`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24492063
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> + }
> +
> + byte[] hmacSHA256(String toSign, byte[] key) {
> + try {
> + ByteProcessor<byte[]> hmacSHA256 = asByteProcessor(crypto.hmacSHA256(key));
> + return readBytes(toInputStream(toSign), hmacSHA256);
> + } catch (IOException e) {
> + throw new HttpException("read bytes error", e);
> + } catch (InvalidKeyException e) {
> + throw new HttpException("invalid key", e);
> + }
> + }
> +
> + protected byte[] hash(InputStream input) throws HTTPException {
> + try {
> + MessageDigest md = MessageDigest.getInstance("SHA-256");
ok. Hashing.sha256 is same as MessageDigest
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24555082
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> + HttpRequest.Builder requestBuilder,
> + String method,
> + URI endpoint,
> + Payload payload
> + ) {
> + InputStream payloadStream;
> + try {
> + payloadStream = usePayloadForQueryParameters(method, payload) ?
> + getQueryStringContent(endpoint)
> + : getPayloadContentWithoutQueryString(payload);
> + } catch (IOException e) {
> + throw new HttpException("Unable to open stream before calculate AWS4 signature", e);
> + }
> + String contentSha256 = base16().lowerCase().encode(hash(payloadStream));
> + try {
> + payloadStream.reset();
payload stream use calculate content hash.
if can not be repeatable, the payload cannot append to http request body.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r25842533
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
it's use for sign a temporary access...
I provided testcase AWSS3BlobSignerV4ExpectTest.
sorry, this's my first pull request, I format some code....
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-76351814
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by pswvg <no...@github.com>.
AFAIK this is reported against 1.8.x. I've applied this PR to a 1.8.1 fork without a problem.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-139468927
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
@zhaojin0 Please address Checkstyle violations and remove gratuitous reindentation. I will look at this some more afterwards.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-73876014
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
>
> - private static final Set<String> SIGNED_PARAMETERS = ImmutableSet.of("acl", "torrent", "logging", "location", "policy",
> + private static final Set<String> SIGNED_PARAMETERS = ImmutableSet.of("acl", "torrent", "logging", "location", "policy",
Should this be in its own class like the V4 signer?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24491771
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
@zhaojin0 I do not understand how this is supposed to work -- you should bind the V4 signer somewhere like `AWSS3BlobStoreContextModule`:
```java
bind(BlobRequestSigner.class).to(AWSS3BlobRequestSignerV4.class);
```
I do not understand the overall strategy here, but I believe that jclouds should use the new V4 signer for all calls and regions. The generic S3 can continue to use the V2 code.
I played around with this a bit and do not see any new live tests. I added one to `AWSBucketsLiveTest` which requires a region with a V4 signer:
```java
public void testV4Region() throws Exception {
String bucketName = getScratchContainerName() + "-v4-only";
getApi().putBucketInRegion(Region.EU_CENTRAL_1, bucketName);
try {
assertEquals(Region.EU_CENTRAL_1, getApi().getBucketLocation(bucketName));
} finally {
destroyContainer(bucketName);
}
}
```
However a see a variety of signing errors of the form:
```
org.jclouds.aws.AWSResponseException: request GET https://gaul-blobstore-2489004073675868056-v4-only.s3-eu-central-1.amazonaws.com/ HTTP/1.1 failed with code 400, error: AWSError{requestId='7EF96215BA50542A', requestToken='t2sOzdsOU5tJqer0pLcfbuAQnuASYf4X3xf1qfWQkjSESCp5hBcgKhMuoO3zRJ6u14OLnNwHokE=', code='InvalidRequest', message='The authorization mechanism you have provided is not supported. Please use AWS4-HMAC-SHA256.', context='{HostId=t2sOzdsOU5tJqer0pLcfbuAQnuASYf4X3xf1qfWQkjSESCp5hBcgKhMuoO3zRJ6u14OLnNwHokE=}'}
```
Finally there are many spurious changes and Checkstyle violations in this pull request which make it hard to read. You must correct these before I can continue to review.
As for the signer tests, the general ones in `BaseBlobSignerLiveTest` should exercise your code if you bind the signer correctly as per my first comment.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-76095010
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> + HttpRequest.Builder requestBuilder,
> + String method,
> + URI endpoint,
> + Payload payload
> + ) {
> + InputStream payloadStream;
> + try {
> + payloadStream = usePayloadForQueryParameters(method, payload) ?
> + getQueryStringContent(endpoint)
> + : getPayloadContentWithoutQueryString(payload);
> + } catch (IOException e) {
> + throw new HttpException("Unable to open stream before calculate AWS4 signature", e);
> + }
> + String contentSha256 = base16().lowerCase().encode(hash(payloadStream));
> + try {
> + payloadStream.reset();
What happens when the payload is not repeatable?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r25824362
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> +
> + @Override
> + public String service() {
> + return service;
> + }
> +
> + @Override
> + public String region(String host) {
> + return AwsHostNameUtils.parseRegionName(host, service());
> + }
> + }
> + }
> +
> + private final SignatureWire signatureWire;
> + private final String headerTag;
> + // private final String apiVersion;
copy from ```org.jclouds.aws.filters.FormSignerV4```
current, s3 api havn't any api version parameters, it's use in ec2 or other aws api...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24556779
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> + SortedMap<String, String> sorted = new TreeMap<String, String>();
> + if (params == null) {
> + return "";
> + }
> + Iterator<Map.Entry<String, String>> pairs = params.entries().iterator();
> + while (pairs.hasNext()) {
> + Map.Entry<String, String> pair = pairs.next();
> + String key = pair.getKey();
> + String value = pair.getValue();
> + sorted.put(urlEncode(key), urlEncode(value));
> + }
> +
> + StringBuilder builder = new StringBuilder();
> + pairs = sorted.entrySet().iterator();
> + while (pairs.hasNext()) {
> + Map.Entry<String, String> pair = pairs.next();
Prefer Guava `Joiner.on("&").withKeyValueSeparator("=").join(sorted)`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24491937
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> -
> - @Provides
> - @Bucket
> - @Singleton
> - protected CacheLoader<String, Optional<String>> bucketToRegion(@Region Supplier<Set<String>> regionSupplier,
> - final S3Client client) {
> - Set<String> regions = regionSupplier.get();
> - if (regions.size() == 0) {
> - return new CacheLoader<String, Optional<String>>() {
> -
> - @Override
> - public Optional<String> load(String bucket) {
> - return Optional.absent();
> - }
> -
> + @SuppressWarnings("unchecked")
Seems like a lot of gratuitous reindentation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24491535
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
@zhaojin0 Any updates on this pull request? I would like to clean it up and include it in the upcoming 1.9.0 release.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-74991773
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
Could you tag this commit with [JCLOUDS-480](https://issues.apache.org/jira/browse/JCLOUDS-480) so that JIRA will note it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-74193650
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> @@ -56,7 +56,7 @@
> public AWSS3BlobRequestSigner(RestAnnotationProcessor processor, BlobToObject blobToObject,
> BlobToHttpGetOptions blob2HttpGetOptions, Class<AWSS3Client> interfaceClass,
> @org.jclouds.location.Provider Supplier<Credentials> credentials,
> - RequestAuthorizeSignature authSigner, @TimeStamp Provider<String> timeStampProvider,
> + RequestAuthorizeSignatureV2 authSigner, @TimeStamp Provider<String> timeStampProvider,
sorry, this is an wrong.. BlobRequestSigner use to generating pre-signed URLs, [Share an Object with Others](http://docs.aws.amazon.com/AmazonS3/latest/dev/ShareObjectPreSignedURL.html)
AWSS3BlobRequestSigner is use for Signature V2
I'll impl AWSS3BlobRequestSignerV4 for Signature V4
[sigv4-query-string-auth](http://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24713534
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Phillips <no...@github.com>.
@andrewgaul Ping? Is this something you'd be able to take a look at?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-165160819
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by pswvg <no...@github.com>.
The last time I tried Multipart Upload with jclouds aws-s3, it also forced me to specify the content length in advance. See my post here:
https://www.mail-archive.com/user@jclouds.apache.org/msg01562.html.
But that's another story.
What is the state about this PR? What is missing before it could be merged? I think the build failure was because of a checkstyle error.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-114101019
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> + try {
> + String encoded = URLEncoder.encode(value, DEFAULT_ENCODING);
> +
> + Matcher matcher = ENCODED_CHARACTERS_PATTERN.matcher(encoded);
> + StringBuffer buffer = new StringBuffer(encoded.length());
> +
> + while (matcher.find()) {
> + String replacement = matcher.group(0);
> +
> + if ("+".equals(replacement)) {
> + replacement = "%20";
> + } else if ("*".equals(replacement)) {
> + replacement = "%2A";
> + } else if ("%7E".equals(replacement)) {
> + replacement = "~";
> + }
URLEncoder.encode can be replace with UrlEscapers.urlFormParameterEscaper.escape
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24557031
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> @@ -56,7 +56,7 @@
> public AWSS3BlobRequestSigner(RestAnnotationProcessor processor, BlobToObject blobToObject,
> BlobToHttpGetOptions blob2HttpGetOptions, Class<AWSS3Client> interfaceClass,
> @org.jclouds.location.Provider Supplier<Credentials> credentials,
> - RequestAuthorizeSignature authSigner, @TimeStamp Provider<String> timeStampProvider,
> + RequestAuthorizeSignatureV2 authSigner, @TimeStamp Provider<String> timeStampProvider,
BlobRequestSigner Generates signed requests for blobs. useful in other tools such as backup utilities.
keep AWSS3BlobRequestSigner use RequestAuthorizeSignatureV2 to signed requests for blobs as before.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24556033
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
> +
> + @Override
> + public String service() {
> + return service;
> + }
> +
> + @Override
> + public String region(String host) {
> + return AwsHostNameUtils.parseRegionName(host, service());
> + }
> + }
> + }
> +
> + private final SignatureWire signatureWire;
> + private final String headerTag;
> + // private final String apiVersion;
Why commented out code?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24492208
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by pswvg <no...@github.com>.
> + ChunkedInputStreamEnumeration(InputStream inputStream, int chunkedBlockSize) {
> + this.inputStream = new BufferedInputStream(inputStream, chunkedBlockSize);
> + buffer = new byte[chunkedBlockSize];
> + lastChunked = false;
> + }
> +
> + @Override
> + public boolean hasMoreElements() {
> + return !lastChunked;
> + }
> +
> + @Override
> + public InputStream nextElement() {
> + int bytesRead;
> + try {
> + bytesRead = inputStream.read(buffer, 0, buffer.length);
bytesRead can be less than buffer.length!
This caused problems during our tests, when the underlying inputStream only allows 8k blocks. The result was, that the pre-calculated content-length (using 64k chunks) was less than the actual send content length (using 8k chunks got from the InputStream).
That leads to HttpUrlConnection's "too many bytes written" Exception later on.
We fixed that on our side with a wrapping InputStream that always returns the requested length when read(buf,off,len) is invoked. Performing n additional reads from the underlying inputStream if necessary.
This way, the expected 64k blocks were send and the pre-calculated content-length matched the actual content length.
Maybe a loop must be added here to read exactly buffer.length bytes from the inputStream matching the chunkedBlockSize, like we did in our inputStream wrapper.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r30581994
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
> + try {
> + String encoded = URLEncoder.encode(value, DEFAULT_ENCODING);
> +
> + Matcher matcher = ENCODED_CHARACTERS_PATTERN.matcher(encoded);
> + StringBuffer buffer = new StringBuffer(encoded.length());
> +
> + while (matcher.find()) {
> + String replacement = matcher.group(0);
> +
> + if ("+".equals(replacement)) {
> + replacement = "%20";
> + } else if ("*".equals(replacement)) {
> + replacement = "%2A";
> + } else if ("%7E".equals(replacement)) {
> + replacement = "~";
> + }
UrlEscapers.urlFormParameterEscaper isn's same as this urlEncode.
* URI encode every byte except the unreserved characters: 'A'-'Z', 'a'-'z', '0'-'9', '-', '.', '_', and '~'.
* The space character is a reserved character and must be encoded as "%20" (and not as "+").
* Each Uri-encoded byte is formed by a '%' and the two-digit hexadecimal value of the byte.
* Letters in the hexadecimal value must be uppercase, for example "%1A".
* Encode the forward slash character, '/', everywhere except in the object key name. For example, if the object key name is photos/Jan/sample.jpg, the forward slash in the key name is not encoded.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24555634
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
>
> - private static final Set<String> SIGNED_PARAMETERS = ImmutableSet.of("acl", "torrent", "logging", "location", "policy",
> + private static final Set<String> SIGNED_PARAMETERS = ImmutableSet.of("acl", "torrent", "logging", "location", "policy",
Signature v4 need signed all of query string parameters.
_CanonicalQueryString specifies the URI-encoded query string parameters. You URI-encode name and values individually. You must also sort the parameters in the canonical query string alphabetically by key name. The sorting occurs after encoding._
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678/files#r24555329
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Fred Simon <no...@github.com>.
Any reason for this not to have been merged?
Is it reported to 2.0 version only?
Could it be merge on 1.9 branch?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-139418881
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Daniel Manzke <no...@github.com>.
hey guys any progress on it? I hate that we have to use a patch version in production, but we already use it for some time and have load tested it as well. :+1:
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-165158363
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
@demobox I will try to make a run at this early next week when I work on some related V4 issues. Sorry for the delay!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-165925177
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Zhao Jin <no...@github.com>.
try Multipart Upload.
initiate Multipart Upload..temporarily store specific length part, upload all stream part......complete multipart upload...
part requires Content-Length. not all a file. but I have not tried :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-104135788
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
Any plans to continue with this pull request?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-88231285
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by Andrew Gaul <no...@github.com>.
@zhaojin0 I am testing this and see many errors of the form:
```
org.jclouds.aws.AWSResponseException: request GET https://gaul-blobstore3760643340725640912-v4-only.s3-eu-central-1.amazonaws.com/ HTTP/1.1 failed with code 400, error: AWSError{requestId='0F84681CE6013127', requestToken='THXHcOkpHKTdN7DedbIb8qCj7/MuvvvGyu31O42h6BjkTFSvHdKNggDL/aQ6Mm1IGROwsAO58fw=', code='InvalidRequest', message='The authorization mechanism you have provided is not supported. Please use AWS4-HMAC-SHA256.', context='{HostId=THXHcOkpHKTdN7DedbIb8qCj7/MuvvvGyu31O42h6BjkTFSvHdKNggDL/aQ6Mm1IGROwsAO58fw=}'}
```
when deleting items in the container between runs. Any suggestions on this? Also do all the integration tests pass for you? I see a few errors:
```
AWSS3ContainerIntegrationLiveTest>BaseContainerIntegrationTest.deleteContainerIfEmptyWithoutContents:315 expected [true] but found [false]
AWSS3ContainerLiveTest>BaseContainerLiveTest.testPublicAccessInNonDefaultLocationWithBigBlob:112->BaseContainerLiveTest.runCreateContainerInLocation:124->BaseBlobStoreIntegrationTest.assertConsistencyAwareContainerExists:361->BaseBlobStoreIntegrationTest.assertConsistencyAware:248->BaseBlobStoreIntegrationTest.assertConsistencyAware:235 » HttpResponse
AWSS3ServiceIntegrationLiveTest>BaseServiceIntegrationTest.testAllLocations:52->BaseBlobStoreIntegrationTest.assertConsistencyAware:248->BaseBlobStoreIntegrationTest.assertConsistencyAware:235 » HttpResponse
AWSS3ServiceIntegrationLiveTest>BaseServiceIntegrationTest.testGetAssignableLocations:93 {scope=REGION, id=eu-central-1, description=eu-central-1, parent=aws-s3, iso3166Codes=[DE-HE]} ||{scope=PROVIDER, id=aws-s3, description=https://s3.amazonaws.com, iso3166Codes=[US, US-CA, US-OR, BR-SP, IE, SG, AU-NSW, JP-13]}
Tests run: 99, Failures: 4, Errors: 0, Skipped: 1
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-94050351
Re: [jclouds] add aws s3 signature v4 (#678)
Posted by pswvg <no...@github.com>.
For our project, we took the PR, applied some minor fixes (https://github.com/zhaojin0/jclouds/pull/1) and backported it to stable 1.8.1. With our Integration tests, it works very well. Tested it against Frankfurt - which supports v4 only.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/678#issuecomment-102297653