[ 
https://issues.apache.org/jira/browse/HADOOP-17311?focusedWorklogId=506311&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-506311
 ]

ASF GitHub Bot logged work on HADOOP-17311:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 29/Oct/20 17:38
            Start Date: 29/Oct/20 17:38
    Worklog Time Spent: 10m 
      Work Description: steveloughran commented on a change in pull request 
#2422:
URL: https://github.com/apache/hadoop/pull/2422#discussion_r514443770



##########
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:
       While you are there
   * add a catch for UnknownHostException
   * move from String.format to Log.warn("unknown host {}", 
httpOperation,getHost()
   

##########
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:
       This is complicated enough it could be pulled out into a static method, 
and so its handling fully tested in (new) Unit tests, as well as in the ITests.

##########
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:
       use LambdaTestUtils.intercept(). Not only simpler, it will (correctly) 
fail if the rest operation didn't actually raise an exception

##########
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:
       now all shaded I'm afraid. Making backporting harder already




----------------------------------------------------------------
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]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 506311)
    Time Spent: 50m  (was: 40m)

> ABFS: Logs should redact SAS signature
> --------------------------------------
>
>                 Key: HADOOP-17311
>                 URL: https://issues.apache.org/jira/browse/HADOOP-17311
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/azure, security
>    Affects Versions: 3.3.0
>            Reporter: Sneha Vijayarajan
>            Assignee: Bilahari T H
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> Signature part of the SAS should be redacted for security purposes.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to