Skip to content

Commit

Permalink
4.x: double-slash URI issue #7474 (#7657)
Browse files Browse the repository at this point in the history
* Double-slash URI issue #7474

* Flaky test fix

* Review issues 2
  • Loading branch information
danielkec authored Sep 25, 2023
1 parent a0d4a50 commit 9f3e9fa
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 9 deletions.
16 changes: 12 additions & 4 deletions common/uri/src/main/java/io/helidon/common/uri/UriPathNoParam.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public String rawPathNoParams() {
@Override
public String path() {
if (decodedPath == null) {
decodedPath = decode(rawPath);
decodedPath = decode(rawPath, false);
}
return decodedPath;
}
Expand Down Expand Up @@ -104,11 +104,11 @@ public String toString() {
@Override
public void validate() {
if (decodedPath == null) {
this.decodedPath = URI.create(rawPath).normalize().getPath();
this.decodedPath = decode(rawPath, true);
}
}

private static String decode(String rawPath) {
private static String decode(String rawPath, boolean validate) {
/*
Raw path may:
- be encoded (%20)
Expand All @@ -121,10 +121,18 @@ private static String decode(String rawPath) {
int dot = rawPath.indexOf(".");
int doubleSlash = rawPath.indexOf("//");

if (percent == -1 && doubleSlash == -1 && dot == -1) {
if (!validate && percent == -1 && doubleSlash == -1 && dot == -1) {
return rawPath;
}

if (doubleSlash == 0) {
/*
RFC2396 - net_path starts with //, that would lead to loosing first path segment.
example: URI.create("//foo/bar").getPath() --> "/bar"
*/
rawPath = rawPath.substring(1);
}

return URI.create(rawPath).normalize().getPath();
}
}
14 changes: 14 additions & 0 deletions common/uri/src/test/java/io/helidon/common/uri/UriPathTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package io.helidon.common.uri;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
Expand All @@ -37,4 +39,16 @@ void testEncodeSpace() {
assertThat(path.path(), is("/my path"));
assertThat(path.rawPath(), is("/my%20path"));
}

@ParameterizedTest
@ValueSource(strings = {
"//foo/bar",
"//foo//bar",
"/foo//bar",
"/foo/bar",
})
void testDoubleSlash(String rawPath) {
UriPath path = UriPath.create(rawPath);
assertThat(path.path(), is("/foo/bar"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@
class TracingPropagationTest {
private static final Duration TIMEOUT = Duration.ofSeconds(15);
private static final AtomicReference<CountDownLatch> SPAN_COUNT_LATCH = new AtomicReference<>();
// 2+1 after re-introduction of content-write span
private static final int EXPECTED_NUMBER_OF_SPANS = 3;
private static MockTracer tracer;
private final Http1Client client;
private final URI uri;
Expand Down Expand Up @@ -92,7 +94,7 @@ protected void onSpanFinished(MockSpan mockSpan) {

@Test
void testTracingSuccess() throws InterruptedException {
SPAN_COUNT_LATCH.set(new CountDownLatch(2));
SPAN_COUNT_LATCH.set(new CountDownLatch(EXPECTED_NUMBER_OF_SPANS));
Context context = Context.builder().id("tracing-unit-test").build();
context.register(tracer);

Expand All @@ -110,10 +112,7 @@ void testTracingSuccess() throws InterruptedException {
List<MockSpan> mockSpans = tracer.finishedSpans();

// the server traces asynchronously, some spans may be written after we receive the response.
// we need to try to wait for such spans
// re-introduced content-write span
assertThat("There should be 3 spans reported", tracer.finishedSpans(), hasSize(3));

assertThat("There should be 3 spans reported", tracer.finishedSpans(), hasSize(EXPECTED_NUMBER_OF_SPANS));

// we need the first span - parentId 0
MockSpan clientSpan = findSpanWithParentId(mockSpans, 0);
Expand Down

0 comments on commit 9f3e9fa

Please sign in to comment.