Skip to content

Disallow unwise characters #1131

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

Merged
merged 1 commit into from
Oct 29, 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
1 change: 1 addition & 0 deletions core/common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ dependencies {

implementation "commons-codec:commons-codec:$commonsCodecVersion"
implementation "jakarta.xml.bind:jakarta.xml.bind-api:$jaxbApiVersion"
implementation "org.apache.commons:commons-lang3:$commonsLangVersion"
implementation "org.eclipse.microprofile.config:microprofile-config-api"
implementation "org.slf4j:slf4j-api:$slf4jVersion"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ public final class HttpConstants {
/** The Memento link parameter indicating the ending range of a TimeMap. */
public static final String UNTIL = "until";

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

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

import org.apache.commons.codec.DecoderException;
import org.apache.commons.codec.net.URLCodec;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;

/**
Expand Down Expand Up @@ -80,8 +81,9 @@ private static String decodeSlug(final String value) {
}

private static String cleanSlugString(final String value) {
// Remove any fragment URIs and query parameters
// Then trim the string and replace any remaining whitespace or slash characters with underscores
return value.split("#")[0].split("\\?")[0].trim().replaceAll("[\\s/]+", "_");
// 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 + "/", "");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
class SlugTest {

private static final String SLUG_VALUE = "slugValue";
private static final String SLUG_UNDERSCORE_VALUE = "slug_value";
private static final String SLUG_UNDERSCORE_VALUE = "slugvalue";
private static final String CHECK_SLUG_VALUE = "Check slug value";

@Test
Expand Down Expand Up @@ -61,6 +61,12 @@ void testQueryParam() {
assertEquals(SLUG_VALUE, slug.getValue(), CHECK_SLUG_VALUE);
}

@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);
}

@Test
void testBadInput() {
assertNull(Slug.valueOf("An invalid % value"), "Check invalid input");
Expand Down
10 changes: 10 additions & 0 deletions core/http/src/main/java/org/trellisldp/http/TrellisHttpFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import javax.ws.rs.core.Link;
import javax.ws.rs.ext.Provider;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.rdf.api.IRI;
import org.trellisldp.common.AcceptDatetime;
import org.trellisldp.common.LdpResource;
Expand Down Expand Up @@ -85,6 +86,8 @@ public void setExtensions(final Map<String, IRI> extensions) {

@Override
public void filter(final ContainerRequestContext ctx) {
// Validate path
validatePath(ctx);
// Validate headers
validateAcceptDatetime(ctx);
validateRange(ctx);
Expand All @@ -103,6 +106,13 @@ public void filter(final ContainerRequestContext ctx) {
}
}

private void validatePath(final ContainerRequestContext ctx) {
final String path = ctx.getUriInfo().getPath();
if (StringUtils.containsAny(path, UNWISE_CHARACTERS)) {
ctx.abortWith(status(BAD_REQUEST).build());
}
}

private void validateAcceptDatetime(final ContainerRequestContext ctx) {
final String acceptDatetime = ctx.getHeaderString(ACCEPT_DATETIME);
if (acceptDatetime != null && AcceptDatetime.valueOf(acceptDatetime) == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ abstract class AbstractTrellisHttpResourceTest extends BaseTrellisHttpResourceTe
private static final String VAL_VERSION = "version";
private static final String VERSION_PARAM = "?version=1496262729";
private static final String PATH_REL_CHILD = "/child";
private static final String PATH_REL_GRANDCHILD = "/child_grandchild";
private static final String GRANDCHILD_SUFFIX = "_grandchild";
private static final String PATH_REL_GRANDCHILD = "/childgrandchild";
private static final String GRANDCHILD_SUFFIX = "grandchild";
private static final String WEAK_PREFIX = "W/\"";
private static final String PREFER_PREFIX = "return=representation; include=\"";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,18 @@ void testTestRootSlash() {
filter.filter(mockContext);
verify(mockContext, never().description("Trailing slash should trigger a redirect!")).abortWith(any());
}

@Test
void testUnwisePath() {
when(mockContext.getUriInfo()).thenReturn(mockUriInfo);
when(mockUriInfo.getPath()).thenReturn("/foo/bar/one|two");
when(mockUriInfo.getQueryParameters()).thenReturn(new MultivaluedHashMap<>());

final TrellisHttpFilter filter = new TrellisHttpFilter();
filter.setMutatingMethods(emptyList());
filter.setExtensions(emptyMap());

filter.filter(mockContext);
verify(mockContext).abortWith(any());
}
}
2 changes: 1 addition & 1 deletion core/http/src/test/resources/logback-test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</encoder>
</appender>

<logger name="org.trellisldp" additivity="false" level="DEBUG">
<logger name="org.trellisldp" additivity="false" level="INFO">
<appender-ref ref="STDOUT"/>
</logger>
<root additivity="false" level="WARN">
Expand Down