Merge branch `jetty-9.4.x` into `jetty-10.0.x`

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

# Conflicts:
#	jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java
#	jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterTest.java
#	jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
#	jetty-util/src/test/java/org/eclipse/jetty/util/resource/FileSystemResourceTest.java
This commit is contained in:
Joakim Erdfelt 2019-08-23 08:52:52 -05:00
commit 36ef975353
8 changed files with 385 additions and 78 deletions

View File

@ -64,15 +64,14 @@ public abstract class CookieCutter
boolean inQuoted = false;
boolean quoted = false;
boolean escaped = false;
boolean reject = false;
int tokenstart = -1;
int tokenend = -1;
for (int i = 0, length = hdr.length(); i <= length; i++)
{
char c = i == length ? 0 : hdr.charAt(i);
// System.err.printf("i=%d/%d c=%s v=%b q=%b/%b e=%b u=%s s=%d e=%d \t%s=%s%n" ,i,length,c==0?"|":(""+c),invalue,inQuoted,quoted,escaped,unquoted,tokenstart,tokenend,name,value);
// Handle quoted values for name or value
// Handle quoted values for value
if (inQuoted)
{
if (escaped)
@ -119,7 +118,7 @@ public abstract class CookieCutter
// Handle name and value state machines
if (invalue)
{
// parse the value
// parse the cookie-value
switch (c)
{
case ' ':
@ -193,7 +192,11 @@ public abstract class CookieCutter
// This is a new cookie, so add the completed last cookie if we have one
if (cookieName != null)
{
addCookie(cookieName, cookieValue, cookieDomain, cookiePath, cookieVersion, cookieComment);
if(!reject)
{
addCookie(cookieName, cookieValue, cookieDomain, cookiePath, cookieVersion, cookieComment);
reject = false;
}
cookieDomain = null;
cookiePath = null;
cookieComment = null;
@ -234,6 +237,15 @@ public abstract class CookieCutter
quoted = false;
continue;
}
if (_complianceMode == CookieCompliance.RFC6265)
{
if (isRFC6265RejectedCharacter(inQuoted, c))
{
reject = true;
}
}
if (tokenstart < 0)
tokenstart = i;
tokenend = i;
@ -242,13 +254,26 @@ public abstract class CookieCutter
}
else
{
// parse the name
// parse the cookie-name
switch (c)
{
case 0:
case ' ':
case '\t':
continue;
case '"':
// Quoted name is not allowed in any version of the Cookie spec
reject = true;
break;
case ';':
// a cookie terminated with no '=' sign.
tokenstart = -1;
invalue = false;
reject = false;
continue;
case '=':
if (quoted)
{
@ -272,6 +297,15 @@ public abstract class CookieCutter
quoted = false;
continue;
}
if (_complianceMode == CookieCompliance.RFC6265)
{
if (isRFC6265RejectedCharacter(inQuoted, c))
{
reject = true;
}
}
if (tokenstart < 0)
tokenstart = i;
tokenend = i;
@ -281,7 +315,7 @@ public abstract class CookieCutter
}
}
if (cookieName != null)
if (cookieName != null && !reject)
addCookie(cookieName, cookieValue, cookieDomain, cookiePath, cookieVersion, cookieComment);
}
}
@ -295,4 +329,31 @@ public abstract class CookieCutter
}
protected abstract void addCookie(String cookieName, String cookieValue, String cookieDomain, String cookiePath, int cookieVersion, String cookieComment);
protected boolean isRFC6265RejectedCharacter(boolean inQuoted, char c)
{
if (inQuoted)
{
// We only reject if a Control Character is encountered
if (Character.isISOControl(c))
{
return true;
}
}
else
{
/* From RFC6265 - Section 4.1.1 - Syntax
* cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
* ; US-ASCII characters excluding CTLs,
* ; whitespace DQUOTE, comma, semicolon,
* ; and backslash
*/
return Character.isISOControl(c) || // control characters
c > 127 || // 8-bit characters
c == ',' || // comma
c == ';'; // semicolon
}
return false;
}
}

View File

