Merge pull request #182 from pdowler/master

cadc-util: include base/part of URL in RemoteServiceException (partially resolves issue #91)
opencadc · Jan 27, 2022 · 2754239 · 2754239
2 parents 0d20f60 + cb4e5e3
commit 2754239
Show file tree

Hide file tree

Showing 3 changed files with 132 additions and 5 deletions.
diff --git a/cadc-util/build.gradle b/cadc-util/build.gradle
@@ -16,7 +16,7 @@ sourceCompatibility = 1.8
 
 group = 'org.opencadc'
 
-version = '1.5.10'
+version = '1.5.11'
 
 description = 'OpenCADC core utility library'
 def git_url = 'https://github.com/opencadc/core'

diff --git a/cadc-util/src/main/java/ca/nrc/cadc/net/HttpTransfer.java b/cadc-util/src/main/java/ca/nrc/cadc/net/HttpTransfer.java
@@ -84,7 +84,6 @@
 import ca.nrc.cadc.util.FileMetadata;
 import ca.nrc.cadc.util.HexUtil;
 import ca.nrc.cadc.util.StringUtil;
-
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
@@ -109,11 +108,9 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
-
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
 import javax.security.auth.Subject;
-
 import org.apache.log4j.Logger;
 
 /**
@@ -898,7 +895,8 @@ protected void checkErrors(URL url, HttpURLConnection conn)
                 throw new ExpectationFailedException(responseBody);
 
             case HttpURLConnection.HTTP_INTERNAL_ERROR:
-                throw new RemoteServiceException(responseBody);
+                String loggableURL = toLoggableString(url);
+                throw new RemoteServiceException("url=" + loggableURL + " msg=" + responseBody);
 
             case HttpURLConnection.HTTP_UNAVAILABLE:
                 throw new TransientException(responseBody);
@@ -907,6 +905,61 @@ protected void checkErrors(URL url, HttpURLConnection conn)
                 throw new IOException(responseBody);
         }
     }
+
+    // reduce url to minimal (max 2 path components) so they don't
+    // contain request-specific content; goal is to capture which service
+    // was called, not the details of the call
+    static String toLoggableString(URL url) {
+        try {
+            StringBuilder sb = new StringBuilder();
+
+            String surl = url.toExternalForm();
+
+            String server = url.getHost();
+            int i = surl.indexOf(server);
+            int j = surl.indexOf("/", i);
+            if (j == -1) {
+                return surl;
+            }
+
+            String base = surl.substring(0, j);
+            log.debug("base: " + base);
+            sb.append(base);
+
+            String path = url.getPath();
+            path = path.substring(1);
+
+            String[] ss = path.split("/");
+            log.debug("path: " + path + " " + ss.length);
+            if (ss.length > 0) {
+                sb.append("/").append(ss[0]);
+            }
+            if (ss.length > 1) {
+                sb.append("/").append(ss[1]);
+            }
+
+            if (ss.length > 2) {
+                // was truncated
+                sb.append("/...");
+            } else {
+                boolean trailingSlash = path.endsWith("/");
+                if (trailingSlash) {
+                    sb.append("/");
+                }
+            }
+
+            String query = url.getQuery();
+            if (query != null && !query.isEmpty()) {
+                // was truncated
+                sb.append("?...");
+            }
+
+            return sb.toString();
+        } catch (Exception oops) {
+            // never want an error message related method to cause a failure
+            return "to-loggable-failed-bug";
+        }
+    }
 
     protected void checkRedirects(URL url, HttpURLConnection conn) 
         throws ResourceNotFoundException, IOException {

diff --git a/cadc-util/src/test/java/ca/nrc/cadc/net/HttpTransferTest.java b/cadc-util/src/test/java/ca/nrc/cadc/net/HttpTransferTest.java
@@ -207,6 +207,80 @@ public void testConstructors() {
         }
     }
 
+    @Test
+    public void testGetLoggableURL() {
+        log.debug("TEST: testGetLoggableURL");
+        try {
+            // no trailing /
+            URL in = new URL("https://www.example.net");
+            String exp = "https://www.example.net";
+            String act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            // with trailing /
+            in = new URL("https://www.example.net/");
+            exp = "https://www.example.net/";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            // one path comp
+            in = new URL("https://www.example.net/foo");
+            exp = "https://www.example.net/foo";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            // one path comp; trailing /
+            in = new URL("https://www.example.net/foo/");
+            exp = "https://www.example.net/foo/";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            in = new URL("https://www.example.net/foo?BAR=1");
+            exp = "https://www.example.net/foo?...";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            in = new URL("https://www.example.net/foo/?BAR=1");
+            exp = "https://www.example.net/foo/?...";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            in = new URL("https://www.example.net/foo/bar?BAZ=1");
+            exp = "https://www.example.net/foo/bar?...";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            in = new URL("https://www.example.net/foo/bar/?BAZ=1");
+            exp = "https://www.example.net/foo/bar/?...";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            in = new URL("https://www.example.net/foo/bar/1234567890?BAZ=1");
+            exp = "https://www.example.net/foo/bar/...?...";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+            in = new URL("https://www.example.net/foo/bar/1234567890/?BAZ=1");
+            exp = "https://www.example.net/foo/bar/...?...";
+            act = HttpTransfer.toLoggableString(in);
+            log.info("in: " + in + " loggable: " + act);
+            Assert.assertEquals(exp, act);
+
+        } catch (Exception unexpected) {
+            log.error("unexpected exception", unexpected);
+            Assert.fail("unexpected exception: " + unexpected);
+        }
+    }
+
     @Test
     public void testBufferSize() throws Exception {
         log.debug("TEST: testBufferSize");