* Modernizing testcase Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> * Issue #4033 Percent Encoded Bad Requests Added test to demonstrate bad percent encoded request Signed-off-by: Greg Wilkins <gregw@webtide.com> * Issue #4033 - adding sanity test for percent paths and checkAlias() Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> * Eliminating 9.3.0.RC0 dependency Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> * Issue #4033 - More tests for Resource checkAlias() behavior Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> * Issue #4033 - Splitting badDecodePath Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> * Issue #4033 - More badDecodePath tests Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> * Issue #4033 Percent Encoded Bad Requests reverted decodePathBehaviour Signed-off-by: Greg Wilkins <gregw@webtide.com> * testing pull request building * Issue #4033 updates after review Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins
GitHub
parent
c336616c96
commit
ee0f9fc1d0
|
|
@ -824,12 +824,11 @@ public class HttpConnectionTest
|
|||
@Test
|
||||
public void testBadURIencoding() throws Exception
|
||||
{
|
||||
// The URI is being leniently decoded, leaving the "%x" alone
|
||||
String response = connector.getResponse("GET /bad/encoding%x HTTP/1.1\r\n" +
|
||||
"Host: localhost\r\n" +
|
||||
"Connection: close\r\n" +
|
||||
"\r\n");
|
||||
checkContains(response, 0, "HTTP/1.1 200");
|
||||
checkContains(response, 0, "HTTP/1.1 400");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
|||
|
|
@ -251,6 +251,24 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testBadURI() throws Exception
|
||||
{
|
||||
configureServer(new HelloWorldHandler());
|
||||
|
||||
try (Socket client = newSocket(_serverURI.getHost(), _serverURI.getPort()))
|
||||
{
|
||||
OutputStream os = client.getOutputStream();
|
||||
|
||||
os.write("GET /%xx HTTP/1.0\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1));
|
||||
os.flush();
|
||||
|
||||
// Read the response.
|
||||
String response = readResponse(client);
|
||||
|
||||
assertThat(response, Matchers.containsString("HTTP/1.1 400 "));
|
||||
}
|
||||
}
|
||||
@Test
|
||||
public void testExceptionThrownInHandlerLoop() throws Exception
|
||||
{
|
||||
|
|
|
|||
|
|
@ -470,70 +470,25 @@ public class URIUtil
|
|||
builder = new Utf8StringBuilder(path.length());
|
||||
builder.append(path, offset, i - offset);
|
||||
}
|
||||
|
||||
// lenient percent decoding
|
||||
if (i >= end)
|
||||
if ((i + 2) < end)
|
||||
{
|
||||
// [LENIENT] a percent sign at end of string.
|
||||
builder.append('%');
|
||||
i = end;
|
||||
}
|
||||
else if (end > (i + 1))
|
||||
{
|
||||
char type = path.charAt(i + 1);
|
||||
if (type == 'u')
|
||||
char u = path.charAt(i + 1);
|
||||
if (u == 'u')
|
||||
{
|
||||
// We have a possible (deprecated) microsoft unicode code point "%u####"
|
||||
// - not recommended to use as it's limited to 2 bytes.
|
||||
if ((i + 5) >= end)
|
||||
{
|
||||
// [LENIENT] we have a partial "%u####" at the end of a string.
|
||||
builder.append(path, i, (end - i));
|
||||
i = end;
|
||||
}
|
||||
else
|
||||
{
|
||||
// this seems wrong, as we are casting to a char, but that's the known
|
||||
// limitation of this deprecated encoding (only 2 bytes allowed)
|
||||
if (StringUtil.isHex(path, i + 2, 4))
|
||||
{
|
||||
builder.append((char)(0xffff & TypeUtil.parseInt(path, i + 2, 4, 16)));
|
||||
i += 5;
|
||||
}
|
||||
else
|
||||
{
|
||||
// [LENIENT] copy the "%u" as-is.
|
||||
builder.append(path, i, 2);
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
else if (end > (i + 2))
|
||||
{
|
||||
// we have a possible "%##" encoding
|
||||
if (StringUtil.isHex(path, i + 1, 2))
|
||||
{
|
||||
builder.append((byte)TypeUtil.parseInt(path, i + 1, 2, 16));
|
||||
i += 2;
|
||||
}
|
||||
else
|
||||
{
|
||||
builder.append(path, i, 3);
|
||||
i += 2;
|
||||
}
|
||||
// TODO remove %u support in jetty-10
|
||||
// this is wrong. This is a codepoint not a char
|
||||
builder.append((char)(0xffff & TypeUtil.parseInt(path, i + 2, 4, 16)));
|
||||
i += 5;
|
||||
}
|
||||
else
|
||||
{
|
||||
// [LENIENT] incomplete "%##" sequence at end of string
|
||||
builder.append(path, i, (end - i));
|
||||
i = end;
|
||||
builder.append((byte)(0xff & (TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(path.charAt(i + 2)))));
|
||||
i += 2;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
// [LENIENT] the "%" at the end of the string
|
||||
builder.append(path, i, (end - i));
|
||||
i = end;
|
||||
throw new IllegalArgumentException("Bad URI % encoding");
|
||||
}
|
||||
|
||||
break;
|
||||
|
|
@ -571,10 +526,18 @@ public class URIUtil
|
|||
}
|
||||
catch (NotUtf8Exception e)
|
||||
{
|
||||
LOG.warn(path.substring(offset, offset + length) + " " + e);
|
||||
LOG.debug(e);
|
||||
LOG.debug(path.substring(offset, offset + length) + " " + e);
|
||||
return decodeISO88591Path(path, offset, length);
|
||||
}
|
||||
catch (IllegalArgumentException e)
|
||||
{
|
||||
throw e;
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
throw new IllegalArgumentException("cannot decode URI", e);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/* Decode a URI path and strip parameters of ISO-8859-1 path
|
||||
|
|
@ -599,13 +562,14 @@ public class URIUtil
|
|||
char u = path.charAt(i + 1);
|
||||
if (u == 'u')
|
||||
{
|
||||
// TODO this is wrong. This is a codepoint not a char
|
||||
// TODO remove %u encoding support in jetty-10
|
||||
// This is wrong. This is a codepoint not a char
|
||||
builder.append((char)(0xffff & TypeUtil.parseInt(path, i + 2, 4, 16)));
|
||||
i += 5;
|
||||
}
|
||||
else
|
||||
{
|
||||
builder.append((byte)(0xff & (TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(path.charAt(i + 2)))));
|
||||
builder.append((char)(0xff & (TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(path.charAt(i + 2)))));
|
||||
i += 2;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
|
|||
import static org.hamcrest.Matchers.is;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
|
|
@ -94,18 +95,6 @@ public class URIUtilTest
|
|||
|
||||
// Deprecated Microsoft Percent-U encoding
|
||||
arguments.add(Arguments.of("abc%u3040", "abc\u3040"));
|
||||
|
||||
// Lenient decode
|
||||
arguments.add(Arguments.of("abc%xyz", "abc%xyz")); // not a "%##"
|
||||
arguments.add(Arguments.of("abc%", "abc%")); // percent at end of string
|
||||
arguments.add(Arguments.of("abc%A", "abc%A")); // incomplete "%##" at end of string
|
||||
arguments.add(Arguments.of("abc%uvwxyz", "abc%uvwxyz")); // not a valid "%u####"
|
||||
arguments.add(Arguments.of("abc%uEFGHIJ", "abc%uEFGHIJ")); // not a valid "%u####"
|
||||
arguments.add(Arguments.of("abc%uABC", "abc%uABC")); // incomplete "%u####"
|
||||
arguments.add(Arguments.of("abc%uAB", "abc%uAB")); // incomplete "%u####"
|
||||
arguments.add(Arguments.of("abc%uA", "abc%uA")); // incomplete "%u####"
|
||||
arguments.add(Arguments.of("abc%u", "abc%u")); // incomplete "%u####"
|
||||
|
||||
return arguments.stream();
|
||||
}
|
||||
|
||||
|
|
@ -117,6 +106,60 @@ public class URIUtilTest
|
|||
assertEquals(expectedPath, path);
|
||||
}
|
||||
|
||||
public static Stream<Arguments> decodeBadPathSource()
|
||||
{
|
||||
List<Arguments> arguments = new ArrayList<>();
|
||||
|
||||
// Test for null character (real world ugly test case)
|
||||
// TODO is this a bad decoding or a bad URI ?
|
||||
// arguments.add(Arguments.of("/%00/"));
|
||||
|
||||
// Deprecated Microsoft Percent-U encoding
|
||||
// TODO still supported for now ?
|
||||
// arguments.add(Arguments.of("abc%u3040"));
|
||||
|
||||
// Bad %## encoding
|
||||
arguments.add(Arguments.of("abc%xyz"));
|
||||
|
||||
// Incomplete %## encoding
|
||||
arguments.add(Arguments.of("abc%"));
|
||||
arguments.add(Arguments.of("abc%A"));
|
||||
|
||||
// Invalid microsoft %u#### encoding
|
||||
arguments.add(Arguments.of("abc%uvwxyz"));
|
||||
arguments.add(Arguments.of("abc%uEFGHIJ"));
|
||||
|
||||
// Incomplete microsoft %u#### encoding
|
||||
arguments.add(Arguments.of("abc%uABC"));
|
||||
arguments.add(Arguments.of("abc%uAB"));
|
||||
arguments.add(Arguments.of("abc%uA"));
|
||||
arguments.add(Arguments.of("abc%u"));
|
||||
|
||||
// Invalid UTF-8 and ISO8859-1
|
||||
// TODO currently ISO8859 is too forgiving to detect these
|
||||
/*
|
||||
arguments.add(Arguments.of("abc%C3%28")); // invalid 2 octext sequence
|
||||
arguments.add(Arguments.of("abc%A0%A1")); // invalid 2 octext sequence
|
||||
arguments.add(Arguments.of("abc%e2%28%a1")); // invalid 3 octext sequence
|
||||
arguments.add(Arguments.of("abc%e2%82%28")); // invalid 3 octext sequence
|
||||
arguments.add(Arguments.of("abc%f0%28%8c%bc")); // invalid 4 octext sequence
|
||||
arguments.add(Arguments.of("abc%f0%90%28%bc")); // invalid 4 octext sequence
|
||||
arguments.add(Arguments.of("abc%f0%28%8c%28")); // invalid 4 octext sequence
|
||||
arguments.add(Arguments.of("abc%f8%a1%a1%a1%a1")); // valid sequence, but not unicode
|
||||
arguments.add(Arguments.of("abc%fc%a1%a1%a1%a1%a1")); // valid sequence, but not unicode
|
||||
arguments.add(Arguments.of("abc%f8%a1%a1%a1")); // incomplete sequence
|
||||
*/
|
||||
|
||||
return arguments.stream();
|
||||
}
|
||||
|
||||
@ParameterizedTest(name = "[{index}] {0}")
|
||||
@MethodSource("decodeBadPathSource")
|
||||
public void testBadDecodePath(String encodedPath)
|
||||
{
|
||||
assertThrows(IllegalArgumentException.class, () -> URIUtil.decodePath(encodedPath));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testDecodePathSubstring()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -18,93 +18,155 @@
|
|||
|
||||
package org.eclipse.jetty.util.resource;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.net.MalformedURLException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.InvalidPathException;
|
||||
import java.nio.file.Path;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.function.Function;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.eclipse.jetty.toolchain.test.FS;
|
||||
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
|
||||
import org.junit.jupiter.api.BeforeAll;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
|
||||
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
|
||||
import org.eclipse.jetty.util.log.Log;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.MethodSource;
|
||||
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
@ExtendWith(WorkDirExtension.class)
|
||||
public class ResourceAliasTest
|
||||
{
|
||||
static File __dir;
|
||||
public WorkDir workDir;
|
||||
|
||||
@BeforeAll
|
||||
public static void beforeClass()
|
||||
public static Stream<Function<Path, Resource>> resourceTypes()
|
||||
{
|
||||
__dir = MavenTestingUtils.getTargetTestingDir("RAT");
|
||||
List<Function<Path, Resource>> types = new ArrayList<>();
|
||||
|
||||
types.add((path) -> new PathResource(path));
|
||||
types.add((path) -> new FileResource(path.toFile()));
|
||||
|
||||
return types.stream();
|
||||
}
|
||||
|
||||
@BeforeEach
|
||||
public void before()
|
||||
@ParameterizedTest
|
||||
@MethodSource("resourceTypes")
|
||||
public void testPercentPaths(Function<Path, Resource> resourceType) throws IOException
|
||||
{
|
||||
FS.ensureDirExists(__dir);
|
||||
FS.ensureEmpty(__dir);
|
||||
Path baseDir = workDir.getEmptyPathDir();
|
||||
|
||||
Path foo = baseDir.resolve("%foo");
|
||||
Files.createDirectories(foo);
|
||||
|
||||
Path bar = foo.resolve("bar%");
|
||||
Files.createDirectories(bar);
|
||||
|
||||
Path text = bar.resolve("test.txt");
|
||||
FS.touch(text);
|
||||
|
||||
// At this point we have a path .../%foo/bar%/test.txt present on the filesystem.
|
||||
// This would also apply for paths found in JAR files (like META-INF/resources/%foo/bar%/test.txt)
|
||||
|
||||
assertTrue(Files.exists(text));
|
||||
|
||||
Resource baseResource = resourceType.apply(baseDir);
|
||||
assertTrue(baseResource.exists(), "baseResource exists");
|
||||
|
||||
Resource fooResource = baseResource.addPath("%foo");
|
||||
assertTrue(fooResource.exists(), "fooResource exists");
|
||||
assertTrue(fooResource.isDirectory(), "fooResource isDir");
|
||||
if (fooResource instanceof FileResource)
|
||||
assertTrue(fooResource.isAlias(), "fooResource isAlias");
|
||||
else
|
||||
assertFalse(fooResource.isAlias(), "fooResource isAlias");
|
||||
|
||||
Resource barResource = fooResource.addPath("bar%");
|
||||
assertTrue(barResource.exists(), "barResource exists");
|
||||
assertTrue(barResource.isDirectory(), "barResource isDir");
|
||||
if (fooResource instanceof FileResource)
|
||||
assertTrue(barResource.isAlias(), "barResource isAlias");
|
||||
else
|
||||
assertFalse(barResource.isAlias(), "barResource isAlias");
|
||||
|
||||
Resource textResource = barResource.addPath("test.txt");
|
||||
assertTrue(textResource.exists(), "textResource exists");
|
||||
assertFalse(textResource.isDirectory(), "textResource isDir");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNullCharEndingFilename() throws Exception
|
||||
{
|
||||
File file = new File(__dir, "test.txt");
|
||||
assertFalse(file.exists());
|
||||
assertTrue(file.createNewFile());
|
||||
assertTrue(file.exists());
|
||||
Path baseDir = workDir.getEmptyPathDir();
|
||||
|
||||
File file0 = new File(__dir, "test.txt\0");
|
||||
if (!file0.exists())
|
||||
return; // this file system does not suffer this problem
|
||||
|
||||
assertTrue(file0.exists()); // This is an alias!
|
||||
|
||||
Resource dir = Resource.newResource(__dir);
|
||||
|
||||
// Test not alias paths
|
||||
Resource resource = Resource.newResource(file);
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = Resource.newResource(file.getAbsoluteFile());
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = Resource.newResource(file.toURI());
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = Resource.newResource(file.toURI().toString());
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = dir.addPath("test.txt");
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
|
||||
// Test alias paths
|
||||
resource = Resource.newResource(file0);
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
resource = Resource.newResource(file0.getAbsoluteFile());
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
resource = Resource.newResource(file0.toURI());
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
resource = Resource.newResource(file0.toURI().toString());
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
Path file = baseDir.resolve("test.txt");
|
||||
FS.touch(file);
|
||||
|
||||
try
|
||||
{
|
||||
resource = dir.addPath("test.txt\0");
|
||||
Path file0 = baseDir.resolve("test.txt\0");
|
||||
if (!Files.exists(file0))
|
||||
return; // this file system does get tricked by ending filenames
|
||||
|
||||
assertThat(file0 + " exists", Files.exists(file0), is(true)); // This is an alias!
|
||||
|
||||
Resource dir = Resource.newResource(baseDir);
|
||||
|
||||
// Test not alias paths
|
||||
Resource resource = Resource.newResource(file);
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = Resource.newResource(file.toAbsolutePath());
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = Resource.newResource(file.toUri());
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = Resource.newResource(file.toUri().toString());
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
resource = dir.addPath("test.txt");
|
||||
assertTrue(resource.exists());
|
||||
assertNull(resource.getAlias());
|
||||
|
||||
// Test alias paths
|
||||
resource = Resource.newResource(file0);
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
resource = Resource.newResource(file0.toAbsolutePath());
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
resource = Resource.newResource(file0.toUri());
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
resource = Resource.newResource(file0.toUri().toString());
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
|
||||
try
|
||||
{
|
||||
resource = dir.addPath("test.txt\0");
|
||||
assertTrue(resource.exists());
|
||||
assertNotNull(resource.getAlias());
|
||||
}
|
||||
catch (MalformedURLException e)
|
||||
{
|
||||
assertTrue(true);
|
||||
}
|
||||
}
|
||||
catch (MalformedURLException e)
|
||||
catch (InvalidPathException e)
|
||||
{
|
||||
assertTrue(true);
|
||||
// this file system does allow null char ending filenames
|
||||
Log.getRootLogger().ignore(e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue