Merge pull request #1820 from AndySpaven/ISSUE-1745

ISSUE-1745 - Change the buildUrl to handle more cases of path traversal

Author: gracekarina

modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RefUtils.java

@@ -11,6 +11,7 @@
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
@@ -150,48 +151,20 @@ public static String readExternalClasspathRef(String file, RefFormat refFormat,
     }
 
     public static String buildUrl(String rootPath, String relativePath) {
-        String[] rootPathParts = rootPath.split("/");
-        String [] relPathParts = relativePath.split("/");
-
-        if(rootPath == null || relativePath == null) {
-            return null;
-        }
-
-        int trimRoot = 0;
-        int trimRel = 0;
-
-        if(!"".equals(rootPathParts[rootPathParts.length - 1])) {
-            trimRoot = 1;
-        }
-        if("".equals(relPathParts[0])) {
-            trimRel = 1; trimRoot = rootPathParts.length-3;
-        }        
-        for(int i = 0; i < rootPathParts.length; i++) {
-            if("".equals(rootPathParts[i])) {
-                trimRel += 1;
-            }
-            else {
-                break;
-            }
-        }
-        for(int i = 0; i < relPathParts.length; i ++) {
-            if(".".equals(relPathParts[i])) {
-                trimRel += 1;
-            }
-            else if ("..".equals(relPathParts[i])) {
-                trimRel += 1; trimRoot += 1;
-            }
-        }
-
-        String [] outputParts = new String[rootPathParts.length + relPathParts.length - trimRoot - trimRel];
-        System.arraycopy(rootPathParts, 0, outputParts, 0, rootPathParts.length - trimRoot);
-        System.arraycopy(relPathParts,
-                trimRel,
-                outputParts,
-                rootPathParts.length - trimRoot,
-                relPathParts.length - trimRel);
-
-        return StringUtils.join(outputParts, "/");
+      if(rootPath == null || relativePath == null) {
+          return null;
+      }
+
+      try {
+          int until = rootPath.lastIndexOf("/")+1;
+          String root = rootPath.substring(0, until);
+          URL rootUrl = new URL(root);
+          URL finalUrl = new URL(rootUrl, relativePath);
+          return finalUrl.toString();
+      }
+      catch(Exception e) {
+          throw new RuntimeException(e);
+      }
     }
 
     public static String readExternalRef(String file, RefFormat refFormat, List<AuthorizationValue> auths,

modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/util/RefUtilsTest.java

@@ -312,6 +312,13 @@ public void testPathJoin2() {
         assertEquals(RefUtils.buildUrl("http://foo.bar.com/my/dir/file.yaml", "/my/newFile.yaml"), "http://foo.bar.com/my/newFile.yaml");
     }
 
+    @Test
+    public void testPathJoinIssue1745() {
+        assertEquals(RefUtils.buildUrl("http://foo.bar.com/my/dir/file.yaml", "./second/../newFile.yaml"), "http://foo.bar.com/my/dir/newFile.yaml");
+        assertEquals(RefUtils.buildUrl("http://foo.bar.com/my/dir/", "./second/../newFile.yaml"), "http://foo.bar.com/my/dir/newFile.yaml");
+        // This is a little strange in the output (beacuse it has not completely eliminated the ..) but is still correct - paste a similar url into a browser and it resolves it correctly.
+        assertEquals(RefUtils.buildUrl("http://foo.bar.com/my/dir/file.yaml", "/second/../newFile.yaml"), "http://foo.bar.com/second/../newFile.yaml");
+    }
 
     @Test
     public void shouldReturnEmptyExternalPathForInternalReference() {