nacx requested changes on this pull request.
Thanks, @ak58588!
Please take another look at fixing indentation.
> public HttpRequest filter(HttpRequest request) throws HttpException {
+ this.sasAuthentication = authSAS(this.credential);
This could be set in the constructor instead of checking this on every request.
> + request = filterSAS(request, this.credential);
+ } else {
+ request = filterKey(request);
+ }
+ utils.logRequest(signatureLog, request, "<<");
+ return request;
+ }
+
+ /**
+ *
+ * this method checks the length of the authentication string, and decides,
which auth method it represents.
+ *
+ */
+ public boolean authSAS(String credential){
+ int length = credential.length();
+ return length < 150 && length > 120;
I'm not familiar with SAS auth. Is there a stronger or more reliable way to
detect the kind of credentials being used?
I'm thinking you could create a proper `SAASCredentials` object to be used when
creating the context (via the `credentialsSupplier` attribute, then this check
would be less error-prone as you can test the object type of the credentials.
Would this make sense?
> + /**
+ * this filter method is applied only for the cases with SAS Authentication.
+ *
+ */
+ public HttpRequest filterSAS(HttpRequest request, String credential) throws
HttpException {
+ String containerName = null;
+ String blobName = null;
+ URI requestUri = request.getEndpoint();
+ try {
+ String[] parametersArray = cutUri(requestUri);
+ containerName = parametersArray[1];
+ if (parametersArray.length == 3) {
+ blobName = parametersArray[2];
+ return
removeAuthorizationHeader(replaceDateHeader(request.toBuilder().endpoint(Uris.uriBuilder(storageUrl).appendPath(containerName).appendPath(blobName).query(credential).build()).build()));
+ }
+ return
removeAuthorizationHeader(replaceDateHeader(request.toBuilder().endpoint(Uris.uriBuilder(storageUrl).appendPath(containerName).query("restype=container&"
+ credential).build()).addHeader("x-ms-version", "2018-03-28").build()));
It would be good to be able to configure the value of this header via
properties. Make it configurable with this as the default value?
> + *
+ */
+ public HttpRequest filterSAS(HttpRequest request, String credential) throws
HttpException {
+ String containerName = null;
+ String blobName = null;
+ URI requestUri = request.getEndpoint();
+ try {
+ String[] parametersArray = cutUri(requestUri);
+ containerName = parametersArray[1];
+ if (parametersArray.length == 3) {
+ blobName = parametersArray[2];
+ return
removeAuthorizationHeader(replaceDateHeader(request.toBuilder().endpoint(Uris.uriBuilder(storageUrl).appendPath(containerName).appendPath(blobName).query(credential).build()).build()));
+ }
+ return
removeAuthorizationHeader(replaceDateHeader(request.toBuilder().endpoint(Uris.uriBuilder(storageUrl).appendPath(containerName).query("restype=container&"
+ credential).build()).addHeader("x-ms-version", "2018-03-28").build()));
+ } catch (NullPointerException e){
+ e.printStackTrace();
Use a logger to log errors instead of printing to stdout
> + }
+
+ /**
+ * this filter method is applied only for the cases with SAS Authentication.
+ *
+ */
+ public HttpRequest filterSAS(HttpRequest request, String credential) throws
HttpException {
+ String containerName = null;
+ String blobName = null;
+ URI requestUri = request.getEndpoint();
+ try {
+ String[] parametersArray = cutUri(requestUri);
+ containerName = parametersArray[1];
+ if (parametersArray.length == 3) {
+ blobName = parametersArray[2];
+ return
removeAuthorizationHeader(replaceDateHeader(request.toBuilder().endpoint(Uris.uriBuilder(storageUrl).appendPath(containerName).appendPath(blobName).query(credential).build()).build()));
Add some newlines here so it is easier to read
> private final HttpUtils utils;
+ private final URI storageUrl;
+ private boolean sasAuthentication;
Make this final once it is initialized int he constructor.
> + 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");
+ }
+
+ HttpRequest req = request.build();
+
+ return req;
+ }
Most of this method is just a copy of the existing one. Break the original one
in reusable methods and reuse code.
> + this.sasAuthentication = authSAS(this.credential);
+ if (this.sasAuthentication){
+ request = filterSAS(request, this.credential);
+ } else {
+ request = filterKey(request);
+ }
+ utils.logRequest(signatureLog, request, "<<");
+ return request;
+ }
+
+ /**
+ *
+ * this method checks the length of the authentication string, and decides,
which auth method it represents.
+ *
+ */
+ public boolean authSAS(String credential){
Make this static
> +
+ if (method.equals("PUT")) {
+ request.replaceHeader("x-ms-blob-type", "BlockBlob");
+ }
+
+ HttpRequest req = request.build();
+
+ return req;
+ }
+
+ /**
+ * 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) {
+ boolean isSAS = auth.authSAS(this.credential);
Again, this could go in the constructor instead of resolving it every time?
--
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-199114854