@ -40,6 +40,7 @@ public class CookieCutterLenientTest
{
return Stream.of(
// Simple test to verify behavior
Arguments.of("x=y", "x", "y"),
Arguments.of("key=value", "key", "value"),
// Tests that conform to RFC2109
@ -62,12 +63,17 @@ public class CookieCutterLenientTest
// quoted-string = ( <"> *(qdtext) <"> )
// qdtext = <any TEXT except <">>
// rejected, as name cannot be DQUOTED
Arguments.of("\"a\"=bcd", null, null),
Arguments.of("\"a\"=\"b c d\"", null, null),
// lenient with spaces and EOF
Arguments.of("abc=", "abc", ""),
Arguments.of("abc = ", "abc", ""),
Arguments.of("abc = ;", "abc", ""),
Arguments.of("abc = ; ", "abc", ""),
Arguments.of("abc = x ", "abc", "x"),
Arguments.of("abc = e f g ", "abc", "e f g"),
Arguments.of("abc=\"\"", "abc", ""),
Arguments.of("abc= \"\" ", "abc", ""),
Arguments.of("abc= \"x\" ", "abc", "x"),
@ -112,28 +118,29 @@ public class CookieCutterLenientTest
// Unterminated Quotes with trailing escape
Arguments.of("x=\"abc\\", "x", "\"abc\\"),
// UTF-8 values
Arguments.of("2sides=\u262F", "2sides", "\u262f"), // 2 byte
Arguments.of("currency=\"\u20AC\"", "currency", "\u20AC"), // 3 byte
Arguments.of("gothic=\"\uD800\uDF48\"", "gothic", "\uD800\uDF48"), // 4 byte
// UTF-8 raw values (not encoded) - VIOLATION of RFC6265
Arguments.of("2sides=\u262F", null, null), // 2 byte (YIN YANG) - rejected due to not being DQUOTED
Arguments.of("currency=\"\u20AC\"", "currency", "\u20AC"), // 3 byte (EURO SIGN)
Arguments.of("gothic=\"\uD800\uDF48\"", "gothic", "\uD800\uDF48"), // 4 byte (GOTHIC LETTER HWAIR)
// Spaces
Arguments.of("foo=bar baz", "foo", "bar baz"),
Arguments.of("foo=\"bar baz\"", "foo", "bar baz"),
Arguments.of("z=a b c d e f g", "z", "a b c d e f g"),
// Bad tspecials usage
// Bad tspecials usage - VIOLATION of RFC6265
Arguments.of("foo=bar;baz", "foo", "bar"),
Arguments.of("foo=\"bar;baz\"", "foo", "bar;baz"),
Arguments.of("z=a;b,c:d;e/f[g]", "z", "a"),
Arguments.of("z=\"a;b,c:d;e/f[g]\"", "z", "a;b,c:d;e/f[g]"),
Arguments.of("name=quoted=\"\\\"badly\\\"\"", "name", "quoted=\"\\\"badly\\\"\""), // someone attempting to escape a DQUOTE from within a DQUOTED pair)
// Quoted with other Cookie keywords
Arguments.of("x=\"$Version=0\"", "x", "$Version=0"),
Arguments.of("x=\"$Path=/\"", "x", "$Path=/"),
Arguments.of("x=\"$Path=/ $Domain=.foo.com\"", "x", "$Path=/ $Domain=.foo.com"),
Arguments.of("x=\" $Path=/ $Domain=.foo.com \"", "x", " $Path=/ $Domain=.foo.com "),
Arguments.of("a=\"b; $Path=/a; c=d; $PATH=/c; e=f\"; $Path=/e/", "a", "b; $Path=/a; c=d; $PATH=/c; e=f"),
Arguments.of("a=\"b; $Path=/a; c=d; $PATH=/c; e=f\"; $Path=/e/", "a", "b; $Path=/a; c=d; $PATH=/c; e=f"), // VIOLATES RFC6265
// Lots of equals signs
Arguments.of("query=b=c&d=e", "query", "b=c&d=e"),
@ -144,7 +151,7 @@ public class CookieCutterLenientTest
// Google cookies (seen in wild, has `tspecials` of ':' in value)
Arguments.of("GAPS=1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-", "GAPS", "1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-"),
// Strong abuse of cookie spec (lots of tspecials)
// Strong abuse of cookie spec (lots of tspecials) - VIOLATION of RFC6265
Arguments.of("$Version=0; rToken=F_TOKEN''!--\"</a>=&{()}", "rToken", "F_TOKEN''!--\"</a>=&{()}"),
// Commas that were not commas

View File

@ -212,6 +212,20 @@ public class CookieCutterTest
assertThat("Cookies.length", cookies.length, is(0));
}
@Test
public void testMultipleCookies()
{
String rawCookie = "testcookie; server.id=abcd; server.detail=cfg";
// The first cookie "testcookie" should be ignored, per RFC6265, as it's missing the "=" sign.
Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie);
assertThat("Cookies.length", cookies.length, is(2));
assertCookie("Cookies[0]", cookies[0], "server.id", "abcd", 0, null);
assertCookie("Cookies[1]", cookies[1], "server.detail", "cfg", 0, null);
}
static class Cookie
{
String name;
@ -282,6 +296,4 @@ public class CookieCutterTest
super.parseFields(Arrays.asList(fields));
}
}
;
}

