Skip to content

Commit

Permalink
Escape pipe characters in HTTP clients
Browse files Browse the repository at this point in the history
Fixes #1053
  • Loading branch information
yawkat committed Jan 24, 2025
1 parent 1f28e1f commit d905720
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnixDomainSocketAddress;
import java.nio.channels.Channels;
import java.nio.channels.SocketChannel;
Expand Down Expand Up @@ -138,6 +137,9 @@ public Object body() {

@Override
public HttpRequest appendPathPart(String encodedPathPart) {
// java-sdk follows whatwg spec for escaping, but RFC 2396 (followed by java.net.URI) does not permit pipe characters
encodedPathPart = encodedPathPart.replace("|", "%7c");

boolean hasSlashLeft = !path.isEmpty() && path.charAt(path.length() - 1) == '/';
boolean hasSlashRight = encodedPathPart.startsWith("/");
if (hasSlashLeft) {
Expand Down Expand Up @@ -177,11 +179,7 @@ private String buildRelativeUri() {

@Override
public URI uri() {
try {
return new URI(client.baseUri.getScheme(), client.baseUri.getAuthority(), path.toString(), query.isEmpty() ? null : query.toString(), null);
} catch (URISyntaxException e) {
throw new IllegalStateException(e);
}
return URI.create(client.baseUri + buildRelativeUri());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ public Object body() {

@Override
public HttpRequest appendPathPart(String encodedPathPart) {
// java-sdk follows whatwg spec for escaping, but RFC 2396 (followed by java.net.URI) does not permit pipe characters
encodedPathPart = encodedPathPart.replace("|", "%7c");

boolean hasSlashLeft = uri.charAt(uri.length() - 1) == '/';
boolean hasSlashRight = encodedPathPart.startsWith("/");
if (hasSlashLeft) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ public Object body() {

@Override
public HttpRequest appendPathPart(String encodedPathPart) {
// java-sdk follows whatwg spec for escaping, but RFC 2396 (followed by java.net.URI) does not permit pipe characters
encodedPathPart = encodedPathPart.replace("|", "%7c");

boolean hasSlashLeft = uri.charAt(uri.length() - 1) == '/';
boolean hasSlashRight = encodedPathPart.startsWith("/");
if (hasSlashLeft) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.oracle.bmc.common.ClientBuilderBase;
import com.oracle.bmc.http.client.HttpClient;
import com.oracle.bmc.http.client.HttpClientBuilder;
import com.oracle.bmc.http.client.HttpRequest;
import com.oracle.bmc.http.client.HttpResponse;
import com.oracle.bmc.http.client.Method;
import com.oracle.bmc.http.client.StandardClientProperties;
Expand Down Expand Up @@ -40,13 +41,15 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.security.cert.CertificateException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.concurrent.ExecutionException;

Expand Down Expand Up @@ -658,6 +661,33 @@ public void sessionTokenTest() throws Exception {
}
}

@Test
public void pipeTest() throws Exception {
String testString = "foo|bar";
netty.handleOneRequest((ctx, request) -> {
Assertions.assertEquals(HttpMethod.GET, request.method());
Assertions.assertEquals("/" + URLEncoder.encode(testString, StandardCharsets.UTF_8).toLowerCase(Locale.ROOT), request.uri().toLowerCase(Locale.ROOT));

DefaultFullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.wrappedBuffer("bar".getBytes(StandardCharsets.UTF_8)));
response.headers().add("Content-Type", "text/plain");
computeContentLength(response);
ctx.writeAndFlush(response);
});

try (HttpClient client = newBuilder()
.build()) {
HttpRequest request = client.createRequest(Method.GET)
.appendPathPart(testString);
Assertions.assertEquals("/" + testString, request.uri().getPath());
try (HttpResponse response = request
.execute().toCompletableFuture()
.get()) {
String s = response.textBody().toCompletableFuture().get();
Assertions.assertEquals("bar", s);
}
}
}

@Serdeable
public static class MyBean {
private String s;
Expand Down

0 comments on commit d905720

Please sign in to comment.