nacx requested changes on this pull request.
This is in a much more good shape. Almost there! :)
@gaul I think the actual unit tests cover the filter functionality, so I don't
think we need special live/integration tests for that? It should be up to the
user running the live tests to use normal credentials or a token, but that
should just work, right?
> utils.logRequest(signatureLog, request, "<<");
return request;
}
-
+
+ /**
+ * this filter method is applied only for the cases with SAS
Authentication.
+ */
+ public HttpRequest filterSAS(HttpRequest request, String credential) throws
HttpException, IllegalArgumentException {
+ String containerName = null;
+ String blobName = null;
This is unused. Remove it.
> utils.logRequest(signatureLog, request, "<<");
return request;
}
-
+
+ /**
+ * this filter method is applied only for the cases with SAS
Authentication.
+ */
+ public HttpRequest filterSAS(HttpRequest request, String credential) throws
HttpException, IllegalArgumentException {
+ String containerName = null;
Remove this and declare the variable just where you use it the first time.
> utils.logRequest(signatureLog, request, "<<");
return request;
}
-
+
+ /**
+ * this filter method is applied only for the cases with SAS
Authentication.
+ */
+ public HttpRequest filterSAS(HttpRequest request, String credential) throws
HttpException, IllegalArgumentException {
+ String containerName = null;
+ String blobName = null;
+ URI requestUri = request.getEndpoint();
+ String initialQuery = requestUri.getQuery();
+ credential = initialQuery == null ? credential : initialQuery + "&" +
credential;
In general, better create a new variable. It is not a good idea to modify the
inputs of a 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);
+ return request.build();
+ }
+
+ /**
+ * modified sign() method, which acts depending on the Auth input.
+ */
+ public HttpRequest sign(String method, String container, String name,
@Nullable GetOptions options, long expires, @Nullable Long contentLength,
@Nullable String contentType) {
+ HttpRequest request = null;
Remove this
> + .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);
+ return request.build();
+ }
+
+ /**
+ * modified sign() method, which acts depending on the Auth input.
+ */
+ public HttpRequest sign(String method, String container, String name,
@Nullable GetOptions options, long expires, @Nullable Long contentLength,
@Nullable String contentType) {
+ HttpRequest request = null;
+ if (isSAS) {
+ return signSAS(method, container, name, options, expires,
contentLength, contentType);
+ }
+ return signKey(method, container, name, options, expires,
contentLength, contentType);
```suggestion
return signKey(method, container, name, options, expires, contentLength,
contentType);
```
> @@ -74,6 +86,15 @@ void testIdempotent(HttpRequest request) {
System.out.printf("%s: %d iterations before the timestamp updated %n",
Thread.currentThread().getName(),
iterations);
}
+
+ /**
+ * this test is similar to testIdempotent; it checks whether request is
properly filtered when it comes to SAS Authentication
+ */
+ @Test(threadPoolSize = 3, dataProvider = "auth-sas-data", timeOut = 3000)
Remove everything but the data provider. Let's use the default config
> @@ -74,6 +86,15 @@ void testIdempotent(HttpRequest request) {
System.out.printf("%s: %d iterations before the timestamp updated %n",
Thread.currentThread().getName(),
iterations);
}
+
+ /**
+ * this test is similar to testIdempotent; it checks whether request is
properly filtered when it comes to SAS Authentication
+ */
+ @Test(threadPoolSize = 3, dataProvider = "auth-sas-data", timeOut = 3000)
+ void testFilter(HttpRequest request, SharedKeyLiteAuthentication filter,
String expected){
```suggestion
void testFilter(HttpRequest request, SharedKeyLiteAuthentication filter,
String expected) {
```
> @@ -42,6 +42,7 @@
private static final String ACCOUNT = "foo";
private Injector injector;
private SharedKeyLiteAuthentication filter;
+ private SharedKeyLiteAuthentication filter2;
Better rename to `sasFilter` or so.
> +@Test(groups = "unit", testName = "AzureBlobHttpApiModuleTest")
+public class AzureBlobHttpApiModuleTest {
+
+ @DataProvider(name = "auth-sas-tokens")
+ public static Object[][] tokens() {
+ return new Object[][]{
+ {false, ""},
+ {false, "sv=2018-03-28&se=2019-02-14T11:12:13Z"},
+ {false,
"sv=2018-03-28&se=2019-02-14T11:12:13Z&sp=abc&st=2019-01-20T11:12:13Z"},
+ {false,
"u2iAP01ARTewyK/MhOM1d1ASPpjqclkldsdkljfas2kfjkh895ssfslkjpXKfhg=="},
+ {false,
"sadf;gjkhflgjkhfdlkfdljghskldjghlfdghw4986754ltjkghdlfkjghst;lyho56[09y7poinh"},
+ {false, "a=apple&b=banana&c=cucumber&d=diet"},
+ {false, "sva=swajak&sta=stancyja&spa=spakoj&sea=mora&sig=podpis"},
+ {true,
"sv=2018-03-28&ss=b&srt=sco&sp=r&se=2019-02-13T17:03:09Z&st=2019-02-13T09:03:09Z&spr=https&sig=wNkWK%2GURTjHWhtqG6Q2Gu%2Qu%3FPukW6N4%2FIH4Mr%2F%2FO42M%3D"},
+ {true,
"sp=rl&st=2019-02-14T08:50:26Z&se=2019-02-15T08:50:26Z&sv=2018-03-28&sig=Ukow8%2GtpQpAiVZBLcWp1%2RSpFq928MAqzp%2BdrdregaB6%3D&sr=b"},
+ {false, ""}
This is duplicated, remove.
--
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-204127304
