Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for reserved characters #1159

Merged
merged 1 commit into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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());
}
}