nacx requested changes on this pull request.
> }
-
+
+ /**
+ * this is an updated filter method, which decides whether the SAS or
SharedKeyLite
+ * is used and applies the right filtering.
+ *
```suggestion
```
> utils.logRequest(signatureLog, request, "<<");
return request;
}
-
+
+ /**
+ * this filter method is applied only for the cases with SAS
Authentication.
+ *
```suggestion
```
> + containerName = parametersArray[1];
+ UriBuilder endpoint =
Uris.uriBuilder(storageUrl).appendPath(containerName);
+ if (parametersArray.length == 3) {
+ endpoint.appendPath(parametersArray[2]).query(credential);
+ } else {
+ endpoint.query("restype=container&" + credential);
+ }
+ return removeAuthorizationHeader(
+ replaceDateHeader(request.toBuilder()
+ .endpoint(endpoint.build())
+ .build()));
+ }
+
+ /**
+ * this is a 'standard' filter method, applied when SharedKeyLite
authentication is used.
+ *
```suggestion
```
> HttpRequest replaceAuthorizationHeader(HttpRequest request, String
> signature) {
return request.toBuilder()
.replaceHeader(HttpHeaders.AUTHORIZATION, "SharedKeyLite " +
creds.get().identity + ":" + signature)
.build();
}
+
+ /**
+ * this method removes Authorisation header, since it is not needed for SAS
Authentication
Properly format the javadoc comment.
> @@ -110,7 +164,22 @@ HttpRequest replaceDateHeader(HttpRequest request) {
request =
request.toBuilder().replaceHeaders(Multimaps.forMap(builder.build())).build();
return request;
}
-
+
+ /**
+ * this is the method to parse container name and blob name from the
HttpRequest.
Properly format the javadoc comment and remove the blank line in the comment.
> @@ -105,8 +111,13 @@ public HttpRequest signGetBlob(String container, String
> name, org.jclouds.blobst
return sign("GET", container, name,
blob2HttpGetOptions.apply(checkNotNull(options, "options")),
DEFAULT_EXPIRY_SECONDS, null, null);
}
-
- private HttpRequest sign(String method, String container, String name,
@Nullable GetOptions options, long expires, @Nullable Long contentLength,
@Nullable String contentType) {
+
+
+ /**
+ * method to sign HttpRequest when SharedKey Authentication is used
+ *
```suggestion
```
> @@ -167,5 +163,61 @@ private HttpRequest sign(String method, String
> container, String name, @Nullable
String signature = auth.calculateSignature(stringToSign);
request.addQueryParam("sig", signature);
return request.build();
+
+ }
+
Remove the unnecessary blank lines
> @@ -167,5 +163,61 @@ private HttpRequest sign(String method, String
> container, String name, @Nullable
String signature = auth.calculateSignature(stringToSign);
request.addQueryParam("sig", signature);
return request.build();
+
```suggestion
```
> + request.replaceHeader(HttpHeaders.CONTENT_LENGTH,
> contentLength.toString());
+ }
+
+ if (contentType != null) {
+ request.replaceHeader("x-ms-blob-content-type", contentType);
+ }
+
+ if (options != null) {
+ request.headers(options.buildRequestHeaders());
+ }
+
+ if (method.equals("PUT")) {
+ request.replaceHeader("x-ms-blob-type", "BlockBlob");
+ }
+ return request;
+
```suggestion
```
> +
+ if (contentType != null) {
+ request.replaceHeader("x-ms-blob-content-type", contentType);
+ }
+
+ if (options != null) {
+ request.headers(options.buildRequestHeaders());
+ }
+
+ if (method.equals("PUT")) {
+ request.replaceHeader("x-ms-blob-type", "BlockBlob");
+ }
+ return request;
+
+ }
+
```suggestion
```
> +
+ if (options != null) {
+ request.headers(options.buildRequestHeaders());
+ }
+
+ if (method.equals("PUT")) {
+ request.replaceHeader("x-ms-blob-type", "BlockBlob");
+ }
+ return request;
+
+ }
+
+
+ /**
+ * method, compatible with SAS Authentication
+ *
```suggestion
```
> + * method, compatible with SAS Authentication
+ *
+ */
+ private HttpRequest signSAS(String method, String container, String name,
@Nullable GetOptions options, long expires, @Nullable Long contentLength,
@Nullable String contentType) {
+ checkNotNull(method, "method");
+ checkNotNull(container, "container");
+ checkNotNull(name, "name");
+ String nowString = timeStampProvider.get();
+
+ HttpRequest.Builder request = HttpRequest.builder()
+ .method(method)
+
.endpoint(Uris.uriBuilder(storageUrl).appendPath(container).appendPath(name).query(this.credential).build())
+ .replaceHeader(HttpHeaders.DATE, nowString);
+
+ request = setHeaders(request, method, options, contentLength,
contentType);
+ HttpRequest req = request.build();
Just `return request.build();`
> @@ -63,6 +72,23 @@ protected final String guiceProvideTimeStamp(@TimeStamp
> Supplier<String> cache)
protected String provideTimeStamp(@TimeStamp Supplier<String> cache) {
return cache.get();
}
+
+ /**
+ * checks which Authentication type is used
+ *
```suggestion
```
> @@ -74,6 +74,23 @@ void testIdempotent(HttpRequest request) {
System.out.printf("%s: %d iterations before the timestamp updated %n",
Thread.currentThread().getName(),
iterations);
}
+
+ @Test
Yes. Take the idempotent test as an example. You may probably want to be able
to create a filter with a SAS token credential and check that the filtered
request has the expected content.
Also, you need to add a new `AzureBlobHttpApiModuleTest.java` and add proper
unit tests for the `authSAS` method with different credential formats as an
input, to make sure it produces the right result for all variety of valid and
invalid inputs.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1270#pullrequestreview-203598965