View File

@ -0,0 +1,78 @@
//
// ========================================================================
// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.server;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import org.eclipse.jetty.server.handler.ContextHandler.AliasCheck;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.resource.PathResource;
import org.eclipse.jetty.util.resource.Resource;
/**
* Alias checking for working with FileSystems that normalize access to the
* File System.
* <p>
* The Java {@link Files#isSameFile(Path, Path)} method is used to determine
* if the requested file is the same as the alias file.
* </p>
* <p>
* For File Systems that are case insensitive (eg: Microsoft Windows FAT32 and NTFS),
* the access to the file can be in any combination or style of upper and lowercase.
* </p>
* <p>
* For File Systems that normalize UTF-8 access (eg: Mac OSX on HFS+ or APFS,
* or Linux on XFS) the the actual file could be stored using UTF-16,
* but be accessed using NFD UTF-8 or NFC UTF-8 for the same file.
* </p>
*/
public class SameFileAliasChecker implements AliasCheck
{
private static final Logger LOG = Log.getLogger(SameFileAliasChecker.class);
@Override
public boolean check(String uri, Resource resource)
{
// Only support PathResource alias checking
if (!(resource instanceof PathResource))
return false;
try
{
PathResource pathResource = (PathResource)resource;
Path path = pathResource.getPath();
Path alias = pathResource.getAliasPath();
if (Files.isSameFile(path, alias))
{
if (LOG.isDebugEnabled())
LOG.debug("Allow alias to same file {} --> {}", path, alias);
return true;
}
}
catch (IOException e)
{
LOG.ignore(e);
}
return false;
}
}

View File

@ -52,7 +52,7 @@ public class AllowSymLinkAliasChecker implements AliasCheck
Path path = pathResource.getPath();
Path alias = pathResource.getAliasPath();
if (path.equals(alias))
if (PathResource.isSameName(alias, path))
return false; // Unknown why this is an alias
if (hasSymbolicLink(path) && Files.isSameFile(path, alias))

View File

