Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoakim Erdfelt2016-03-17 15:00:18 -0400
committerJoakim Erdfelt2016-03-17 19:13:07 -0400
commit05691e16463e02b8376d05c8416722e437e86470 (patch)
tree18ca33b1908635b514f730e1eca0b73ebf41d4cf
parentf8626ecb3dd5910c7d32dadb04df2b38e9d2d89d (diff)
downloadorg.eclipse.jetty.project-05691e16463e02b8376d05c8416722e437e86470.tar.gz
org.eclipse.jetty.project-05691e16463e02b8376d05c8416722e437e86470.tar.xz
org.eclipse.jetty.project-05691e16463e02b8376d05c8416722e437e86470.zip
Issue #438 - File and Path Resources with control characters should be rejected
+ Adding testcases + Cleaning up unit tests, adding more + Fixing one testcase related to FileResource.addPath() + Adding validation of filesystem paths Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
-rw-r--r--jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java22
-rw-r--r--jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java13
-rw-r--r--jetty-util/src/test/java/org/eclipse/jetty/util/resource/AbstractFSResourceTest.java95
-rw-r--r--jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileResourceTest.java15
4 files changed, 123 insertions, 22 deletions
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java
index d2f07c93a1..5ecc6c5e12 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/FileResource.java
@@ -29,8 +29,11 @@ import java.net.URL;
import java.net.URLConnection;
import java.nio.channels.FileChannel;
import java.nio.channels.ReadableByteChannel;
+import java.nio.file.InvalidPathException;
import java.nio.file.StandardOpenOption;
import java.security.Permission;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.URIUtil;
@@ -50,6 +53,7 @@ import org.eclipse.jetty.util.log.Logger;
public class FileResource extends Resource
{
private static final Logger LOG = Log.getLogger(FileResource.class);
+ private static final Pattern CNTRL_PATTERN = Pattern.compile("\\p{Cntrl}");
/* ------------------------------------------------------------ */
private final File _file;
@@ -65,6 +69,7 @@ public class FileResource extends Resource
{
// Try standard API to convert URL to file.
file =new File(url.toURI());
+ assertValidPath(file.toString());
}
catch (URISyntaxException e)
{
@@ -108,6 +113,7 @@ public class FileResource extends Resource
_file=file;
URI file_uri=_file.toURI();
_uri=normalizeURI(_file,uri);
+ assertValidPath(file.toString());
// Is it a URI alias?
if (!URIUtil.equalsIgnoreEncodings(_uri,file_uri.toString()))
@@ -119,6 +125,7 @@ public class FileResource extends Resource
/* -------------------------------------------------------- */
FileResource(File file)
{
+ assertValidPath(file.toString());
_file=file;
_uri=normalizeURI(_file,_file.toURI());
_alias=checkFileAlias(_file);
@@ -179,6 +186,7 @@ public class FileResource extends Resource
public Resource addPath(String path)
throws IOException,MalformedURLException
{
+ assertValidPath(path);
path = org.eclipse.jetty.util.URIUtil.canonicalPath(path);
if (path==null)
@@ -204,13 +212,21 @@ public class FileResource extends Resource
}
catch(final URISyntaxException e)
{
- throw new MalformedURLException(){{initCause(e);}};
+ throw new InvalidPathException(path, e.getMessage());
}
return new FileResource(uri);
}
-
-
+
+ private void assertValidPath(String path)
+ {
+ Matcher mat = CNTRL_PATTERN.matcher(path);
+ if(mat.find())
+ {
+ throw new InvalidPathException(path, "Invalid Character at index " + mat.start());
+ }
+ }
+
/* ------------------------------------------------------------ */
@Override
public URI getAlias()
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java
index 5d0c7bc4ab..68ee7628e0 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java
@@ -38,6 +38,8 @@ import java.nio.file.StandardOpenOption;
import java.nio.file.attribute.FileTime;
import java.util.ArrayList;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
@@ -48,6 +50,7 @@ import org.eclipse.jetty.util.log.Logger;
public class PathResource extends Resource
{
private static final Logger LOG = Log.getLogger(PathResource.class);
+ private static final Pattern CNTRL_PATTERN = Pattern.compile("\\p{Cntrl}");
private final Path path;
private final URI uri;
@@ -61,6 +64,7 @@ public class PathResource extends Resource
public PathResource(Path path)
{
this.path = path;
+ assertValidPath(path);
this.uri = this.path.toUri();
}
@@ -110,6 +114,15 @@ public class PathResource extends Resource
return new PathResource(this.path.getFileSystem().getPath(path.toString(), apath));
}
+ private void assertValidPath(Path path)
+ {
+ Matcher mat = CNTRL_PATTERN.matcher(path.toString());
+ if(mat.find())
+ {
+ throw new InvalidPathException(path.toString(), "Invalid Character at index " + mat.start());
+ }
+ }
+
@Override
public void close()
{
diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/AbstractFSResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/AbstractFSResourceTest.java
index deaff22dc1..3edda21811 100644
--- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/AbstractFSResourceTest.java
+++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/AbstractFSResourceTest.java
@@ -18,10 +18,6 @@
package org.eclipse.jetty.util.resource;
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-import static org.junit.Assume.*;
-
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
@@ -51,6 +47,14 @@ import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeNoException;
+
public abstract class AbstractFSResourceTest
{
@Rule
@@ -518,6 +522,89 @@ public abstract class AbstractFSResourceTest
}
@Test
+ public void testExist_BadControlChars_Encoded() throws Exception
+ {
+ createEmptyFile("a.jsp");
+
+ try
+ {
+ // request with control characters
+ URI ref = testdir.getDir().toURI().resolve("a.jsp%1F%10");
+ assertThat("ControlCharacters URI",ref,notNullValue());
+
+ Resource fileref = newResource(ref);
+ assertThat("File Resource should not exists", fileref.exists(), is(false));
+ }
+ catch (InvalidPathException e)
+ {
+ // Expected path
+ }
+ }
+
+ @Test
+ public void testExist_BadControlChars_Decoded() throws Exception
+ {
+ createEmptyFile("a.jsp");
+
+ try
+ {
+ // request with control characters
+ File badFile = new File(testdir.getDir(), "a.jsp\014\010");
+ newResource(badFile);
+ fail("Should have thrown " + InvalidPathException.class);
+ }
+ catch (InvalidPathException e)
+ {
+ // Expected path
+ }
+ }
+
+ @Test
+ public void testExist_AddPath_BadControlChars_Decoded() throws Exception
+ {
+ createEmptyFile("a.jsp");
+
+ try
+ {
+ // base resource
+ URI ref = testdir.getDir().toURI();
+ Resource base = newResource(ref);
+ assertThat("Base Resource URI",ref,notNullValue());
+
+ // add path with control characters (raw/decoded control characters)
+ // This MUST fail
+ base.addPath("/a.jsp\014\010");
+ fail("Should have thrown " + InvalidPathException.class);
+ }
+ catch (InvalidPathException e)
+ {
+ // Expected path
+ }
+ }
+
+ @Test
+ public void testExist_AddPath_BadControlChars_Encoded() throws Exception
+ {
+ createEmptyFile("a.jsp");
+
+ try
+ {
+ // base resource
+ URI ref = testdir.getDir().toURI();
+ Resource base = newResource(ref);
+ assertThat("Base Resource URI",ref,notNullValue());
+
+ // add path with control characters
+ Resource fileref = base.addPath("/a.jsp%14%10");
+ assertThat("File Resource should not exists", fileref.exists(), is(false));
+ }
+ catch (InvalidPathException e)
+ {
+ // Expected path
+ }
+ }
+
+ @Test
public void testUtf8Dir() throws Exception
{
File dir=new File(testdir.getDir(),"bãm");
diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileResourceTest.java
index ff63159f91..afa16e5726 100644
--- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileResourceTest.java
+++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileResourceTest.java
@@ -22,9 +22,6 @@ import java.io.File;
import java.io.IOException;
import java.net.URI;
-import org.junit.Ignore;
-import org.junit.Test;
-
public class FileResourceTest extends AbstractFSResourceTest
{
@Override
@@ -38,16 +35,4 @@ public class FileResourceTest extends AbstractFSResourceTest
{
return new FileResource(file);
}
-
- @Ignore("Cannot get null to be seen by FileResource")
- @Test
- public void testExist_BadNull() throws Exception
- {
- }
-
- @Ignore("Validation shouldn't be done in FileResource")
- @Test
- public void testExist_BadNullX() throws Exception
- {
- }
}

Back to the top