bilaharith commented on a change in pull request #2422:
URL: https://github.com/apache/hadoop/pull/2422#discussion_r515894489
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java
##########
@@ -36,6 +36,7 @@
import org.codehaus.jackson.JsonParser;
import org.codehaus.jackson.JsonToken;
import org.codehaus.jackson.map.ObjectMapper;
+import com.google.common.annotations.VisibleForTesting;
Review comment:
Done
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java
##########
@@ -256,7 +256,9 @@ private boolean executeHttpOperation(final int retryCount)
throws AzureBlobFileS
}
} catch (IOException ex) {
if (ex instanceof UnknownHostException) {
- LOG.warn(String.format("Unknown host name: %s. Retrying to resolve the
host name...", httpOperation.getUrl().getHost()));
+ LOG.warn(String.format(
+ "Unknown host name: %s. Retrying to resolve the host name...",
+ httpOperation.getHost()));
Review comment:
Done
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelegationSAS.java
##########
@@ -381,4 +383,39 @@ public void testProperties() throws Exception {
assertArrayEquals(propertyValue, fs.getXAttr(reqPath, propertyName));
}
+
+ @Test
+ public void testSignatureMask() throws Exception {
+ final AzureBlobFileSystem fs = getFileSystem();
+ String src = "/testABC/test.xt";
+ fs.create(new Path(src));
+ AbfsRestOperation abfsHttpRestOperation = fs.getAbfsClient()
+ .renamePath(src, "/testABC" + "/abc.txt", null);
+ AbfsHttpOperation result = abfsHttpRestOperation.getResult();
+ String url = result.getSignatureMaskedUrlStr();
+ String encodedUrl = result.getSignatureMaskedEncodedUrlStr();
+ Assertions.assertThat(url.substring(url.indexOf("sig=")))
+ .describedAs("Signature query param should be masked")
+ .startsWith("sig=XXXX");
+ Assertions.assertThat(encodedUrl.substring(encodedUrl.indexOf("sig%3D")))
+ .describedAs("Signature query param should be masked")
+ .startsWith("sig%3DXXXX");
+ }
+
+ @Test
+ public void testSignatureMaskOnExceptionMessage() {
+ final AzureBlobFileSystem fs;
+ String msg = null;
+ try {
+ fs = getFileSystem();
Review comment:
Done
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java
##########
@@ -513,4 +509,45 @@ private void parseListFilesResponse(final InputStream
stream) throws IOException
private boolean isNullInputStream(InputStream stream) {
return stream == null ? true : false;
}
+
+ @VisibleForTesting
+ public String getSignatureMaskedUrlStr() {
+ if (this.maskedUrlStr != null) {
+ return this.maskedUrlStr;
+ }
+ final String urlStr = url.toString();
Review comment:
Done
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java
##########
@@ -513,4 +509,45 @@ private void parseListFilesResponse(final InputStream
stream) throws IOException
private boolean isNullInputStream(InputStream stream) {
return stream == null ? true : false;
}
+
+ @VisibleForTesting
+ public String getSignatureMaskedUrlStr() {
+ if (this.maskedUrlStr != null) {
+ return this.maskedUrlStr;
+ }
+ final String urlStr = url.toString();
+ final String qpStr = "sig=";
Review comment:
Done
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java
##########
@@ -513,4 +509,45 @@ private void parseListFilesResponse(final InputStream
stream) throws IOException
private boolean isNullInputStream(InputStream stream) {
return stream == null ? true : false;
}
+
+ @VisibleForTesting
+ public String getSignatureMaskedUrlStr() {
+ if (this.maskedUrlStr != null) {
+ return this.maskedUrlStr;
+ }
+ final String urlStr = url.toString();
+ final String qpStr = "sig=";
+ final int qpStrIdx = urlStr.indexOf(qpStr);
+ if (qpStrIdx < 0) {
+ return urlStr;
+ }
+ final StringBuilder sb = new StringBuilder();
+ sb.append(urlStr, 0, qpStrIdx);
+ sb.append(qpStr);
+ sb.append("XXXX");
+ if (qpStrIdx + qpStr.length() < urlStr.length()) {
+ String urlStrSecondPart = urlStr.substring(qpStrIdx + qpStr.length());
+ int idx = urlStrSecondPart.indexOf("&");
+ if (idx > -1) {
+ sb.append(urlStrSecondPart.substring(idx));
+ }
Review comment:
Done
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsIoUtils.java
##########
@@ -58,6 +58,9 @@ public static void dumpHeadersToDebugLog(final String origin,
if (key.contains("Cookie")) {
values = "*cookie info*";
}
+ if (key.equals("sig")) {
Review comment:
The latency tracker sends all the query params through headers.
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java
##########
@@ -61,6 +64,8 @@
private final String method;
private final URL url;
+ private String maskedUrlStr;
Review comment:
Remove Str suffix
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsHttpOperation.java
##########
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.apache.hadoop.fs.azurebfs.services;
+
+import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLEncoder;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+import static
org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation.getAbfsHttpOperationWithFixedResult;
+
+public class TestAbfsHttpOperation {
+
+ @Test
+ public void testForURLs()
+ throws MalformedURLException, UnsupportedEncodingException {
+ testIfMaskedSuccessfully("Where sig is the only query param"
+ ,"http://www.testurl.net?sig=abcd";
+ ,"http://www.testurl.net?sig=XXXX";);
+
+ testIfMaskedSuccessfully("Where sig is the first query param"
+ ,"http://www.testurl.net?sig=abcd&abc=xyz";
+ ,"http://www.testurl.net?sig=XXXX&abc=xyz";);
+
+ testIfMaskedSuccessfully("Where sig is neither first nor last query param"
+ ,"http://www.testurl.net?lmn=abc&sig=abcd&abc=xyz";
+ ,"http://www.testurl.net?lmn=abc&sig=XXXX&abc=xyz";);
+
+ testIfMaskedSuccessfully("Where sig is the last query param"
+ ,"http://www.testurl.net?abc=xyz&sig=abcd";
+ ,"http://www.testurl.net?abc=xyz&sig=XXXX";);
+
+ testIfMaskedSuccessfully("Where sig query param is not present"
Review comment:
query params ending mysig/*sig.
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsHttpOperation.java
##########
@@ -0,0 +1,75 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.apache.hadoop.fs.azurebfs.services;
+
+import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLEncoder;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+
+import static
org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation.getAbfsHttpOperationWithFixedResult;
+
+public class TestAbfsHttpOperation {
+
+ @Test
+ public void testForURLs()
+ throws MalformedURLException, UnsupportedEncodingException {
+ testIfMaskedSuccessfully("Where sig is the only query param"
+ ,"http://www.testurl.net?sig=abcd";
+ ,"http://www.testurl.net?sig=XXXX";);
+
+ testIfMaskedSuccessfully("Where sig is the first query param"
+ ,"http://www.testurl.net?sig=abcd&abc=xyz";
+ ,"http://www.testurl.net?sig=XXXX&abc=xyz";);
+
+ testIfMaskedSuccessfully("Where sig is neither first nor last query param"
+ ,"http://www.testurl.net?lmn=abc&sig=abcd&abc=xyz";
+ ,"http://www.testurl.net?lmn=abc&sig=XXXX&abc=xyz";);
+
+ testIfMaskedSuccessfully("Where sig is the last query param"
+ ,"http://www.testurl.net?abc=xyz&sig=abcd";
+ ,"http://www.testurl.net?abc=xyz&sig=XXXX";);
+
+ testIfMaskedSuccessfully("Where sig query param is not present"
Review comment:
sig in other cases, like caps,
sig as suffix in the param names and values
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
