From a30aad05edc9885ff9c20c495a597cf7482b9936 Mon Sep 17 00:00:00 2001 From: Andrew Gaul Date: Mon, 14 May 2012 10:13:15 -0700 Subject: [PATCH] Harmonize filesystem and transient getBlob This commit also simplifies HTTP range logic calculations, sets the correct Content-Length, and avoids an unneeded copy. --- .../filesystem/FilesystemAsyncBlobStore.java | 31 ++++++++++++------- .../blobstore/TransientAsyncBlobStore.java | 27 +++++++++------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java b/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java index ec2d91f3f7..66b8dde35a 100644 --- a/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java +++ b/apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemAsyncBlobStore.java @@ -91,7 +91,9 @@ import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; import org.jclouds.http.HttpResponseException; +import org.jclouds.http.HttpUtils; import org.jclouds.http.options.HttpRequestOptions; +import org.jclouds.io.ContentMetadata; import org.jclouds.io.Payloads; import org.jclouds.io.payloads.BaseMutableContentMetadata; import org.jclouds.javax.annotation.Nullable; @@ -557,25 +559,32 @@ public class FilesystemAsyncBlobStore extends BaseAsyncBlobStore { } ByteArrayOutputStream out = new ByteArrayOutputStream(); for (String s : options.getRanges()) { + // HTTP uses a closed interval while Java array indexing uses a + // half-open interval. + int offset = 0; + int last = data.length - 1; if (s.startsWith("-")) { - int length = Integer.parseInt(s.substring(1)); - out.write(data, data.length - length, length); + offset = last - Integer.parseInt(s.substring(1)) + 1; } else if (s.endsWith("-")) { - int offset = Integer.parseInt(s.substring(0, s.length() - 1)); - out.write(data, offset, data.length - offset); + offset = Integer.parseInt(s.substring(0, s.length() - 1)); } else if (s.contains("-")) { String[] firstLast = s.split("\\-"); - int offset = Integer.parseInt(firstLast[0]); - int last = Integer.parseInt(firstLast[1]); - int length = last - offset + 1; // the range end is included - out.write(data, offset, length); + offset = Integer.parseInt(firstLast[0]); + last = Integer.parseInt(firstLast[1]); } else { - return immediateFailedFuture(new IllegalArgumentException("first and last were null!")); + return immediateFailedFuture(new IllegalArgumentException("illegal range: " + s)); } + if (offset > last || last + 1 > data.length) { + return immediateFailedFuture(new IllegalArgumentException("illegal range: " + s)); + } + out.write(data, offset, last - offset + 1); } - blob.setPayload(out.toByteArray()); - blob.getMetadata().getContentMetadata().setContentLength(new Long(data.length)); + ContentMetadata cmd = blob.getPayload().getContentMetadata(); + byte[] byteArray = out.toByteArray(); + blob.setPayload(byteArray); + HttpUtils.copy(cmd, blob.getPayload().getContentMetadata()); + blob.getPayload().getContentMetadata().setContentLength(new Long(byteArray.length)); } } checkNotNull(blob.getPayload(), "payload " + blob); diff --git a/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java b/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java index ca882c2661..182d015093 100644 --- a/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java +++ b/blobstore/src/main/java/org/jclouds/blobstore/TransientAsyncBlobStore.java @@ -590,27 +590,32 @@ public class TransientAsyncBlobStore extends BaseAsyncBlobStore { } ByteArrayOutputStream out = new ByteArrayOutputStream(); for (String s : options.getRanges()) { + // HTTP uses a closed interval while Java array indexing uses a + // half-open interval. + int offset = 0; + int last = data.length - 1; if (s.startsWith("-")) { - int length = Integer.parseInt(s.substring(1)); - out.write(data, data.length - length, length); + offset = last - Integer.parseInt(s.substring(1)) + 1; } else if (s.endsWith("-")) { - int offset = Integer.parseInt(s.substring(0, s.length() - 1)); - out.write(data, offset, data.length - offset); + offset = Integer.parseInt(s.substring(0, s.length() - 1)); } else if (s.contains("-")) { String[] firstLast = s.split("\\-"); - int offset = Integer.parseInt(firstLast[0]); - int last = Integer.parseInt(firstLast[1]); - int length = (last < data.length) ? last + 1 : data.length - offset; - out.write(data, offset, length); + offset = Integer.parseInt(firstLast[0]); + last = Integer.parseInt(firstLast[1]); } else { - return immediateFailedFuture(new IllegalArgumentException("first and last were null!")); + return immediateFailedFuture(new IllegalArgumentException("illegal range: " + s)); } + if (offset > last || last + 1 > data.length) { + return immediateFailedFuture(new IllegalArgumentException("illegal range: " + s)); + } + out.write(data, offset, last - offset + 1); } ContentMetadata cmd = blob.getPayload().getContentMetadata(); - blob.setPayload(out.toByteArray()); + byte[] byteArray = out.toByteArray(); + blob.setPayload(byteArray); HttpUtils.copy(cmd, blob.getPayload().getContentMetadata()); - blob.getPayload().getContentMetadata().setContentLength(new Long(out.toByteArray().length)); + blob.getPayload().getContentMetadata().setContentLength(new Long(byteArray.length)); } } checkNotNull(blob.getPayload(), "payload " + blob);