@ -24,7 +24,6 @@ import java.io.OutputStream;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
@ -54,12 +53,14 @@ import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.ResourceContentFactory;
import org.eclipse.jetty.server.ResourceService;
import org.eclipse.jetty.server.SameFileAliasChecker;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.AllowSymLinkAliasChecker;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.eclipse.jetty.util.resource.PathResource;
import org.eclipse.jetty.util.resource.Resource;
@ -73,6 +74,7 @@ import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jetty.http.tools.matchers.HttpFieldsMatchers.containsHeader;
import static org.eclipse.jetty.http.tools.matchers.HttpFieldsMatchers.containsHeaderValue;
import static org.hamcrest.MatcherAssert.assertThat;
@ -2010,6 +2012,76 @@ public class DefaultServletTest
assertThat(response.toString(), response.getStatus(), is(HttpStatus.PRECONDITION_FAILED_412));
}
@Test
public void testGetUtf8NfcFile() throws Exception
{
FS.ensureEmpty(docRoot);
context.addServlet(DefaultServlet.class, "/");
context.addAliasCheck(new SameFileAliasChecker());
// UTF-8 NFC format
String filename = "swedish-" + new String(TypeUtil.fromHexString("C3A5"), UTF_8) + ".txt";
createFile(docRoot.resolve(filename), "hi a-with-circle");
String rawResponse;
HttpTester.Response response;
// Request as UTF-8 NFC
rawResponse = connector.getResponse("GET /context/swedish-%C3%A5.txt HTTP/1.1\r\nHost:test\r\nConnection:close\r\n\r\n");
response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat(response.getContent(), is("hi a-with-circle"));
// Request as UTF-8 NFD
rawResponse = connector.getResponse("GET /context/swedish-a%CC%8A.txt HTTP/1.1\r\nHost:test\r\nConnection:close\r\n\r\n");
response = HttpTester.parseResponse(rawResponse);
if (OS.MAC.isCurrentOs())
{
assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat(response.getContent(), is("hi a-with-circle"));
}
else
{
assertThat(response.getStatus(), is(HttpStatus.NOT_FOUND_404));
}
}
@Test
public void testGetUtf8NfdFile() throws Exception
{
FS.ensureEmpty(docRoot);
context.addServlet(DefaultServlet.class, "/");
context.addAliasCheck(new SameFileAliasChecker());
// UTF-8 NFD format
String filename = "swedish-" + new String(TypeUtil.fromHexString("61CC8A"), UTF_8) + ".txt";
createFile(docRoot.resolve(filename), "hi a-with-circle");
String rawResponse;
HttpTester.Response response;
// Request as UTF-8 NFD
rawResponse = connector.getResponse("GET /context/swedish-a%CC%8A.txt HTTP/1.1\r\nHost:test\r\nConnection:close\r\n\r\n");
response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat(response.getContent(), is("hi a-with-circle"));
// Request as UTF-8 NFC
rawResponse = connector.getResponse("GET /context/swedish-%C3%A5.txt HTTP/1.1\r\nHost:test\r\nConnection:close\r\n\r\n");
response = HttpTester.parseResponse(rawResponse);
if (OS.MAC.isCurrentOs())
{
assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat(response.getContent(), is("hi a-with-circle"));
}
else
{
assertThat(response.getStatus(), is(HttpStatus.NOT_FOUND_404));
}
}
public static class OutputFilter implements Filter
{
@Override
@ -2055,7 +2127,7 @@ public class DefaultServletTest
{
try (OutputStream out = Files.newOutputStream(path))
{
out.write(str.getBytes(StandardCharsets.UTF_8));
out.write(str.getBytes(UTF_8));
out.flush();
}
}

View File

@ -104,56 +104,10 @@ public class PathResource extends Resource
{
Path real = abs.toRealPath(FOLLOW_LINKS);
/*
* If the real path is not the same as the absolute path
* then we know that the real path is the alias for the
* provided path.
*
* For OS's that are case insensitive, this should
* return the real (on-disk / case correct) version
* of the path.
*
* We have to be careful on Windows and OSX.
*
* Assume we have the following scenario
* Path a = new File("foo").toPath();
* Files.createFile(a);
* Path b = new File("FOO").toPath();
*
* There now exists a file called "foo" on disk.
* Using Windows or OSX, with a Path reference of
* "FOO", "Foo", "fOO", etc.. means the following
*
* | OSX | Windows | Linux
* -----------------------+---------+------------+---------
* Files.exists(a) | True | True | True
* Files.exists(b) | True | True | False
* Files.isSameFile(a,b) | True | True | False
* a.equals(b) | False | True | False
*
* See the javadoc for Path.equals() for details about this FileSystem
* behavior difference
*
* We also cannot rely on a.compareTo(b) as this is roughly equivalent
* in implementation to a.equals(b)
*/
int absCount = abs.getNameCount();
int realCount = real.getNameCount();
if (absCount != realCount)
if (!isSameName(abs, real))
{
// different number of segments
return real;
}
// compare each segment of path, backwards
for (int i = realCount - 1; i >= 0; i--)
{
if (!abs.getName(i).toString().equals(real.getName(i).toString()))
{
return real;
}
}
}
}
catch (IOException e)
@ -167,6 +121,82 @@ public class PathResource extends Resource
return null;
}
/**
* Test if the paths are the same name.
*
* <p>
* If the real path is not the same as the absolute path
* then we know that the real path is the alias for the
* provided path.
* </p>
*
* <p>
* For OS's that are case insensitive, this should
* return the real (on-disk / case correct) version
* of the path.
* </p>
*
* <p>
* We have to be careful on Windows and OSX.
* </p>
*
* <p>
* Assume we have the following scenario:
* </p>
*
* <pre>
* Path a = new File("foo").toPath();
* Files.createFile(a);
* Path b = new File("FOO").toPath();
* </pre>
*
* <p>
* There now exists a file called {@code foo} on disk.
* Using Windows or OSX, with a Path reference of
* {@code FOO}, {@code Foo}, {@code fOO}, etc.. means the following
* </p>
*
* <pre>
* | OSX | Windows | Linux
* -----------------------+---------+------------+---------
* Files.exists(a) | True | True | True
* Files.exists(b) | True | True | False
* Files.isSameFile(a,b) | True | True | False
* a.equals(b) | False | True | False
* </pre>
*
* <p>
* See the javadoc for Path.equals() for details about this FileSystem
* behavior difference
* </p>
*
* <p>
* We also cannot rely on a.compareTo(b) as this is roughly equivalent
* in implementation to a.equals(b)
* </p>
*/
public static boolean isSameName(Path pathA, Path pathB)
{
int aCount = pathA.getNameCount();
int bCount = pathB.getNameCount();
if (aCount != bCount)
{
// different number of segments
return false;
}
// compare each segment of path, backwards
for (int i = bCount; i-- > 0; )
{
if (!pathA.getName(i).toString().equals(pathB.getName(i).toString()))
{
return false;
}
}
return true;
}
/**
* Construct a new PathResource from a File object.
* <p>

View File

@ -19,7 +19,9 @@
package org.eclipse.jetty.util.resource;
import java.io.BufferedWriter;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.StringReader;
@ -319,6 +321,42 @@ public class FileSystemResourceTest
}
}
@ParameterizedTest
@MethodSource("fsResourceProvider")
public void testAccessUniCodeFile(Class resourceClass) throws Exception
{
Path dir = workDir.getEmptyPathDir();
String readableRootDir = findRootDir(dir.getFileSystem());
assumeTrue(readableRootDir != null, "Readable Root Dir found");
Path subdir = dir.resolve("sub");
Files.createDirectories(subdir);
touchFile(subdir.resolve("swedish-å.txt"), "hi a-with-circle");
touchFile(subdir.resolve("swedish-ä.txt"), "hi a-with-two-dots");
touchFile(subdir.resolve("swedish-ö.txt"), "hi o-with-two-dots");
try (Resource base = newResource(resourceClass, subdir.toFile()))
{
Resource refA1 = base.addPath("swedish-å.txt");
Resource refA2 = base.addPath("swedish-ä.txt");
Resource refO1 = base.addPath("swedish-ö.txt");
assertThat("Ref A1 exists", refA1.exists(), is(true));
assertThat("Ref A2 exists", refA2.exists(), is(true));
assertThat("Ref O1 exists", refO1.exists(), is(true));
assertThat("Ref A1 alias", refA1.isAlias(), is(false));
assertThat("Ref A2 alias", refA2.isAlias(), is(false));
assertThat("Ref O1 alias", refO1.isAlias(), is(false));
assertThat("Ref A1 contents", toString(refA1), is("hi a-with-circle"));
assertThat("Ref A2 contents", toString(refA2), is("hi a-with-two-dots"));
assertThat("Ref O1 contents", toString(refO1), is("hi o-with-two-dots"));
}
}
private String findRootDir(FileSystem fs)
{
// look for a directory off of a root path
@ -419,12 +457,7 @@ public class FileSystemResourceTest
Files.createDirectories(dir);
Path file = dir.resolve("foo");
try (StringReader reader = new StringReader("foo");
BufferedWriter writer = Files.newBufferedWriter(file))
{
IO.copy(reader, writer);
}
touchFile(file, "foo");
long expected = Files.size(file);
@ -513,12 +546,7 @@ public class FileSystemResourceTest
Path file = dir.resolve("foo");
String content = "Foo is here";
try (StringReader reader = new StringReader(content);
BufferedWriter writer = Files.newBufferedWriter(file))
{
IO.copy(reader, writer);
}
touchFile(file, content);
try (Resource base = newResource(resourceClass, dir.toFile()))
{
@ -1485,4 +1513,23 @@ public class FileSystemResourceTest
assertThat("getAlias()", resource.getAlias(), nullValue());
}
}
private String toString(Resource resource) throws IOException
{
try (InputStream inputStream = resource.getInputStream();
ByteArrayOutputStream outputStream = new ByteArrayOutputStream())
{
IO.copy(inputStream, outputStream);
return outputStream.toString("utf-8");
}
}
private void touchFile(Path outputFile, String content) throws IOException
{
try (StringReader reader = new StringReader(content);
BufferedWriter writer = Files.newBufferedWriter(outputFile))
{
IO.copy(reader, writer);
}
}
}