Skip to content

Commit

Permalink
Improve support for reserved characters
Browse files Browse the repository at this point in the history
Resolves #1158
  • Loading branch information
acoburn committed Nov 9, 2020
1 parent b001a42 commit 952034f
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 17 deletions.
33 changes: 33 additions & 0 deletions components/test/src/main/java/org/trellisldp/test/LdpRdfTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ default Stream<Executable> runTests() throws Exception {
this::testGetRDF,
this::testRdfContainment,
this::testPostJsonLd,
this::testPutReservedCharacters,
this::testInvalidRDF);
}

Expand Down Expand Up @@ -324,6 +325,38 @@ default void testRdfContainment() throws Exception {
}
}

/**
* Verify that reserved characters are properly escaped.
* @throws Exception if the RDF resource does not close cleanly
*/
default void testPutReservedCharacters() throws Exception {
final RDF rdf = RDFFactory.getInstance();
final String content = getResourceAsString(SIMPLE_RESOURCE);
final String path = getResourceLocation() + "-%5B-%5D-%3A-%3F-%23-%60-%5E-%5C-%25-%22-%7C";

// PUT an LDP-RS
try (final Response res = target(path).request()
.put(entity(content, TEXT_TURTLE))) {
assertAll("Check PUTting an RDF resource", checkRdfResponse(res, LDP.RDFSource, null));
}

// Test the parent container
try (final Response res = target(getBaseURL()).request().get();
final Graph g = readEntityAsGraph(res.getEntity(), getBaseURL(), TURTLE)) {
assertTrue(res.getMediaType().isCompatible(TEXT_TURTLE_TYPE), "Check that the container is RDF");
assertTrue(g.contains(rdf.createIRI(getBaseURL()), LDP.contains,
rdf.createIRI(path)), "Check for an ldp:contains property");
}

// Test the child resource
try (final Response res = target(path).request().get();
final Graph g = readEntityAsGraph(res.getEntity(), path, TURTLE)) {
assertTrue(res.getMediaType().isCompatible(TEXT_TURTLE_TYPE), "Check that the container is RDF");
assertTrue(g.contains(rdf.createIRI(path), DC.subject, rdf.createIRI("http://example.org/subject/1")),
"Check for an ldp:contains property");
}
}

/**
* Verify that POSTing JSON-LD is supported.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ public final class HttpConstants {
/** Configuration key defining whether to include dates in memento headers. */
public static final String CONFIG_HTTP_MEMENTO_HEADER_DATES = "trellis.http.memento-header-dates";

/** Configuration key defining path characters to disallow. */
public static final String CONFIG_HTTP_PATH_CHARACTERS_DISALLOW = "trellis.http.path-chars-disallow";

/** Configuration key defining path characters to URL-encode. */
public static final String CONFIG_HTTP_PATH_CHARACTERS_ENCODE = "trellis.http.path-chars-encode";

/** Configuration key defining whether to use weak ETags for RDF responses. */
public static final String CONFIG_HTTP_WEAK_ETAG = "trellis.http.weak-etag";

