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

Fix NullPointerException when uri is null #7387

Merged
merged 1 commit into from
Dec 12, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ public String url(HttpClientConfig request) {

// use the baseUrl if it was configured
String baseUrl = request.baseUrl();

if (uri == null) {
// internally reactor netty appends "/" to the baseUrl
return baseUrl.endsWith("/") ? baseUrl : baseUrl + "/";
}

if (baseUrl != null) {
if (baseUrl.endsWith("/") && uri.startsWith("/")) {
baseUrl = baseUrl.substring(0, baseUrl.length() - 1);
Expand All @@ -48,7 +54,7 @@ public String url(HttpClientConfig request) {
}

private static boolean isAbsolute(String uri) {
return uri.startsWith("http://") || uri.startsWith("https://");
return uri != null && !uri.isEmpty() && !uri.startsWith("/");
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private static String getHost(HttpClientConfig request) {
String baseUrl = request.baseUrl();
String uri = request.uri();

if (baseUrl != null && uri.startsWith("/")) {
if (baseUrl != null && !isAbsolute(uri)) {
return UrlParser.getHost(baseUrl);
} else {
return UrlParser.getHost(uri);
Expand All @@ -68,10 +68,14 @@ private static Integer getPort(HttpClientConfig request) {
String baseUrl = request.baseUrl();
String uri = request.uri();

if (baseUrl != null && uri.startsWith("/")) {
if (baseUrl != null && !isAbsolute(uri)) {
return UrlParser.getPort(baseUrl);
} else {
return UrlParser.getPort(uri);
}
}

private static boolean isAbsolute(String uri) {
return uri != null && !uri.isEmpty() && !uri.startsWith("/");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.reactornetty.v1_0;

import static io.opentelemetry.api.trace.SpanKind.CLIENT;
import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
import static io.opentelemetry.api.trace.SpanKind.SERVER;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.HttpFlavorValues.HTTP_1_1;
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.trace.SpanBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.test.server.http.RequestContextGetter;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import io.opentelemetry.testing.internal.armeria.common.HttpData;
import io.opentelemetry.testing.internal.armeria.common.HttpResponse;
import io.opentelemetry.testing.internal.armeria.common.HttpStatus;
import io.opentelemetry.testing.internal.armeria.common.ResponseHeaders;
import io.opentelemetry.testing.internal.armeria.common.ResponseHeadersBuilder;
import io.opentelemetry.testing.internal.armeria.server.ServerBuilder;
import io.opentelemetry.testing.internal.armeria.testing.junit5.server.ServerExtension;
import org.assertj.core.api.AbstractLongAssert;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import reactor.netty.http.client.HttpClient;

class ReactorNettyBaseUrlOnlyTest {

@RegisterExtension
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();

static ServerExtension server;

@BeforeAll
static void setUp() {
server =
new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) {
sb.service(
"/base/",
(ctx, req) -> {
ResponseHeadersBuilder headers = ResponseHeaders.builder(HttpStatus.OK);

SpanBuilder span =
GlobalOpenTelemetry.getTracer("test")
.spanBuilder("test-http-server")
.setSpanKind(SERVER)
.setParent(
GlobalOpenTelemetry.getPropagators()
.getTextMapPropagator()
.extract(Context.current(), ctx, RequestContextGetter.INSTANCE));

span.startSpan().end();

return HttpResponse.of(headers.build(), HttpData.ofAscii("Hello."));
});
}
};
server.start();
}

@AfterAll
static void tearDown() {
server.stop();
}

@Test
void testSuccessfulRequest() {
HttpClient httpClient = HttpClient.create();
String uri = "http://localhost:" + server.httpPort() + "/base";

int responseCode =
testing.runWithSpan(
"parent",
() ->
httpClient
.baseUrl(uri)
.get()
.responseSingle(
(resp, content) -> {
// Make sure to consume content since that's when we close the span.
return content.map(unused -> resp);
})
.block()
.status()
.code());

assertThat(responseCode).isEqualTo(200);

testing.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactlyInAnyOrder(
span -> span.hasName("parent").hasKind(INTERNAL).hasNoParent(),
span ->
span.hasName("HTTP GET")
.hasKind(CLIENT)
.hasParent(trace.getSpan(0))
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.HTTP_METHOD, "GET"),
equalTo(SemanticAttributes.HTTP_URL, uri + "/"),
equalTo(SemanticAttributes.HTTP_FLAVOR, HTTP_1_1),
equalTo(SemanticAttributes.HTTP_STATUS_CODE, 200),
satisfies(
SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH,
AbstractLongAssert::isNotNegative),
equalTo(SemanticAttributes.NET_PEER_NAME, "localhost"),
equalTo(SemanticAttributes.NET_PEER_PORT, server.httpPort()),
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1")),
span ->
span.hasName("test-http-server").hasKind(SERVER).hasParent(trace.getSpan(1))));
}
}