Author: markt Date: Thu Mar 13 10:42:49 2014 New Revision: 1577103 URL: http://svn.apache.org/r1577103 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56253 When listing resources that are provided by a JAR, fix possible StringIndexOutOfBoundsExceptions. Add some unit tests for this and similar scenarios and fix the additional issues those unit tests identified. Based on a patch by Larry Isaacs.
Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java tomcat/trunk/java/org/apache/catalina/webresources/JarResourceRoot.java tomcat/trunk/java/org/apache/catalina/webresources/LocalStrings.properties tomcat/trunk/test/org/apache/catalina/webresources/AbstractTestResourceSet.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java?rev=1577103&r1=1577102&r2=1577103&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java (original) +++ tomcat/trunk/java/org/apache/catalina/webresources/AbstractArchiveResourceSet.java Thu Mar 13 10:42:49 2014 @@ -75,7 +75,7 @@ public abstract class AbstractArchiveRes String pathInJar = getInternalPath() + path.substring(webAppMount.length()); // Always strip off the leading '/' to get the JAR path - if (pathInJar.charAt(0) == '/') { + if (pathInJar.length() > 0 && pathInJar.charAt(0) == '/') { pathInJar = pathInJar.substring(1); } Iterator<String> entries = jarFileEntries.keySet().iterator(); @@ -128,11 +128,13 @@ public abstract class AbstractArchiveRes getInternalPath() + path.substring(webAppMount.length()); // Always strip off the leading '/' to get the JAR path and make // sure it ends in '/' - if (pathInJar.charAt(pathInJar.length() - 1) != '/') { - pathInJar = pathInJar.substring(1) + '/'; - } - if (pathInJar.charAt(0) == '/') { - pathInJar = pathInJar.substring(1); + if (pathInJar.length() > 0) { + if (pathInJar.charAt(pathInJar.length() - 1) != '/') { + pathInJar = pathInJar.substring(1) + '/'; + } + if (pathInJar.charAt(0) == '/') { + pathInJar = pathInJar.substring(1); + } } Iterator<String> entries = jarFileEntries.keySet().iterator(); @@ -218,6 +220,10 @@ public abstract class AbstractArchiveRes } if (pathInJar.equals("")) { // Special case + // This is a directory resource so the path must end with / + if (!path.endsWith("/")) { + path = path + "/"; + } return new JarResourceRoot(root, new File(getBase()), baseUrlString, path); } else { Modified: tomcat/trunk/java/org/apache/catalina/webresources/JarResourceRoot.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/JarResourceRoot.java?rev=1577103&r1=1577102&r2=1577103&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/webresources/JarResourceRoot.java (original) +++ tomcat/trunk/java/org/apache/catalina/webresources/JarResourceRoot.java Thu Mar 13 10:42:49 2014 @@ -38,16 +38,16 @@ public class JarResourceRoot extends Abs public JarResourceRoot(WebResourceRoot root, File base, String baseUrl, String webAppPath) { super(root, webAppPath); + // Validate the webAppPath before going any further + if (!webAppPath.endsWith("/")) { + throw new IllegalArgumentException(sm.getString( + "jarResourceRoot.invalidWebAppPath", webAppPath)); + } this.base = base; this.baseUrl = "jar:" + baseUrl; // Extract the name from the webAppPath - // Strip any trailing '/' character - String resourceName; - if (webAppPath.endsWith("/")) { - resourceName = webAppPath.substring(0, webAppPath.length() - 1); - } else { - resourceName = webAppPath; - } + // Strip the trailing '/' character + String resourceName = webAppPath.substring(0, webAppPath.length() - 1); int i = resourceName.lastIndexOf('/'); if (i > -1) { resourceName = resourceName.substring(i + 1); Modified: tomcat/trunk/java/org/apache/catalina/webresources/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/LocalStrings.properties?rev=1577103&r1=1577102&r2=1577103&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/webresources/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/catalina/webresources/LocalStrings.properties Thu Mar 13 10:42:49 2014 @@ -35,6 +35,8 @@ fileResource.getUrlFail=Unable to determ jarResource.getInputStreamFail=Unable to obtain an InputStream for the resource [{0}] located in the JAR [{1}] jarResource.getUrlFail=Unable to determine a URL for the resource [{0}] located in the JAR [{1}] +jarResourceRoot.invalidWebAppPath=This resource always refers to a directory so the supplied webAppPath must end with / but the provided webAppPath was [{0}] + standardRoot.checkStateNotStarted=The resources may not be accessed if they are not currently started standardRoot.createInvalidFile=Unable to create WebResourceSet from [{0}] standardRoot.createNoFileResourceSet=The FileResourceSet feature has not yet been implemented Modified: tomcat/trunk/test/org/apache/catalina/webresources/AbstractTestResourceSet.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/webresources/AbstractTestResourceSet.java?rev=1577103&r1=1577102&r2=1577103&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/webresources/AbstractTestResourceSet.java (original) +++ tomcat/trunk/test/org/apache/catalina/webresources/AbstractTestResourceSet.java Thu Mar 13 10:42:49 2014 @@ -66,7 +66,24 @@ public abstract class AbstractTestResour @Test public final void testGetResourceRoot() { - WebResource webResource = resourceRoot.getResource(getMount() + "/"); + doTestGetResourceRoot(true); + } + + @Test + public final void testGetResourceRootNoSlash() { + doTestGetResourceRoot(false); + } + + + private void doTestGetResourceRoot(boolean slash) { + String mount = getMount(); + if (!slash && mount.length() == 0) { + return; + } + mount = mount + (slash ? "/" : ""); + + WebResource webResource = resourceRoot.getResource(mount); + Assert.assertTrue(webResource.isDirectory()); String expected; if (getMount().length() > 0) { @@ -75,7 +92,7 @@ public abstract class AbstractTestResour expected = ""; } Assert.assertEquals(expected, webResource.getName()); - Assert.assertEquals(getMount() + "/", webResource.getWebappPath()); + Assert.assertEquals(mount + (!slash ? "/" : ""), webResource.getWebappPath()); } @Test @@ -135,7 +152,22 @@ public abstract class AbstractTestResour @Test public final void testListRoot() { - String[] results = resourceRoot.list(getMount() + "/"); + doTestListRoot(true); + } + + @Test + public final void testListRootNoSlash() { + doTestListRoot(false); + } + + + private void doTestListRoot(boolean slash) { + String mount = getMount(); + if (!slash && mount.length() == 0) { + return; + } + + String[] results = resourceRoot.list(mount + (slash ? "/" : "")); Set<String> expected = new HashSet<>(); expected.add("d1"); @@ -192,7 +224,22 @@ public abstract class AbstractTestResour @Test public final void testListWebAppPathsRoot() { - Set<String> results = resourceRoot.listWebAppPaths(getMount() + "/"); + doTestListWebAppPathsRoot(true); + } + + @Test + public final void testListWebAppPathsRootNoSlash() { + doTestListWebAppPathsRoot(false); + } + + + private void doTestListWebAppPathsRoot(boolean slash) { + String mount = getMount(); + if (!slash && mount.length() == 0) { + return; + } + + Set<String> results = resourceRoot.listWebAppPaths(mount + (slash ? "/" : "")); Set<String> expected = new HashSet<>(); expected.add(getMount() + "/d1/"); Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1577103&r1=1577102&r2=1577103&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu Mar 13 10:42:49 2014 @@ -112,6 +112,12 @@ <bug>56246</bug>: Fix NullPointerException in MemoryRealm when authenticating an unknown user. (markt) </fix> + <fix> + <bug>56253</bug>: When listing resources that are provided by a JAR, fix + possible <code>StringIndexOutOfBoundsException</code>s. Add some unit + tests for this and similar scenarios and fix the additional issues those + unit tests identified. Based on a patch by Larry Isaacs. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org