From 5cf3875c2ee7e5a104ca930d48f9763fd5dae23c Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Mon, 6 Oct 2025 11:28:50 +0200 Subject: [PATCH] safer resttemplate status code access --- .../ClientHttpResponseAdapter.java | 58 +++++++++++++++++++ .../SpringRestTemplateAdvice.java | 3 +- .../SpringRestTemplateInstrumentation.java | 5 +- .../AbstractServerInstrumentationTest.java | 2 +- .../testapp/ApplicationTest.java | 2 +- 5 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/ClientHttpResponseAdapter.java diff --git a/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/ClientHttpResponseAdapter.java b/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/ClientHttpResponseAdapter.java new file mode 100644 index 0000000000..cc5113a041 --- /dev/null +++ b/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/ClientHttpResponseAdapter.java @@ -0,0 +1,58 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.resttemplate; + +import org.springframework.http.client.ClientHttpResponse; + +public class ClientHttpResponseAdapter { + + private static final int UNKNOWN_STATUS = -1; + + public static int getStatusCode(ClientHttpResponse response) { + int status = internalGetStatusCode(response); + if (status == UNKNOWN_STATUS) { + status = legacyGetStatusCode(response); + } + return status; + } + + private static int internalGetStatusCode(ClientHttpResponse response) { + try { + return response.getStatusCode().value(); + } catch (Exception e) { + // using broad exception to handle when method is missing for pre 6.x versions + return UNKNOWN_STATUS; + } + } + + private static int legacyGetStatusCode(ClientHttpResponse response) { + // getRawStatusCode has been introduced in 3.1.1 + // but deprecated in 6.x, will be removed in 7.x + try { + return response.getRawStatusCode(); + } catch (Exception e) { + // using broad exception to handle when method is missing in pre-3.1.1 and post 7.x + return UNKNOWN_STATUS; + } + } + + private ClientHttpResponseAdapter() { + + } +} diff --git a/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateAdvice.java b/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateAdvice.java index c78fb0dda7..f7f78b6616 100644 --- a/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateAdvice.java +++ b/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateAdvice.java @@ -65,8 +65,7 @@ public static void afterExecute(@Advice.Return @Nullable ClientHttpResponse clie Span span = (Span) spanObj; try { if (clientHttpResponse != null) { - // getRawStatusCode has been introduced in 3.1.1 - span.getContext().getHttp().withStatusCode(clientHttpResponse.getRawStatusCode()); + span.getContext().getHttp().withStatusCode(ClientHttpResponseAdapter.getStatusCode(clientHttpResponse)); } if (t != null) { span.withOutcome(Outcome.FAILURE); diff --git a/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateInstrumentation.java b/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateInstrumentation.java index 81ddbdf4e5..f5adea8487 100644 --- a/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateInstrumentation.java +++ b/apm-agent-plugins/apm-spring-resttemplate/apm-spring-resttemplate-plugin/src/main/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateInstrumentation.java @@ -56,7 +56,10 @@ public ElementMatcher getMethodMatcher() { .and(returns( hasSuperType(named("org.springframework.http.client.ClientHttpResponse")) // getRawStatusCode added in 3.1.1 thus we rely on that to filter unsupported versions - .and(declaresMethod(named("getRawStatusCode"))) + // will be removed in 7.x + .and(declaresMethod(named("getRawStatusCode")) + // getStatusCode added in 6.x and replaces getRawStatusCode in 7.x + .or(declaresMethod(named("getStatusCode")))) )); } diff --git a/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-spring5/src/test/java/co/elastic/apm/agent/springwebflux/AbstractServerInstrumentationTest.java b/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-spring5/src/test/java/co/elastic/apm/agent/springwebflux/AbstractServerInstrumentationTest.java index f4cacf4877..991ad179ed 100644 --- a/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-spring5/src/test/java/co/elastic/apm/agent/springwebflux/AbstractServerInstrumentationTest.java +++ b/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-spring5/src/test/java/co/elastic/apm/agent/springwebflux/AbstractServerInstrumentationTest.java @@ -199,7 +199,7 @@ void dispatch404_usePathAsName() { private Predicate expectClientError(int expectedStatus) { return error -> (error instanceof WebClientResponseException) - && ((WebClientResponseException) error).getRawStatusCode() == expectedStatus; + && ((WebClientResponseException) error).getStatusCode().value() == expectedStatus; } @ParameterizedTest diff --git a/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-testapp/src/test/java/co/elastic/apm/agent/springwebflux/testapp/ApplicationTest.java b/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-testapp/src/test/java/co/elastic/apm/agent/springwebflux/testapp/ApplicationTest.java index 82dff718df..3da2354516 100644 --- a/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-testapp/src/test/java/co/elastic/apm/agent/springwebflux/testapp/ApplicationTest.java +++ b/apm-agent-plugins/apm-spring-webflux/apm-spring-webflux-testapp/src/test/java/co/elastic/apm/agent/springwebflux/testapp/ApplicationTest.java @@ -136,6 +136,6 @@ private static Predicate> checkSSE(final int index) { private Predicate expectClientError(int expectedStatus) { return error -> (error instanceof WebClientResponseException) - && ((WebClientResponseException) error).getRawStatusCode() == expectedStatus; + && ((WebClientResponseException) error).getStatusCode().value() == expectedStatus; } }