This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 4785433 Use java.nio.file.Path for consistent sub-directory checking 4785433 is described below commit 4785433a226a20df6acbea49296e1ce7e23de453 Author: Mark Thomas <[hidden email]> AuthorDate: Wed Jan 20 13:28:57 2021 +0000 Use java.nio.file.Path for consistent sub-directory checking --- .../apache/catalina/servlets/DefaultServlet.java | 2 +- java/org/apache/catalina/session/FileStore.java | 2 +- java/org/apache/catalina/startup/ContextConfig.java | 3 ++- java/org/apache/catalina/startup/ExpandWar.java | 21 +++++++-------------- java/org/apache/catalina/startup/HostConfig.java | 3 +-- webapps/docs/changelog.xml | 4 ++++ 6 files changed, 16 insertions(+), 19 deletions(-) diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java index 2aca1b2..e0b6711 100644 --- a/java/org/apache/catalina/servlets/DefaultServlet.java +++ b/java/org/apache/catalina/servlets/DefaultServlet.java @@ -2137,7 +2137,7 @@ public class DefaultServlet extends HttpServlet { // First check that the resulting path is under the provided base try { - if (!candidate.getCanonicalPath().startsWith(base.getCanonicalPath())) { + if (!candidate.getCanonicalFile().toPath().startsWith(base.getCanonicalFile().toPath())) { return null; } } catch (IOException ioe) { diff --git a/java/org/apache/catalina/session/FileStore.java b/java/org/apache/catalina/session/FileStore.java index cf3ea88..cac6027 100644 --- a/java/org/apache/catalina/session/FileStore.java +++ b/java/org/apache/catalina/session/FileStore.java @@ -351,7 +351,7 @@ public final class FileStore extends StoreBase { File file = new File(storageDir, filename); // Check the file is within the storage directory - if (!file.getCanonicalPath().startsWith(storageDir.getCanonicalPath())) { + if (!file.getCanonicalFile().toPath().startsWith(storageDir.getCanonicalFile().toPath())) { log.warn(sm.getString("fileStore.invalid", file.getPath(), id)); return null; } diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java index 2e02b7f..5926bfc 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -858,7 +858,8 @@ public class ContextConfig implements LifecycleListener { String docBaseCanonical = docBaseAbsoluteFile.getCanonicalPath(); // Re-calculate now docBase is a canonical path - boolean docBaseCanonicalInAppBase = docBaseCanonical.startsWith(appBase.getPath() + File.separatorChar); + boolean docBaseCanonicalInAppBase = + docBaseAbsoluteFile.getCanonicalFile().toPath().startsWith(appBase.toPath()); String docBase; if (docBaseCanonicalInAppBase) { docBase = docBaseCanonical.substring(appBase.getPath().length()); diff --git a/java/org/apache/catalina/startup/ExpandWar.java b/java/org/apache/catalina/startup/ExpandWar.java index a76acc2..ebc3e46 100644 --- a/java/org/apache/catalina/startup/ExpandWar.java +++ b/java/org/apache/catalina/startup/ExpandWar.java @@ -26,6 +26,7 @@ import java.net.JarURLConnection; import java.net.URL; import java.net.URLConnection; import java.nio.channels.FileChannel; +import java.nio.file.Path; import java.util.Enumeration; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -116,10 +117,7 @@ public class ExpandWar { } // Expand the WAR into the new document base directory - String canonicalDocBasePrefix = docBase.getCanonicalPath(); - if (!canonicalDocBasePrefix.endsWith(File.separator)) { - canonicalDocBasePrefix += File.separator; - } + Path canonicalDocBasePath = docBase.getCanonicalFile().toPath(); // Creating war tracker parent (normally META-INF) File warTrackerParent = warTracker.getParentFile(); @@ -134,14 +132,13 @@ public class ExpandWar { JarEntry jarEntry = jarEntries.nextElement(); String name = jarEntry.getName(); File expandedFile = new File(docBase, name); - if (!expandedFile.getCanonicalPath().startsWith( - canonicalDocBasePrefix)) { + if (!expandedFile.getCanonicalFile().toPath().startsWith(canonicalDocBasePath)) { // Trying to expand outside the docBase // Throw an exception to stop the deployment throw new IllegalArgumentException( sm.getString("expandWar.illegalPath",war, name, expandedFile.getCanonicalPath(), - canonicalDocBasePrefix)); + canonicalDocBasePath)); } int last = name.lastIndexOf('/'); if (last >= 0) { @@ -217,10 +214,7 @@ public class ExpandWar { File docBase = new File(host.getAppBaseFile(), pathname); // Calculate the document base directory - String canonicalDocBasePrefix = docBase.getCanonicalPath(); - if (!canonicalDocBasePrefix.endsWith(File.separator)) { - canonicalDocBasePrefix += File.separator; - } + Path canonicalDocBasePath = docBase.getCanonicalFile().toPath(); JarURLConnection juc = (JarURLConnection) war.openConnection(); juc.setUseCaches(false); try (JarFile jarFile = juc.getJarFile()) { @@ -229,14 +223,13 @@ public class ExpandWar { JarEntry jarEntry = jarEntries.nextElement(); String name = jarEntry.getName(); File expandedFile = new File(docBase, name); - if (!expandedFile.getCanonicalPath().startsWith( - canonicalDocBasePrefix)) { + if (!expandedFile.getCanonicalFile().toPath().startsWith(canonicalDocBasePath)) { // Entry located outside the docBase // Throw an exception to stop the deployment throw new IllegalArgumentException( sm.getString("expandWar.illegalPath",war, name, expandedFile.getCanonicalPath(), - canonicalDocBasePrefix)); + canonicalDocBasePath)); } } } catch (IOException e) { diff --git a/java/org/apache/catalina/startup/HostConfig.java b/java/org/apache/catalina/startup/HostConfig.java index e3b3d13..fd81ad2 100644 --- a/java/org/apache/catalina/startup/HostConfig.java +++ b/java/org/apache/catalina/startup/HostConfig.java @@ -598,8 +598,7 @@ public class HostConfig implements LifecycleListener { docBase = new File(host.getAppBaseFile(), context.getDocBase()); } // If external docBase, register .xml as redeploy first - if (!docBase.getCanonicalPath().startsWith( - host.getAppBaseFile().getAbsolutePath() + File.separator)) { + if (!docBase.getCanonicalFile().toPath().startsWith(host.getAppBaseFile().toPath())) { isExternal = true; deployedApp.redeployResources.put( contextXml.getAbsolutePath(), diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 9eba6ae..effbe27 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -225,6 +225,10 @@ <update> Migrate to new code signing service. (markt) </update> + <scode> + Use <code>java.nio.file.Path</code> to test for one directory being a + sub-directory of another in a consistent way. (markt) + </scode> </changelog> </subsection> </section> --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
Mark,
On 1/20/21 08:56, [hidden email] wrote: > This is an automated email from the ASF dual-hosted git repository. > > markt pushed a commit to branch 9.0.x > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > The following commit(s) were added to refs/heads/9.0.x by this push: > new 4785433 Use java.nio.file.Path for consistent sub-directory checking Good call. No more File.separator yay! -chris > 4785433 is described below > > commit 4785433a226a20df6acbea49296e1ce7e23de453 > Author: Mark Thomas <[hidden email]> > AuthorDate: Wed Jan 20 13:28:57 2021 +0000 > > Use java.nio.file.Path for consistent sub-directory checking > --- > .../apache/catalina/servlets/DefaultServlet.java | 2 +- > java/org/apache/catalina/session/FileStore.java | 2 +- > java/org/apache/catalina/startup/ContextConfig.java | 3 ++- > java/org/apache/catalina/startup/ExpandWar.java | 21 +++++++-------------- > java/org/apache/catalina/startup/HostConfig.java | 3 +-- > webapps/docs/changelog.xml | 4 ++++ > 6 files changed, 16 insertions(+), 19 deletions(-) > > diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java > index 2aca1b2..e0b6711 100644 > --- a/java/org/apache/catalina/servlets/DefaultServlet.java > +++ b/java/org/apache/catalina/servlets/DefaultServlet.java > @@ -2137,7 +2137,7 @@ public class DefaultServlet extends HttpServlet { > > // First check that the resulting path is under the provided base > try { > - if (!candidate.getCanonicalPath().startsWith(base.getCanonicalPath())) { > + if (!candidate.getCanonicalFile().toPath().startsWith(base.getCanonicalFile().toPath())) { > return null; > } > } catch (IOException ioe) { > diff --git a/java/org/apache/catalina/session/FileStore.java b/java/org/apache/catalina/session/FileStore.java > index cf3ea88..cac6027 100644 > --- a/java/org/apache/catalina/session/FileStore.java > +++ b/java/org/apache/catalina/session/FileStore.java > @@ -351,7 +351,7 @@ public final class FileStore extends StoreBase { > File file = new File(storageDir, filename); > > // Check the file is within the storage directory > - if (!file.getCanonicalPath().startsWith(storageDir.getCanonicalPath())) { > + if (!file.getCanonicalFile().toPath().startsWith(storageDir.getCanonicalFile().toPath())) { > log.warn(sm.getString("fileStore.invalid", file.getPath(), id)); > return null; > } > diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java > index 2e02b7f..5926bfc 100644 > --- a/java/org/apache/catalina/startup/ContextConfig.java > +++ b/java/org/apache/catalina/startup/ContextConfig.java > @@ -858,7 +858,8 @@ public class ContextConfig implements LifecycleListener { > String docBaseCanonical = docBaseAbsoluteFile.getCanonicalPath(); > > // Re-calculate now docBase is a canonical path > - boolean docBaseCanonicalInAppBase = docBaseCanonical.startsWith(appBase.getPath() + File.separatorChar); > + boolean docBaseCanonicalInAppBase = > + docBaseAbsoluteFile.getCanonicalFile().toPath().startsWith(appBase.toPath()); > String docBase; > if (docBaseCanonicalInAppBase) { > docBase = docBaseCanonical.substring(appBase.getPath().length()); > diff --git a/java/org/apache/catalina/startup/ExpandWar.java b/java/org/apache/catalina/startup/ExpandWar.java > index a76acc2..ebc3e46 100644 > --- a/java/org/apache/catalina/startup/ExpandWar.java > +++ b/java/org/apache/catalina/startup/ExpandWar.java > @@ -26,6 +26,7 @@ import java.net.JarURLConnection; > import java.net.URL; > import java.net.URLConnection; > import java.nio.channels.FileChannel; > +import java.nio.file.Path; > import java.util.Enumeration; > import java.util.jar.JarEntry; > import java.util.jar.JarFile; > @@ -116,10 +117,7 @@ public class ExpandWar { > } > > // Expand the WAR into the new document base directory > - String canonicalDocBasePrefix = docBase.getCanonicalPath(); > - if (!canonicalDocBasePrefix.endsWith(File.separator)) { > - canonicalDocBasePrefix += File.separator; > - } > + Path canonicalDocBasePath = docBase.getCanonicalFile().toPath(); > > // Creating war tracker parent (normally META-INF) > File warTrackerParent = warTracker.getParentFile(); > @@ -134,14 +132,13 @@ public class ExpandWar { > JarEntry jarEntry = jarEntries.nextElement(); > String name = jarEntry.getName(); > File expandedFile = new File(docBase, name); > - if (!expandedFile.getCanonicalPath().startsWith( > - canonicalDocBasePrefix)) { > + if (!expandedFile.getCanonicalFile().toPath().startsWith(canonicalDocBasePath)) { > // Trying to expand outside the docBase > // Throw an exception to stop the deployment > throw new IllegalArgumentException( > sm.getString("expandWar.illegalPath",war, name, > expandedFile.getCanonicalPath(), > - canonicalDocBasePrefix)); > + canonicalDocBasePath)); > } > int last = name.lastIndexOf('/'); > if (last >= 0) { > @@ -217,10 +214,7 @@ public class ExpandWar { > File docBase = new File(host.getAppBaseFile(), pathname); > > // Calculate the document base directory > - String canonicalDocBasePrefix = docBase.getCanonicalPath(); > - if (!canonicalDocBasePrefix.endsWith(File.separator)) { > - canonicalDocBasePrefix += File.separator; > - } > + Path canonicalDocBasePath = docBase.getCanonicalFile().toPath(); > JarURLConnection juc = (JarURLConnection) war.openConnection(); > juc.setUseCaches(false); > try (JarFile jarFile = juc.getJarFile()) { > @@ -229,14 +223,13 @@ public class ExpandWar { > JarEntry jarEntry = jarEntries.nextElement(); > String name = jarEntry.getName(); > File expandedFile = new File(docBase, name); > - if (!expandedFile.getCanonicalPath().startsWith( > - canonicalDocBasePrefix)) { > + if (!expandedFile.getCanonicalFile().toPath().startsWith(canonicalDocBasePath)) { > // Entry located outside the docBase > // Throw an exception to stop the deployment > throw new IllegalArgumentException( > sm.getString("expandWar.illegalPath",war, name, > expandedFile.getCanonicalPath(), > - canonicalDocBasePrefix)); > + canonicalDocBasePath)); > } > } > } catch (IOException e) { > diff --git a/java/org/apache/catalina/startup/HostConfig.java b/java/org/apache/catalina/startup/HostConfig.java > index e3b3d13..fd81ad2 100644 > --- a/java/org/apache/catalina/startup/HostConfig.java > +++ b/java/org/apache/catalina/startup/HostConfig.java > @@ -598,8 +598,7 @@ public class HostConfig implements LifecycleListener { > docBase = new File(host.getAppBaseFile(), context.getDocBase()); > } > // If external docBase, register .xml as redeploy first > - if (!docBase.getCanonicalPath().startsWith( > - host.getAppBaseFile().getAbsolutePath() + File.separator)) { > + if (!docBase.getCanonicalFile().toPath().startsWith(host.getAppBaseFile().toPath())) { > isExternal = true; > deployedApp.redeployResources.put( > contextXml.getAbsolutePath(), > diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml > index 9eba6ae..effbe27 100644 > --- a/webapps/docs/changelog.xml > +++ b/webapps/docs/changelog.xml > @@ -225,6 +225,10 @@ > <update> > Migrate to new code signing service. (markt) > </update> > + <scode> > + Use <code>java.nio.file.Path</code> to test for one directory being a > + sub-directory of another in a consistent way. (markt) > + </scode> > </changelog> > </subsection> > </section> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
Free forum by Nabble | Edit this page |