You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2015/07/26 14:40:56 UTC

mina-sshd git commit: [SSHD-538] Handle correctly paths with double-slashes in them

Repository: mina-sshd
Updated Branches:
  refs/heads/master 54b56b844 -> c9524a24a


[SSHD-538] Handle correctly paths with double-slashes in them

Fixed subtle bug that was related to normalization issues


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/c9524a24
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/c9524a24
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/c9524a24

Branch: refs/heads/master
Commit: c9524a24a651b7b9af996e4673d0a541bc5106bb
Parents: 54b56b8
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Sun Jul 26 15:40:46 2015 +0300
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Sun Jul 26 15:40:46 2015 +0300

----------------------------------------------------------------------
 .../apache/sshd/common/util/SelectorUtils.java  |  90 +++++++++++++++-
 .../server/subsystem/sftp/SftpSubsystem.java    |   6 +-
 .../sshd/client/subsystem/sftp/SftpTest.java    |  33 ++++++
 .../sshd/common/util/SelectorUtilsTest.java     | 102 +++++++++++++++++++
 4 files changed, 226 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/c9524a24/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java
index f38a75b..00af8b2 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/util/SelectorUtils.java
@@ -533,7 +533,6 @@ public final class SelectorUtils {
         return ret;
     }
 
-
     /**
      * Normalizes the path by removing '.', '..' and double separators (e.g. '//')
      *
@@ -594,6 +593,95 @@ public final class SelectorUtils {
     }
 
     /**
+     * Applies the &quot;slashification&quot; rules as specified in
+     * <A HREF="http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_266">Single Unix Specification version 3, section 3.266</A>
+     * and <A HREF="http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap04.html#tag_04_11">section 4.11 - Pathname resolution</A>
+     * @param path The original path - ignored if {@code null}/empty or does
+     * not contain any slashes
+     * @param sepChar The &quot;slash&quot; character
+     * @return The effective path - may be same as input if no changes required
+     */
+    public static String applySlashifyRules(String path, char sepChar) {
+        if (GenericUtils.isEmpty(path)) {
+            return path;
+        }
+        
+        int curPos = path.indexOf(sepChar);
+        if (curPos < 0) {
+            return path;    // no slashes to handle
+        }
+        
+        int lastPos = 0;
+        StringBuilder sb = null;
+        while (curPos < path.length()) {
+            curPos++;   // skip the 1st '/'
+            
+            /*
+             * As per Single Unix Specification version 3, section 3.266:
+             * 
+             *      Multiple successive slashes are considered to be the
+             *      same as one slash
+             */
+            int nextPos = curPos;
+            while ((nextPos < path.length()) && (path.charAt(nextPos) == sepChar)) {
+                nextPos++;
+            }
+            
+            /*
+             * At this stage, nextPos is the first non-slash character after a
+             * possibly 'seqLen' sequence of consecutive slashes.
+             */
+            int seqLen = nextPos - curPos;
+            if (seqLen > 0) {
+                if (sb == null) {
+                    sb = new StringBuilder(path.length() - seqLen);
+                }
+                
+                if (lastPos < curPos) {
+                    String clrText = path.substring(lastPos, curPos);
+                    sb.append(clrText);
+                }
+                
+                lastPos = nextPos;
+            }
+            
+            if (nextPos >= path.length()) {
+                break;  // no more data
+            }
+            
+            curPos = path.indexOf(sepChar, nextPos);
+            if (curPos < nextPos) {
+                break;  // no more slashes
+            }
+        }
+
+        // check if any leftovers for the modified path
+        if (sb != null) {
+            if (lastPos < path.length()) {
+                String clrText = path.substring(lastPos);
+                sb.append(clrText);
+            }
+            
+            path = sb.toString();
+        }
+
+        /*
+         * At this point we know for sure that 'path' contains only SINGLE
+         * slashes. According to section 4.11 - Pathname resolution
+         * 
+         *      A pathname that contains at least one non-slash character
+         *      and that ends with one or more trailing slashes shall be
+         *      resolved as if a single dot character ( '.' ) were appended
+         *      to the pathname.
+         */
+        if ((path.length() > 1) && (path.charAt(path.length() - 1) == sepChar)) {
+            return path + ".";
+        } else {
+            return path;
+        }
+    }
+
+    /**
      * Converts a possibly '/' separated path to a local path. <B>Note:</B>
      * takes special care of Windows drive paths - e.g., {@code C:}
      * by converting them to &quot;C:\&quot;

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/c9524a24/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java b/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
index fa63181..103c5db 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
@@ -2927,15 +2927,13 @@ public class SftpSubsystem extends AbstractLoggingBean implements Command, Runna
 
     protected Path resolveFile(String remotePath) throws IOException, InvalidPathException {
         // In case double slashes and other patterns are used 
-        String path = SelectorUtils.normalizePath(remotePath, "/");
-        String localPath = SelectorUtils.translateToLocalPath(path);
-
+        String path = SelectorUtils.applySlashifyRules(remotePath, '/');
         // In case we are running on Windows
+        String localPath = SelectorUtils.translateToLocalPath(path);
         Path p = defaultDir.resolve(localPath);
         if (log.isTraceEnabled()) {
             log.trace("resolveFile({}) {}", remotePath, p);
         }
-        
         return p;
     }
 }

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/c9524a24/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
index 82e5578..e7af9ee 100644
--- a/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/client/subsystem/sftp/SftpTest.java
@@ -31,8 +31,10 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.Iterator;
@@ -49,7 +51,9 @@ import org.apache.sshd.client.subsystem.sftp.extensions.BuiltinSftpClientExtensi
 import org.apache.sshd.client.subsystem.sftp.extensions.SftpClientExtension;
 import org.apache.sshd.common.Factory;
 import org.apache.sshd.common.FactoryManager;
+import org.apache.sshd.common.file.FileSystemFactory;
 import org.apache.sshd.common.random.Random;
+import org.apache.sshd.common.session.Session;
 import org.apache.sshd.common.subsystem.sftp.SftpConstants;
 import org.apache.sshd.common.subsystem.sftp.extensions.ParserUtils;
 import org.apache.sshd.common.subsystem.sftp.extensions.Supported2Parser.Supported2;
@@ -109,6 +113,35 @@ public class SftpTest extends AbstractSftpClientTestSupport {
         Thread.sleep(5 * 60000);
     }
 
+    @Test   // see extra fix for SSHD-538
+    public void testNavigateBeyondRootFolder() throws Exception {
+        Path rootLocation = Paths.get(OsUtils.isUNIX() ? "/" : "C:\\");
+        final FileSystem fsRoot = rootLocation.getFileSystem();
+        sshd.setFileSystemFactory(new FileSystemFactory() {
+                @Override
+                public FileSystem createFileSystem(Session session) throws IOException {
+                    return fsRoot;
+                }
+            });
+
+        try (SshClient client = SshClient.setUpDefaultClient()) {
+            client.start();
+
+            try (ClientSession session = client.connect(getCurrentTestName(), "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) {
+                session.addPasswordIdentity(getCurrentTestName());
+                session.auth().verify(5L, TimeUnit.SECONDS);
+
+                try (SftpClient sftp = session.createSftpClient()) {
+                    String rootDir = sftp.canonicalPath("/");
+                    String upDir = sftp.canonicalPath(rootDir + "/..");
+                    assertEquals("Mismatched root dir parent", rootDir, upDir);
+                }
+            } finally {
+                client.stop();
+            }
+        }
+    }
+
     @Test
     public void testNormalizeRemoteRootValues() throws Exception {
         try (SshClient client = SshClient.setUpDefaultClient()) {

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/c9524a24/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java b/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java
new file mode 100644
index 0000000..82d6367
--- /dev/null
+++ b/sshd-core/src/test/java/org/apache/sshd/common/util/SelectorUtilsTest.java
@@ -0,0 +1,102 @@
+/*
+ * 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.sshd.common.util;
+
+import java.util.Random;
+
+import org.apache.sshd.util.BaseTestSupport;
+import org.junit.FixMethodOrder;
+import org.junit.Test;
+import org.junit.runners.MethodSorters;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
+public class SelectorUtilsTest extends BaseTestSupport {
+    public SelectorUtilsTest() {
+        super();
+    }
+
+    @Test
+    public void testApplySlashifyRules() {
+        for (String expected : new String[] {
+                null, "", getCurrentTestName(), getClass().getSimpleName() + "/" + getCurrentTestName(),
+                "/" + getClass().getSimpleName(), "/" + getClass().getSimpleName() + "/" + getCurrentTestName()
+            }) {
+            String actual = SelectorUtils.applySlashifyRules(expected, '/');
+            assertSame("Mismatched results for '" + expected + "'", expected, actual);
+        }
+        
+        String[] comps = { getClass().getSimpleName(),  getCurrentTestName() };
+        Random rnd = new Random(System.nanoTime());
+        StringBuilder sb = new StringBuilder(Byte.MAX_VALUE);
+        for (int index = 0; index < Long.SIZE; index++) {
+            if (sb.length() > 0) {
+                sb.setLength(0);        // start from scratch
+            }
+            
+            boolean prepend = rnd.nextBoolean();
+            if (prepend) {
+                slashify(sb, rnd);
+            }
+
+            sb.append(comps[0]);
+            for (int j = 1; j < comps.length; j++) {
+                slashify(sb, rnd);
+                sb.append(comps[j]);
+            }
+            
+            boolean append = rnd.nextBoolean();
+            if (append) {
+                slashify(sb, rnd);
+            }
+            
+            String path = sb.toString();
+            sb.setLength(0);
+            if (prepend) {
+                sb.append('/');
+            }
+
+            sb.append(comps[0]);
+            for (int j = 1; j < comps.length; j++) {
+                sb.append('/').append(comps[j]);
+            }
+            
+            if (append) {
+                sb.append('/').append('.');
+            }
+            
+            String expected = sb.toString();
+            String actual = SelectorUtils.applySlashifyRules(path, '/');
+            assertEquals("Mismatched results for path=" + path, expected, actual);
+        }
+    }
+    
+
+    private static int slashify(StringBuilder sb, Random rnd) {
+        int slashes = 1 /* at least one slash */ + rnd.nextInt(Byte.SIZE);
+        for (int k = 0; k < slashes; k++) {
+            sb.append('/');
+        }
+
+        return slashes;
+    }
+
+}