From 72928ac2e1bcf0f88e21198d499fc2a4384655d1 Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Tue, 30 Jan 2024 15:56:05 -0500 Subject: [PATCH 1/3] Fixes problems converting a ClientUri to a URI. Improved test. Signed-off-by: Santiago Pericasgeertsen --- .../common/uri/UriQueryWriteableImpl.java | 4 ++-- .../jersey/connector/ConnectorBase.java | 6 ++--- .../io/helidon/webclient/api/ClientUri.java | 22 ++++++++++++------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteableImpl.java b/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteableImpl.java index 0dc4b5e6857..4db501a77ab 100644 --- a/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteableImpl.java +++ b/common/uri/src/main/java/io/helidon/common/uri/UriQueryWriteableImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023 Oracle and/or its affiliates. + * Copyright (c) 2022, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -79,7 +79,7 @@ public UriQueryWriteable from(UriQuery uriQuery) { .addAll(raw); List decoded = uriQuery.all(name); decodedQueryParams.computeIfAbsent(name, it -> new ArrayList<>()) - .addAll(raw); + .addAll(decoded); } } diff --git a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java index 0ab11b7896e..b1179dbbe76 100644 --- a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java +++ b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java @@ -115,11 +115,11 @@ public void testBasicPost() { @Test public void queryGetTest() { try (Response response = target("basic").path("getquery") - .queryParam("first", "hello there ") - .queryParam("second", "world") + .queryParam("first", "\"hello there ") + .queryParam("second", "world\"") .request().get()) { assertThat(response.getStatus(), is(200)); - assertThat(response.readEntity(String.class), is("hello there world")); + assertThat(response.readEntity(String.class), is("\"hello there world\"")); } } diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java index 45149e92469..ba5191da302 100644 --- a/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java +++ b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023 Oracle and/or its affiliates. + * Copyright (c) 2022, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -103,7 +103,8 @@ public String toString() { } /** - * Convert instance to {@link java.net.URI}. + * Convert instance to {@link java.net.URI}. Note that we must use raw here + * to make sure no illegal URL characters are passed to the create method. * * @return the converted URI */ @@ -111,7 +112,7 @@ public URI toUri() { UriInfo info = uriBuilder.build(); return URI.create(info.scheme() + "://" + info.authority() - + pathWithQueryAndFragment()); + + pathWithQueryAndFragment(false)); // URI needs encoded } /** @@ -293,15 +294,20 @@ public UriQueryWriteable writeableQuery() { } /** - * Encoded path with query and fragment. + * Path with query and fragment. Result is encoded or decoded depending on the + * value of {@link #skipUriEncoding}. * - * @return string containing encoded path with query + * @return string containing path with query */ public String pathWithQueryAndFragment() { + return pathWithQueryAndFragment(skipUriEncoding); + } + + private String pathWithQueryAndFragment(boolean decoded) { UriInfo info = uriBuilder.query(query).build(); - String queryString = skipUriEncoding ? info.query().value() : info.query().rawValue(); - String path = skipUriEncoding ? info.path().path() : info.path().rawPath(); + String queryString = decoded ? info.query().value() : info.query().rawValue(); + String path = decoded ? info.path().path() : info.path().rawPath(); boolean hasQuery = !queryString.isEmpty(); @@ -314,7 +320,7 @@ public String pathWithQueryAndFragment() { } if (info.fragment().hasValue()) { - String fragmentValue = skipUriEncoding ? info.fragment().value() : info.fragment().rawValue(); + String fragmentValue = decoded ? info.fragment().value() : info.fragment().rawValue(); path = path + '#' + fragmentValue; } From 9c697bba422aa71535cf97b52e9fcbc5af6c1f13 Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Thu, 1 Feb 2024 16:48:05 -0500 Subject: [PATCH 2/3] Drops skip encoding setting in connector after fixing handling of + in ClientUri query string. Signed-off-by: Santiago Pericasgeertsen --- .../jersey/connector/HelidonConnector.java | 1 - .../io/helidon/webclient/api/ClientUri.java | 28 +++++++++---------- .../helidon/webclient/api/ClientUriTest.java | 16 +++++++++-- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java index 5ed9b6030ae..ae407b707ff 100644 --- a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java +++ b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java @@ -151,7 +151,6 @@ private HttpClientRequest mapRequest(ClientRequest request) { HttpClientRequest httpRequest = webClient .method(Method.create(request.getMethod())) .proxy(requestProxy) - .skipUriEncoding(true) // already encoded by Jersey .uri(uri); // map request headers diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java index ba5191da302..9903579584a 100644 --- a/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java +++ b/webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java @@ -18,6 +18,7 @@ import java.net.URI; +import io.helidon.common.uri.UriEncoding; import io.helidon.common.uri.UriFragment; import io.helidon.common.uri.UriInfo; import io.helidon.common.uri.UriPath; @@ -103,8 +104,7 @@ public String toString() { } /** - * Convert instance to {@link java.net.URI}. Note that we must use raw here - * to make sure no illegal URL characters are passed to the create method. + * Convert instance to {@link java.net.URI}. * * @return the converted URI */ @@ -112,7 +112,7 @@ public URI toUri() { UriInfo info = uriBuilder.build(); return URI.create(info.scheme() + "://" + info.authority() - + pathWithQueryAndFragment(false)); // URI needs encoded + + pathWithQueryAndFragment()); } /** @@ -194,8 +194,11 @@ public ClientUri resolve(URI uri) { uriBuilder.path(resolvePath(uriBuilder.path().path(), uri.getPath())); - if (uri.getQuery() != null) { - query.fromQueryString(uri.getQuery()); + String queryString = uri.getQuery(); + if (queryString != null) { + // class URI does not decode +'s, so we do it here + query.fromQueryString(queryString.indexOf('+') >= 0 ? UriEncoding.decodeUri(queryString) + : queryString); } if (uri.getRawFragment() != null) { @@ -294,20 +297,15 @@ public UriQueryWriteable writeableQuery() { } /** - * Path with query and fragment. Result is encoded or decoded depending on the - * value of {@link #skipUriEncoding}. + * Encoded path with query and fragment. * - * @return string containing path with query + * @return string containing encoded path with query */ public String pathWithQueryAndFragment() { - return pathWithQueryAndFragment(skipUriEncoding); - } - - private String pathWithQueryAndFragment(boolean decoded) { UriInfo info = uriBuilder.query(query).build(); - String queryString = decoded ? info.query().value() : info.query().rawValue(); - String path = decoded ? info.path().path() : info.path().rawPath(); + String queryString = skipUriEncoding ? info.query().value() : info.query().rawValue(); + String path = skipUriEncoding ? info.path().path() : info.path().rawPath(); boolean hasQuery = !queryString.isEmpty(); @@ -320,7 +318,7 @@ private String pathWithQueryAndFragment(boolean decoded) { } if (info.fragment().hasValue()) { - String fragmentValue = decoded ? info.fragment().value() : info.fragment().rawValue(); + String fragmentValue = skipUriEncoding ? info.fragment().value() : info.fragment().rawValue(); path = path + '#' + fragmentValue; } diff --git a/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java b/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java index 173abf830df..56c29a00d7b 100644 --- a/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java +++ b/webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023 Oracle and/or its affiliates. + * Copyright (c) 2022, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,6 @@ import io.helidon.common.uri.UriPath; import io.helidon.common.uri.UriQueryWriteable; - import org.junit.jupiter.api.Test; import static org.hamcrest.CoreMatchers.is; @@ -105,4 +104,17 @@ void testResolveAll() { assertThat(helper.port(), is(80)); assertThat(helper.scheme(), is("https")); } + + /** + * Verifies that "+" is interpreted as a space character the query strings. + * Note that the {@link URI} class does not appear to handle this correctly. + */ + @Test + void testResolveQuery() { + URI uri = URI.create("http://localhost:8080/greet?filter=a+b+c"); + ClientUri clientUri = ClientUri.create(); + clientUri.resolve(uri); + assertThat(clientUri.query().get("filter"), is("a b c")); + assertThat(clientUri.query().getRaw("filter"), is("a%20b%20c")); + } } \ No newline at end of file From 55d1208a61efa605a2c3cb558e3a7447b0d8c71a Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Mon, 5 Feb 2024 15:22:53 -0500 Subject: [PATCH 3/3] Trying using forkCount set to 1. Signed-off-by: Santiago Pericasgeertsen --- microprofile/telemetry/pom.xml | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/microprofile/telemetry/pom.xml b/microprofile/telemetry/pom.xml index 499feda88a8..c5dd937b0ca 100644 --- a/microprofile/telemetry/pom.xml +++ b/microprofile/telemetry/pom.xml @@ -16,8 +16,8 @@ --> + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 helidon-microprofile-project @@ -137,28 +137,10 @@ org.apache.maven.plugins maven-surefire-plugin - - - default-test - - - **/WithSpanWithExplicitAppTest.java - - - - - - test-with-explicit-app - - test - - - - **/WithSpanWithExplicitAppTest.java - - - - + + 1 + false +