Expand Down Expand Up @@ -131,7 +137,10 @@ public final class HttpConstants {
public static final String UNTIL = "until";

/** A collection of "unwise" characters according to RFC 3987. */
public static final String UNWISE_CHARACTERS = "<>{}`^\\%\"|";
public static final String UNWISE_CHARACTERS = "[]:?#`^\\%\"|";

/** A collection of "invalid" characters according to RFC 3987. */
public static final String INVALID_CHARACTERS = "<>{}";

/** The implied or default set of IRIs used with a Prefer header. */
public static final Set<IRI> DEFAULT_REPRESENTATION = Set.of(PreferContainment, PreferMembership,
Expand Down
8 changes: 7 additions & 1 deletion core/common/src/main/java/org/trellisldp/common/Slug.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
package org.trellisldp.common;

import static java.util.Objects.requireNonNull;
import static org.eclipse.microprofile.config.ConfigProvider.getConfig;
import static org.slf4j.LoggerFactory.getLogger;
import static org.trellisldp.common.HttpConstants.*;

import org.apache.commons.codec.DecoderException;
import org.apache.commons.codec.net.URLCodec;
Expand All @@ -35,6 +37,10 @@
*/
public class Slug {

private static final String UNWISE_CHARS = getConfig()
.getOptionalValue(CONFIG_HTTP_PATH_CHARACTERS_ENCODE, String.class).orElse(UNWISE_CHARACTERS);
private static final String INVALID_CHARS = getConfig()
.getOptionalValue(CONFIG_HTTP_PATH_CHARACTERS_DISALLOW, String.class).orElse(INVALID_CHARACTERS);
private static final Logger LOGGER = getLogger(Slug.class);
private static final URLCodec DECODER = new URLCodec();

Expand Down Expand Up @@ -84,6 +90,6 @@ private static String cleanSlugString(final String value) {
// Remove any fragment URIs, query parameters and whitespace
final String base = StringUtils.deleteWhitespace(value.split("#")[0].split("\\?")[0]);
// Remove any "unwise" characters plus '/'
return StringUtils.replaceChars(base, HttpConstants.UNWISE_CHARACTERS + "/", "");
return StringUtils.replaceChars(base, UNWISE_CHARS + INVALID_CHARS + "/", "");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,16 @@
*/
package org.trellisldp.common;

import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Collections.emptyList;
import static javax.ws.rs.core.HttpHeaders.CONTENT_TYPE;
import static javax.ws.rs.core.HttpHeaders.LINK;
import static org.trellisldp.common.HttpConstants.ACCEPT_DATETIME;
import static org.trellisldp.common.HttpConstants.PREFER;
import static org.trellisldp.common.HttpConstants.RANGE;
import static org.trellisldp.common.HttpConstants.SLUG;
import static org.eclipse.microprofile.config.ConfigProvider.getConfig;
import static org.trellisldp.common.HttpConstants.*;

import java.net.URI;
import java.net.URLEncoder;
import java.util.ArrayList;
import java.util.List;

import javax.ws.rs.core.HttpHeaders;
Expand All @@ -35,6 +36,7 @@
import javax.ws.rs.core.UriBuilder;
import javax.ws.rs.core.UriInfo;

import org.apache.commons.lang3.StringUtils;
import org.trellisldp.vocabulary.LDP;

/**
Expand All @@ -44,6 +46,11 @@
*/
public class TrellisRequest {

private static final String ESCAPE_CHARACTERS = getConfig()
.getOptionalValue(CONFIG_HTTP_PATH_CHARACTERS_ENCODE, String.class).orElse(UNWISE_CHARACTERS);
private static final String[] ESCAPE_SEARCH = getSearchArray(ESCAPE_CHARACTERS);
private static final String[] ESCAPE_REPLACE = getReplaceArray(ESCAPE_CHARACTERS);

private final boolean trailingSlash;
private final String path;
private final String baseUrl;
Expand Down Expand Up @@ -80,9 +87,11 @@ public TrellisRequest(final Request request, final UriInfo uriInfo, final HttpHe
// Extract URI values
this.parameters = uriInfo.getQueryParameters();
this.baseUrl = buildBaseUrl(uriInfo.getBaseUri(), this.headers);
this.path = uriInfo.getPathParameters().getFirst("path");
this.trailingSlash = uriInfo.getPath().endsWith("/");

final String urlPath = uriInfo.getPathParameters().getFirst("path");
this.path = StringUtils.replaceEach(urlPath, ESCAPE_SEARCH, ESCAPE_REPLACE);

// Extract request method
this.method = request.getMethod();

Expand Down Expand Up @@ -263,4 +272,20 @@ public static String buildBaseUrl(final URI uri, final MultivaluedMap<String, St
}
return builder.build().toString();
}

static String[] getSearchArray(final String escapeChars) {
final List<String> search = new ArrayList<>();
for (char ch : escapeChars.toCharArray()) {
search.add(String.valueOf(ch));
}
return search.toArray(String[]::new);
}

static String[] getReplaceArray(final String escapeChars) {
final List<String> replace = new ArrayList<>();
for (char ch : escapeChars.toCharArray()) {
replace.add(URLEncoder.encode(String.valueOf(ch), UTF_8));
}
return replace.toArray(String[]::new);
}
}
8 changes: 4 additions & 4 deletions core/common/src/test/java/org/trellisldp/common/SlugTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ void testSlug() {
@Test
void testEncodedInput() {
final Slug slug = Slug.valueOf("slug%3Avalue");
assertEquals("slug:value", slug.getValue(), "Check decoding slug value");
assertEquals("slugvalue", slug.getValue(), "Check decoding slug value");
}

@ParameterizedTest
@ValueSource(strings = {"slug value", "slug/value", "slug\t/ value"})
@ValueSource(strings = {"slug value", "slug/value", "slug\t/ value", "slug|value", "sl[ug^val]ue"})
void testNormalization(final String value) {
final Slug slug = Slug.valueOf(value);
assertEquals(SLUG_UNDERSCORE_VALUE, slug.getValue(), CHECK_SLUG_VALUE);
Expand All @@ -63,8 +63,8 @@ void testQueryParam() {

@Test
void testUnwiseCharacters() {
final Slug slug = Slug.valueOf("a|b^c\"d{e}f\\g`h^i<j>k");
assertEquals("abcdefghijk", slug.getValue(), CHECK_SLUG_VALUE);
final Slug slug = Slug.valueOf("a|b^c\"d\\e`f^g\"h<i>j{k}l");
assertEquals("abcdefghijkl", slug.getValue(), CHECK_SLUG_VALUE);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
package org.trellisldp.common;

import static java.net.URI.create;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Collections.singletonList;
import static javax.ws.rs.HttpMethod.GET;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;

import java.net.URI;
import java.net.URLEncoder;

import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Link;
Expand All @@ -32,6 +34,8 @@

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.ValueSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

Expand Down Expand Up @@ -116,8 +120,11 @@ void testTrellisRequestForwarded() {
final MultivaluedMap<String, String> headers = new MultivaluedHashMap<>();
headers.putSingle("Forwarded", "host=app.example.com;proto=https");

final MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
pathParams.add("path", "resource");

when(mockUriInfo.getPath()).thenReturn("resource");
when(mockUriInfo.getPathParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getPathParameters()).thenReturn(pathParams);
when(mockUriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getBaseUri()).thenReturn(uri);
when(mockHeaders.getRequestHeaders()).thenReturn(headers);
Expand All @@ -133,8 +140,11 @@ void testTrellisRequestForwardedWithPort() {
final MultivaluedMap<String, String> headers = new MultivaluedHashMap<>();
headers.putSingle("Forwarded", "host=app.example.com:9000;proto=https");

final MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
pathParams.add("path", "resource");

when(mockUriInfo.getPath()).thenReturn("resource");
when(mockUriInfo.getPathParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getPathParameters()).thenReturn(pathParams);
when(mockUriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getBaseUri()).thenReturn(uri);
when(mockHeaders.getRequestHeaders()).thenReturn(headers);
Expand All @@ -147,12 +157,15 @@ void testTrellisRequestForwardedWithPort() {
void testTrellisRequestBadXForwardedPort() {
final URI uri = create("http://example.com/");

final MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
pathParams.add("path", "resource");

final MultivaluedMap<String, String> headers = new MultivaluedHashMap<>();
headers.putSingle("X-Forwarded-Proto", "foo");
headers.putSingle("X-Forwarded-Host", "app.example.com");

when(mockUriInfo.getPath()).thenReturn("resource");
when(mockUriInfo.getPathParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getPathParameters()).thenReturn(pathParams);
when(mockUriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getBaseUri()).thenReturn(uri);
when(mockHeaders.getRequestHeaders()).thenReturn(headers);
Expand Down Expand Up @@ -250,4 +263,43 @@ void testSkippedLinkHeaders() {
final TrellisRequest req = new TrellisRequest(mockRequest, mockUriInfo, mockHeaders);
assertNull(req.getLink());
}

@ParameterizedTest
@ValueSource(strings = { "[", "]", "%", ":", "?", "#", "\"", "\\", "|", "^", "`" })
void testExcapeChars(final String character) {
final String path = "before" + character + "after";
final MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
pathParams.add("path", path);

when(mockUriInfo.getPath()).thenReturn(path);
when(mockUriInfo.getPathParameters()).thenReturn(pathParams);
when(mockUriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockHeaders.getRequestHeaders()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getBaseUri()).thenReturn(create("http://example.com"));
when(mockRequest.getMethod()).thenReturn(GET);
when(mockHeaders.getAcceptableMediaTypes()).thenReturn(singletonList(RdfMediaType.TEXT_TURTLE_TYPE));

final TrellisRequest req = new TrellisRequest(mockRequest, mockUriInfo, mockHeaders);
assertEquals(URLEncoder.encode(path, UTF_8), req.getPath());
}

@ParameterizedTest
@ValueSource(strings = { "before[middle]after", "a:b%c", "c%b:a", "a?b#c", "a%b?c",
"A|B", "before^after", "x%y|z" })
void testExcapeMultipleChars(final String character) {
final String path = "before" + character + "after";
final MultivaluedMap<String, String> pathParams = new MultivaluedHashMap<>();
pathParams.add("path", path);

when(mockUriInfo.getPath()).thenReturn(path);
when(mockUriInfo.getPathParameters()).thenReturn(pathParams);
when(mockUriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>());
when(mockHeaders.getRequestHeaders()).thenReturn(new MultivaluedHashMap<>());
when(mockUriInfo.getBaseUri()).thenReturn(create("http://example.com"));
when(mockRequest.getMethod()).thenReturn(GET);
when(mockHeaders.getAcceptableMediaTypes()).thenReturn(singletonList(RdfMediaType.TEXT_TURTLE_TYPE));

final TrellisRequest req = new TrellisRequest(mockRequest, mockUriInfo, mockHeaders);
assertEquals(URLEncoder.encode(path, UTF_8), req.getPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.rdf.api.IRI;
import org.eclipse.microprofile.config.Config;
import org.trellisldp.common.AcceptDatetime;
import org.trellisldp.common.LdpResource;
import org.trellisldp.common.Prefer;
Expand All @@ -57,15 +58,20 @@
@LdpResource
public class TrellisHttpFilter implements ContainerRequestFilter {

private final String invalidChars;

private List<String> mutatingMethods;
private Map<String, IRI> extensions;

/**
* Create a simple pre-matching filter.
*/
public TrellisHttpFilter() {
final Config config = getConfig();
this.mutatingMethods = asList(POST, PUT, DELETE, PATCH);
this.extensions = buildExtensionMapFromConfig(getConfig());
this.extensions = buildExtensionMapFromConfig(config);
this.invalidChars = config.getOptionalValue(CONFIG_HTTP_PATH_CHARACTERS_DISALLOW, String.class)
.orElse(INVALID_CHARACTERS);
}

/**
Expand Down Expand Up @@ -108,7 +114,7 @@ public void filter(final ContainerRequestContext ctx) {

private void validatePath(final ContainerRequestContext ctx) {
final String path = ctx.getUriInfo().getPath();
if (StringUtils.containsAny(path, UNWISE_CHARACTERS)) {
if (StringUtils.containsAny(path, invalidChars)) {
ctx.abortWith(status(BAD_REQUEST).build());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,6 @@ void testUnwisePath() {
filter.setExtensions(emptyMap());

filter.filter(mockContext);
verify(mockContext).abortWith(any());
verify(mockContext, never()).abortWith(any());
}
}

0 comments on commit 952034f

Please sign in to comment.