diff --git a/.github/dependabot.yml b/.github/dependabot.yml index fa14f951f4..d25a05febb 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -24,6 +24,9 @@ updates: # SAAJ impl at 1.x (soap) and 3.x (soap-jakarta) - dependency-name: "com.sun.xml.messaging.saaj:saaj-impl" update-types: ["version-update:semver-major"] + # feign-validation is intentionally on javax.validation; Hibernate Validator 7+ is Jakarta-only. + - dependency-name: "org.hibernate.validator:hibernate-validator" + update-types: ["version-update:semver-major"] # Jersey 2.x for jaxrs2 - package-ecosystem: "maven" diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml index 1c05e35670..bfbb33c261 100644 --- a/.mvn/extensions.xml +++ b/.mvn/extensions.xml @@ -19,7 +19,7 @@ com.gradle develocity-maven-extension - 2.4.1 + 2.4.2 com.gradle diff --git a/api/pom.xml b/api/pom.xml index ab97e63860..48dce106da 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -58,7 +58,7 @@ org.springframework spring-context - ${spring-context.version} + ${spring.version} test diff --git a/api/src/main/java/feign/BaseBuilder.java b/api/src/main/java/feign/BaseBuilder.java index 2d71f8f550..265e330221 100644 --- a/api/src/main/java/feign/BaseBuilder.java +++ b/api/src/main/java/feign/BaseBuilder.java @@ -366,6 +366,8 @@ List getFieldsToEnrich() { .filter(field -> !Objects.equals(field.getName(), "requestInterceptors")) .filter(field -> !Objects.equals(field.getName(), "responseInterceptors")) .filter(field -> !Objects.equals(field.getName(), "methodInterceptors")) + // caller-owned lifecycle resources are not capability-enriched + .filter(field -> !Objects.equals(field.getName(), "executorService")) // skip primitive types .filter(field -> !field.getType().isPrimitive()) // skip enumerations diff --git a/api/src/main/java/feign/Response.java b/api/src/main/java/feign/Response.java index 2bf64d56e2..c1b175ced8 100644 --- a/api/src/main/java/feign/Response.java +++ b/api/src/main/java/feign/Response.java @@ -20,7 +20,9 @@ import feign.Request.ProtocolVersion; import java.io.*; import java.nio.charset.Charset; +import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.StandardCharsets; +import java.nio.charset.UnsupportedCharsetException; import java.util.*; /** An immutable response to an http invocation which only returns string content. */ @@ -219,7 +221,11 @@ public Charset charset() { String[] charsetParts = contentTypeParmeters[1].split("="); if (charsetParts.length == 2 && "charset".equalsIgnoreCase(charsetParts[0].trim())) { String charsetString = charsetParts[1].replaceAll("\"", ""); - return Charset.forName(charsetString); + try { + return Charset.forName(charsetString); + } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { + return Util.UTF_8; + } } } } diff --git a/api/src/main/java/feign/codec/ErrorDecoder.java b/api/src/main/java/feign/codec/ErrorDecoder.java index 2b24f31705..24c0db08bc 100644 --- a/api/src/main/java/feign/codec/ErrorDecoder.java +++ b/api/src/main/java/feign/codec/ErrorDecoder.java @@ -105,14 +105,14 @@ public Long apply(String retryAfter) { if (retryAfter == null) { return null; } - if (retryAfter.matches("^[0-9]+\\.?0*$")) { - retryAfter = retryAfter.replaceAll("\\.0*$", ""); - long deltaMillis = SECONDS.toMillis(Long.parseLong(retryAfter)); - return currentTimeMillis() + deltaMillis; - } try { + if (retryAfter.matches("^[0-9]+\\.?0*$")) { + retryAfter = retryAfter.replaceAll("\\.0*$", ""); + long deltaMillis = SECONDS.toMillis(Long.parseLong(retryAfter)); + return currentTimeMillis() + deltaMillis; + } return ZonedDateTime.parse(retryAfter, dateTimeFormatter).toInstant().toEpochMilli(); - } catch (NullPointerException | DateTimeParseException ignored) { + } catch (NumberFormatException | NullPointerException | DateTimeParseException ignored) { return null; } } diff --git a/api/src/main/java/feign/template/Expressions.java b/api/src/main/java/feign/template/Expressions.java index 65fa5123e2..27b5f5e545 100644 --- a/api/src/main/java/feign/template/Expressions.java +++ b/api/src/main/java/feign/template/Expressions.java @@ -22,10 +22,18 @@ import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; public final class Expressions { - private static final int MAX_EXPRESSION_LENGTH = 10000; + /** + * System property controlling the maximum allowed length of a single expression. Defaults to + * {@link #DEFAULT_MAX_EXPRESSION_LENGTH}. Setting it to {@code 0} (or any non-positive value) + * disables the length check entirely. + */ + static final String MAX_EXPRESSION_LENGTH_PROPERTY = "feign.template.expression.maxLength"; + + private static final int DEFAULT_MAX_EXPRESSION_LENGTH = 10000; private static final String PATH_STYLE_OPERATOR = ";"; @@ -73,10 +81,16 @@ public static Expression create(final String value) { throw new IllegalArgumentException("an expression is required."); } - /* Check if the expression is too long */ - if (expression.length() > MAX_EXPRESSION_LENGTH) { + /* + * Check if the expression is too long. The limit is configurable through the + * "feign.template.expression.maxLength" system property and can be disabled by setting it to a + * non-positive value. + */ + final int maxExpressionLength = + Integer.getInteger(MAX_EXPRESSION_LENGTH_PROPERTY, DEFAULT_MAX_EXPRESSION_LENGTH); + if (maxExpressionLength > 0 && expression.length() > maxExpressionLength) { throw new IllegalArgumentException( - "expression is too long. Max length: " + MAX_EXPRESSION_LENGTH); + "expression is too long. Max length: " + maxExpressionLength); } /* create a new regular expression matcher for the expression */ @@ -104,15 +118,26 @@ public static Expression create(final String value) { } } - /* check for an operator */ - if (PATH_STYLE_OPERATOR.equalsIgnoreCase(operator)) { - return new PathStyleExpression(variableName, variablePattern); - } + /* + * The value modifier after the ':' is compiled as a regular expression. When the chunk is a + * dynamic value (for example a header-map value that happens to contain '{' and ':') the + * modifier is not a valid pattern, so treat the chunk as a literal instead of letting the + * PatternSyntaxException escape. + */ + try { + /* check for an operator */ + if (PATH_STYLE_OPERATOR.equalsIgnoreCase(operator)) { + return new PathStyleExpression(variableName, variablePattern); + } - /* default to simple */ - return SimpleExpression.isSimpleExpression(value) - ? new SimpleExpression(variableName, variablePattern) - : null; // Return null if it can't be validated as a Simple Expression -- Probably a Literal + /* default to simple */ + // Return null if it can't be validated as a Simple Expression -- Probably a Literal + return SimpleExpression.isSimpleExpression(value) + ? new SimpleExpression(variableName, variablePattern) + : null; + } catch (PatternSyntaxException e) { + return null; + } } private static String stripBraces(String expression) { diff --git a/api/src/test/java/feign/template/ExpressionsTest.java b/api/src/test/java/feign/template/ExpressionsTest.java index 0586bd231e..451a3c18a9 100644 --- a/api/src/test/java/feign/template/ExpressionsTest.java +++ b/api/src/test/java/feign/template/ExpressionsTest.java @@ -16,13 +16,53 @@ package feign.template; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatObject; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Collections; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; class ExpressionsTest { + @AfterEach + void clearMaxExpressionLengthProperty() { + System.clearProperty(Expressions.MAX_EXPRESSION_LENGTH_PROPERTY); + } + + @Test + void tooLongExpressionFailsWithDefaultLimit() { + String tooLong = "{" + "a".repeat(10001) + "}"; + assertThatThrownBy(() -> Expressions.create(tooLong)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("expression is too long"); + } + + @Test + void maxExpressionLengthIsConfigurable() { + System.setProperty(Expressions.MAX_EXPRESSION_LENGTH_PROPERTY, "5"); + assertThatThrownBy(() -> Expressions.create("{foobar}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Max length: 5"); + } + + @Test + void lengthCheckCanBeDisabled() { + // An expression well beyond the default 10000 limit, expressed as a name plus a regular + // expression value modifier so the disabled length check is exercised in isolation. + String longExpression = "{name:" + "a".repeat(15000) + "}"; + assertThatThrownBy(() -> Expressions.create(longExpression)) + .as("guarded by default limit") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("expression is too long"); + + System.setProperty(Expressions.MAX_EXPRESSION_LENGTH_PROPERTY, "0"); + assertThatNoException() + .as("length check disabled") + .isThrownBy(() -> Expressions.create(longExpression)); + } + @Test void simpleExpression() { Expression expression = Expressions.create("{foo}"); @@ -44,6 +84,18 @@ void malformedExpression() { } } + @Test + void invalidValueModifierIsTreatedAsLiteral() { + // The text after ':' is compiled as a regex; an invalid one must not escape as a + // PatternSyntaxException, the chunk is a literal instead (Expressions.create returns null). + assertThatNoException().isThrownBy(() -> Expressions.create("{range:[1:10}")); + assertThat(Expressions.create("{a:[}")).isNull(); + assertThat(Expressions.create("{a:(}")).isNull(); + + // a valid value modifier still produces an expression + assertThat(Expressions.create("{id:[0-9]+}")).isNotNull(); + } + @Test void malformedBodyTemplate() { String bodyTemplate = "{" + "a".repeat(65536) + "}"; diff --git a/core/pom.xml b/core/pom.xml index fe620e760a..ce60b8e07e 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -73,7 +73,7 @@ org.springframework spring-context - ${spring-context.version} + ${spring.version} test diff --git a/core/src/test/java/feign/codec/RetryAfterDecoderTest.java b/core/src/test/java/feign/codec/RetryAfterDecoderTest.java index 7d14f0c918..795069780c 100644 --- a/core/src/test/java/feign/codec/RetryAfterDecoderTest.java +++ b/core/src/test/java/feign/codec/RetryAfterDecoderTest.java @@ -54,6 +54,11 @@ void relativeSecondsParseDecimalIntegers() throws Exception { assertThat(decoder.apply("86400.0")).isEqualTo(parseDateTime("Sun, 2 Jan 2000 00:00:00 GMT")); } + @Test + void overflowingRelativeSecondsFailsGracefully() { + assertThat(decoder.apply("99999999999999999999")).isNull(); + } + private Long parseDateTime(String text) { try { return ZonedDateTime.parse(text, RFC_1123_DATE_TIME).toInstant().toEpochMilli(); diff --git a/form-spring/pom.xml b/form-spring/pom.xml index a28bafc604..ea75b7efd8 100644 --- a/form-spring/pom.xml +++ b/form-spring/pom.xml @@ -35,6 +35,18 @@ true + + + + org.springframework.boot + spring-boot-dependencies + ${springboot.version} + pom + import + + + + org.apache.commons @@ -58,7 +70,7 @@ org.springframework spring-web - ${spring-web.version} + ${spring.version} compile diff --git a/form/src/main/java/feign/form/FormEncoder.java b/form/src/main/java/feign/form/FormEncoder.java index 348c21b28b..deb4656733 100644 --- a/form/src/main/java/feign/form/FormEncoder.java +++ b/form/src/main/java/feign/form/FormEncoder.java @@ -27,6 +27,8 @@ import feign.core.codec.DefaultEncoder; import java.lang.reflect.Type; import java.nio.charset.Charset; +import java.nio.charset.IllegalCharsetNameException; +import java.nio.charset.UnsupportedCharsetException; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -129,6 +131,13 @@ private String getContentTypeValue(Map> headers) { private Charset getCharset(String contentTypeValue) { final var matcher = CHARSET_PATTERN.matcher(contentTypeValue); - return matcher.find() ? Charset.forName(matcher.group(1)) : UTF_8; + if (!matcher.find()) { + return UTF_8; + } + try { + return Charset.forName(matcher.group(1)); + } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { + return UTF_8; + } } } diff --git a/form/src/main/java/feign/form/MultipartFormContentProcessor.java b/form/src/main/java/feign/form/MultipartFormContentProcessor.java index 256c9b09e4..73b811ba63 100644 --- a/form/src/main/java/feign/form/MultipartFormContentProcessor.java +++ b/form/src/main/java/feign/form/MultipartFormContentProcessor.java @@ -34,6 +34,7 @@ import feign.form.multipart.Writer; import java.io.IOException; import java.nio.charset.Charset; +import java.security.SecureRandom; import java.util.Collection; import java.util.Collections; import java.util.Deque; @@ -49,6 +50,8 @@ @FieldDefaults(level = PRIVATE, makeFinal = true) public class MultipartFormContentProcessor implements ContentProcessor { + private static final SecureRandom RANDOM = new SecureRandom(); + Deque writers; Writer defaultPerocessor; @@ -75,7 +78,7 @@ public MultipartFormContentProcessor(Encoder delegate) { @Override public void process(RequestTemplate template, Charset charset, Map data) throws EncodeException { - final var boundary = Long.toHexString(System.currentTimeMillis()); + final var boundary = randomBoundary(); try (final var output = new Output(charset)) { for (var entry : data.entrySet()) { if (entry == null || entry.getKey() == null || entry.getValue() == null) { @@ -158,4 +161,15 @@ private Writer findApplicableWriter(Object value) { } return defaultPerocessor; } + + private static String randomBoundary() { + var token = new byte[16]; + RANDOM.nextBytes(token); + var builder = new StringBuilder(token.length * 2); + for (var octet : token) { + builder.append(Character.forDigit((octet >> 4) & 0xF, 16)); + builder.append(Character.forDigit(octet & 0xF, 16)); + } + return builder.toString(); + } } diff --git a/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java b/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java index 09dddb0414..f3527a2543 100644 --- a/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java +++ b/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java @@ -18,7 +18,6 @@ import static feign.form.ContentType.URLENCODED; import feign.CollectionFormat; -import feign.Request; import feign.RequestTemplate; import feign.codec.EncodeException; import java.net.URLEncoder; @@ -74,7 +73,7 @@ public void process(RequestTemplate template, Charset charset, MapemptyList()); // reset header template.header(CONTENT_TYPE_HEADER, contentTypeValue); - template.body(Request.Body.of(bytes)); + template.body(bytes, charset); } @Override @@ -106,7 +105,10 @@ private CharSequence createKeyValuePair( private CharSequence createKeyValuePair( CollectionFormat collectionFormat, String key, Stream values, Charset charset) { final var stringValues = - values.filter(Objects::nonNull).map(Object::toString).collect(Collectors.toList()); + values + .filter(Objects::nonNull) + .map(value -> encode(value, charset)) + .collect(Collectors.toList()); return collectionFormat.join(key, stringValues, charset); } } diff --git a/form/src/main/java/feign/form/multipart/AbstractWriter.java b/form/src/main/java/feign/form/multipart/AbstractWriter.java index b36ca38f94..1913a22256 100644 --- a/form/src/main/java/feign/form/multipart/AbstractWriter.java +++ b/form/src/main/java/feign/form/multipart/AbstractWriter.java @@ -63,10 +63,14 @@ protected void writeFileMetadata( final var contentDespositionBuilder = new StringBuilder() .append("Content-Disposition: form-data; name=\"") - .append(name) + .append(escapeHeaderParameter(name)) .append("\""); if (fileName != null) { - contentDespositionBuilder.append("; ").append("filename=\"").append(fileName).append("\""); + contentDespositionBuilder + .append("; ") + .append("filename=\"") + .append(escapeHeaderParameter(fileName)) + .append("\""); } String fileContentType = contentType; @@ -84,7 +88,7 @@ protected void writeFileMetadata( .append(contentDespositionBuilder.toString()) .append(CRLF) .append("Content-Type: ") - .append(fileContentType) + .append(stripCrlf(fileContentType)) .append(CRLF) .append("Content-Transfer-Encoding: binary") .append(CRLF) @@ -93,4 +97,30 @@ protected void writeFileMetadata( output.write(string); } + + /** + * Escapes a {@code multipart/form-data} header parameter value so an attacker-supplied name or + * file name cannot break out of the quoted string and inject extra headers or part boundaries. + * Carriage return, line feed and double quote are percent-encoded, matching the WHATWG form-data + * encoding rules. + * + * @param value the raw parameter value. + * @return the escaped value, safe to place inside a quoted header parameter. + */ + protected static String escapeHeaderParameter(String value) { + return value.replace("\r", "%0D").replace("\n", "%0A").replace("\"", "%22"); + } + + /** + * Removes carriage return and line feed from a media type so an attacker-supplied content type + * cannot inject extra part headers or boundaries. Unlike a quoted parameter the {@code + * Content-Type} value is not quoted, so it cannot be percent-encoded without corrupting a + * legitimate media type; the control characters are dropped instead. + * + * @param contentType the raw content type value. + * @return the content type with CR and LF removed. + */ + protected static String stripCrlf(String contentType) { + return contentType.replace("\r", "").replace("\n", ""); + } } diff --git a/form/src/main/java/feign/form/multipart/DelegateWriter.java b/form/src/main/java/feign/form/multipart/DelegateWriter.java index 8f539dd111..377d1825c3 100644 --- a/form/src/main/java/feign/form/multipart/DelegateWriter.java +++ b/form/src/main/java/feign/form/multipart/DelegateWriter.java @@ -48,15 +48,27 @@ public boolean isApplicable(Object value) { protected void write(Output output, String key, Object value) throws EncodeException { final var fake = new RequestTemplate(); delegate.encode(value, value.getClass(), fake); - fake.requestBody().ifPresent(body -> write(output, key, body)); + fake.requestBody().ifPresent(body -> write(output, key, body, contentType(fake))); } - private void write(Output output, String key, Request.Body body) { + private void write(Output output, String key, Request.Body body, String contentType) { try { final var encoded = body.writeToString(StandardCharsets.UTF_8).replaceAll("\n", ""); - parameterWriter.write(output, key, encoded); + parameterWriter.writeWithContentType(output, key, encoded, contentType); } catch (IOException e) { throw new EncodeException("Failed to write request body for key: " + key, e); } } + + private static String contentType(RequestTemplate template) { + final var headers = template.headers().get("Content-Type"); + if (headers != null) { + for (var header : headers) { + if (header != null && !header.isEmpty()) { + return header; + } + } + } + return null; + } } diff --git a/form/src/main/java/feign/form/multipart/SingleParameterWriter.java b/form/src/main/java/feign/form/multipart/SingleParameterWriter.java index 30af610d82..33fafb4be8 100644 --- a/form/src/main/java/feign/form/multipart/SingleParameterWriter.java +++ b/form/src/main/java/feign/form/multipart/SingleParameterWriter.java @@ -33,14 +33,31 @@ public boolean isApplicable(Object value) { @Override protected void write(Output output, String key, Object value) throws EncodeException { + writeWithContentType(output, key, value, null); + } + + /** + * Writes a single parameter using the given content type. + * + * @param output output writer. + * @param key name for piece of data. + * @param value piece of data. + * @param contentType the content type of the part. May be {@code null}, in which case {@code + * text/plain} with the output charset is used. + * @throws EncodeException in case of write errors + */ + protected void writeWithContentType(Output output, String key, Object value, String contentType) + throws EncodeException { + final var contentTypeHeader = + contentType != null ? contentType : "text/plain; charset=" + output.getCharset().name(); final var string = new StringBuilder() .append("Content-Disposition: form-data; name=\"") - .append(key) + .append(escapeHeaderParameter(key)) .append('"') .append(CRLF) - .append("Content-Type: text/plain; charset=") - .append(output.getCharset().name()) + .append("Content-Type: ") + .append(stripCrlf(contentTypeHeader)) .append(CRLF) .append(CRLF) .append(value.toString()) diff --git a/form/src/test/java/feign/form/FormEncoderCharsetTest.java b/form/src/test/java/feign/form/FormEncoderCharsetTest.java new file mode 100644 index 0000000000..3eca4d1fcd --- /dev/null +++ b/form/src/test/java/feign/form/FormEncoderCharsetTest.java @@ -0,0 +1,64 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed 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 feign.form; + +import static org.assertj.core.api.Assertions.assertThat; + +import feign.Request; +import feign.RequestTemplate; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class FormEncoderCharsetTest { + + @Test + void illegalCharsetInContentTypeFallsBackToUtf8() { + RequestTemplate template = new RequestTemplate(); + template.header("Content-Type", "application/x-www-form-urlencoded; charset=_bad"); + + Map data = new LinkedHashMap<>(); + data.put("foo", "bar"); + + new FormEncoder().encode(data, Map.class, template); + + assertThat(bodyAsUtf8String(template)).isEqualTo("foo=bar"); + } + + @Test + void unsupportedCharsetInContentTypeFallsBackToUtf8() { + RequestTemplate template = new RequestTemplate(); + template.header("Content-Type", "application/x-www-form-urlencoded; charset=made-up-99"); + + Map data = new LinkedHashMap<>(); + data.put("foo", "bar"); + + new FormEncoder().encode(data, Map.class, template); + + assertThat(bodyAsUtf8String(template)).isEqualTo("foo=bar"); + } + + private static String bodyAsUtf8String(RequestTemplate template) { + Request.Body body = template.requestBody().orElseThrow(); + try { + return body.writeToString(StandardCharsets.UTF_8); + } catch (IOException e) { + throw new AssertionError("Failed to read request body", e); + } + } +} diff --git a/form/src/test/java/feign/form/MultipartBoundaryTest.java b/form/src/test/java/feign/form/MultipartBoundaryTest.java new file mode 100644 index 0000000000..26399fcdc6 --- /dev/null +++ b/form/src/test/java/feign/form/MultipartBoundaryTest.java @@ -0,0 +1,59 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed 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 feign.form; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import feign.RequestTemplate; +import feign.codec.Encoder; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class MultipartBoundaryTest { + + private static final Encoder NOOP_DELEGATE = (object, bodyType, template) -> {}; + + @Test + void boundaryIsNotDerivedFromTheClock() { + long before = System.currentTimeMillis(); + String boundary = generateBoundary(); + long after = System.currentTimeMillis(); + + for (long millis = before; millis <= after; millis++) { + assertThat(boundary).isNotEqualTo(Long.toHexString(millis)); + } + } + + @Test + void boundaryDiffersBetweenRequests() { + assertThat(generateBoundary()).isNotEqualTo(generateBoundary()); + } + + private static String generateBoundary() { + MultipartFormContentProcessor processor = new MultipartFormContentProcessor(NOOP_DELEGATE); + RequestTemplate template = new RequestTemplate(); + + Map data = new LinkedHashMap<>(); + data.put("field", "value"); + processor.process(template, UTF_8, data); + + String contentType = template.headers().get("Content-Type").iterator().next(); + int index = contentType.indexOf("boundary="); + return contentType.substring(index + "boundary=".length()); + } +} diff --git a/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java b/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java index 17cf99ea68..be915fab92 100644 --- a/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java +++ b/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java @@ -46,6 +46,20 @@ void collectionValueUsesDefaultExplodedCollectionFormat() { Arrays.asList("one", "two"), Client::map); } + @Test + void arrayValueEncodesReservedCharacters() { + assertEncodedBody( + "from=%2B987654321&to=%2B123456789&tags=a%26b%3Dc&tags=d", + new String[] {"a&b=c", "d"}, Client::map); + } + + @Test + void collectionValueEncodesReservedCharacters() { + assertEncodedBody( + "from=%2B987654321&to=%2B123456789&tags=a%26b%3Dc&tags=d", + Arrays.asList("a&b=c", "d"), Client::map); + } + @Test void arrayValueUsesCsvCollectionFormat() { assertEncodedBody( diff --git a/form/src/test/java/feign/form/multipart/AbstractWriterTest.java b/form/src/test/java/feign/form/multipart/AbstractWriterTest.java new file mode 100644 index 0000000000..5b00f778f2 --- /dev/null +++ b/form/src/test/java/feign/form/multipart/AbstractWriterTest.java @@ -0,0 +1,74 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed 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 feign.form.multipart; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import feign.form.FormData; +import org.junit.jupiter.api.Test; + +class AbstractWriterTest { + + @Test + void fileNameWithCrlfAndQuoteIsEscaped() { + Output output = new Output(UTF_8); + FormData formData = + new FormData("text/plain", "evil\"\r\nX-Injected: 1", "body".getBytes(UTF_8)); + + new FormDataWriter().write(output, "boundary", "file", formData); + String written = new String(output.toByteArray(), UTF_8); + + assertThat(written).contains("filename=\"evil%22%0D%0AX-Injected: 1\""); + assertThat(written).doesNotContain("\r\nX-Injected: 1"); + } + + @Test + void parameterNameWithCrlfIsEscaped() { + Output output = new Output(UTF_8); + + new SingleParameterWriter().write(output, "boundary", "a\"\r\nX-Injected: 1", "value"); + String written = new String(output.toByteArray(), UTF_8); + + assertThat(written).contains("name=\"a%22%0D%0AX-Injected: 1\""); + assertThat(written).doesNotContain("\r\nX-Injected: 1"); + } + + @Test + void fileContentTypeWithCrlfIsStripped() { + Output output = new Output(UTF_8); + FormData formData = + new FormData("text/plain\r\nX-Injected: 1", "file.txt", "body".getBytes(UTF_8)); + + new FormDataWriter().write(output, "boundary", "file", formData); + String written = new String(output.toByteArray(), UTF_8); + + assertThat(written).contains("Content-Type: text/plainX-Injected: 1"); + assertThat(written).doesNotContain("\r\nX-Injected: 1"); + } + + @Test + void parameterContentTypeWithCrlfIsStripped() { + Output output = new Output(UTF_8); + + new SingleParameterWriter() + .writeWithContentType(output, "name", "value", "text/plain\r\nX-Injected: 1"); + String written = new String(output.toByteArray(), UTF_8); + + assertThat(written).contains("Content-Type: text/plainX-Injected: 1"); + assertThat(written).doesNotContain("\r\nX-Injected: 1"); + } +} diff --git a/form/src/test/java/feign/form/multipart/DelegateWriterTest.java b/form/src/test/java/feign/form/multipart/DelegateWriterTest.java new file mode 100644 index 0000000000..3a3f7a2487 --- /dev/null +++ b/form/src/test/java/feign/form/multipart/DelegateWriterTest.java @@ -0,0 +1,58 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed 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 feign.form.multipart; + +import static org.assertj.core.api.Assertions.assertThat; + +import feign.Request; +import feign.codec.Encoder; +import java.nio.charset.StandardCharsets; +import org.junit.jupiter.api.Test; + +class DelegateWriterTest { + + private static final String BOUNDARY = "boundary"; + + private static final String KEY = "metadata"; + + @Test + void usesContentTypeFromDelegate() throws Exception { + Encoder delegate = + (object, bodyType, template) -> { + template.header("Content-Type", "application/json"); + template.body(Request.Body.of("{\"hash\":\"somehash\"}")); + }; + + assertThat(write(delegate)) + .contains("Content-Type: application/json") + .doesNotContain("Content-Type: text/plain"); + } + + @Test + void fallsBackToTextPlainWhenDelegateSetsNoContentType() throws Exception { + Encoder delegate = (object, bodyType, template) -> template.body(Request.Body.of("plain")); + + assertThat(write(delegate)).contains("Content-Type: text/plain; charset=UTF-8"); + } + + private static String write(Encoder delegate) throws Exception { + DelegateWriter writer = new DelegateWriter(delegate); + try (Output output = new Output(StandardCharsets.UTF_8)) { + writer.write(output, BOUNDARY, KEY, new Object()); + return new String(output.toByteArray(), StandardCharsets.UTF_8); + } + } +} diff --git a/java11/src/main/java/feign/http2client/Http2Client.java b/java11/src/main/java/feign/http2client/Http2Client.java index f1f4bb49b8..060a794538 100644 --- a/java11/src/main/java/feign/http2client/Http2Client.java +++ b/java11/src/main/java/feign/http2client/Http2Client.java @@ -163,6 +163,10 @@ public CompletableFuture execute( protected Response toFeignResponse(Request request, HttpResponse httpResponse) { final OptionalLong length = httpResponse.headers().firstValueAsLong("Content-Length"); + final Integer contentLength = + length.isPresent() && length.getAsLong() >= 0 && length.getAsLong() <= Integer.MAX_VALUE + ? (int) length.getAsLong() + : null; InputStream body = httpResponse.body(); @@ -177,7 +181,7 @@ protected Response toFeignResponse(Request request, HttpResponse ht return Response.builder() .protocolVersion(enumForName(ProtocolVersion.class, httpResponse.version())) - .body(body, length.isPresent() ? (int) length.getAsLong() : null) + .body(body, contentLength) .reason(httpResponse.headers().firstValue("Reason-Phrase").orElse(null)) .request(request) .status(httpResponse.statusCode()) @@ -295,13 +299,26 @@ private BodyPublisher createBodyPublisher(Request.Body body) { * * @see jdk.internal.net.http.common.Utils.DISALLOWED_HEADERS_SET */ - private static final Set DISALLOWED_HEADERS_SET; + private static final Set DISALLOWED_HEADERS_SET = + disallowedHeaders(System.getProperty("jdk.httpclient.allowRestrictedHeaders")); - static { + /** + * Builds the set of headers the underlying JDK HttpClient refuses to send. Mirrors {@code + * jdk.internal.net.http.common.Utils#getDisallowedHeaders()}: headers listed (comma separated) in + * the {@code jdk.httpclient.allowRestrictedHeaders} system property are removed from the set, so + * that callers who opt in at the JDK level (e.g. to set {@code Host}) are not silently filtered + * out here as well. + */ + static Set disallowedHeaders(String allowRestrictedHeaders) { // A case insensitive TreeSet of strings. final TreeSet treeSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); treeSet.addAll(Set.of("connection", "content-length", "expect", "host", "upgrade")); - DISALLOWED_HEADERS_SET = Collections.unmodifiableSet(treeSet); + if (allowRestrictedHeaders != null) { + for (String header : allowRestrictedHeaders.split(",")) { + treeSet.remove(header.trim()); + } + } + return Collections.unmodifiableSet(treeSet); } private Map> filterRestrictedHeaders( diff --git a/java11/src/test/java/feign/http2client/Http2ClientContentLengthTest.java b/java11/src/test/java/feign/http2client/Http2ClientContentLengthTest.java new file mode 100644 index 0000000000..adb8b6b4d9 --- /dev/null +++ b/java11/src/test/java/feign/http2client/Http2ClientContentLengthTest.java @@ -0,0 +1,107 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed 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 feign.http2client; + +import static org.assertj.core.api.Assertions.assertThat; + +import feign.Request; +import feign.Request.HttpMethod; +import feign.Response; +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.net.URI; +import java.net.http.HttpClient.Version; +import java.net.http.HttpHeaders; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import javax.net.ssl.SSLSession; +import org.junit.jupiter.api.Test; + +class Http2ClientContentLengthTest { + + private static HttpResponse responseWithContentLength(String contentLength) { + final HttpHeaders headers = + HttpHeaders.of(Map.of("Content-Length", List.of(contentLength)), (name, value) -> true); + return new HttpResponse<>() { + @Override + public int statusCode() { + return 200; + } + + @Override + public HttpRequest request() { + return null; + } + + @Override + public Optional> previousResponse() { + return Optional.empty(); + } + + @Override + public HttpHeaders headers() { + return headers; + } + + @Override + public InputStream body() { + return new ByteArrayInputStream(new byte[0]); + } + + @Override + public Optional sslSession() { + return Optional.empty(); + } + + @Override + public URI uri() { + return URI.create("http://localhost"); + } + + @Override + public Version version() { + return Version.HTTP_2; + } + }; + } + + private static Response decode(String contentLength) { + final Request request = + Request.create( + HttpMethod.GET, "http://localhost", Collections.emptyMap(), (Request.Body) null, null); + return new Http2Client().toFeignResponse(request, responseWithContentLength(contentLength)); + } + + @Test + void contentLengthAboveIntMaxIsReportedAsUnknown() { + // 2^31, a valid Content-Length larger than Integer.MAX_VALUE + assertThat(decode("2147483648").body().length()).isNull(); + } + + @Test + void negativeContentLengthIsReportedAsUnknown() { + assertThat(decode("-1").body().length()).isNull(); + } + + @Test + void contentLengthWithinIntRangeIsPreserved() { + assertThat(decode("1024").body().length()).isEqualTo(1024); + } +} diff --git a/java11/src/test/java/feign/http2client/Http2ClientHeadersTest.java b/java11/src/test/java/feign/http2client/Http2ClientHeadersTest.java new file mode 100644 index 0000000000..a185d033e8 --- /dev/null +++ b/java11/src/test/java/feign/http2client/Http2ClientHeadersTest.java @@ -0,0 +1,43 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed 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 feign.http2client; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +class Http2ClientHeadersTest { + + @Test + void filtersAllRestrictedHeadersByDefault() { + assertThat(Http2Client.disallowedHeaders(null)) + .contains("connection", "content-length", "expect", "host", "upgrade"); + } + + @Test + void allowsHeaderListedInSystemProperty() { + assertThat(Http2Client.disallowedHeaders("host")) + .doesNotContain("host") + .contains("connection", "content-length", "expect", "upgrade"); + } + + @Test + void allowsMultipleHeadersCaseInsensitivelyAndIgnoresWhitespace() { + assertThat(Http2Client.disallowedHeaders("Host, Connection")) + .doesNotContain("host", "connection") + .contains("content-length", "expect", "upgrade"); + } +} diff --git a/jaxrs3/pom.xml b/jaxrs3/pom.xml index 812176c93e..944dff263b 100644 --- a/jaxrs3/pom.xml +++ b/jaxrs3/pom.xml @@ -31,7 +31,7 @@ 11 - 3.1.11 + 3.1.12 diff --git a/micrometer/pom.xml b/micrometer/pom.xml index 54f7b05859..216e237a2e 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -28,7 +28,7 @@ Feign Micrometer Application Metrics - 1.16.5 + 1.17.0 diff --git a/micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java b/micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java index e49f43d40e..dfa5eaabd3 100644 --- a/micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java +++ b/micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java @@ -18,10 +18,10 @@ import feign.AsyncClient; import feign.Capability; import feign.Client; -import feign.FeignException; import feign.Response; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationRegistry; +import java.util.concurrent.CompletionException; /** Wrap feign {@link Client} with metrics. */ public class MicrometerObservationCapability implements Capability { @@ -55,13 +55,15 @@ public Client enrich(Client client) { this.observationRegistry) .start(); - try { + try (Observation.Scope scope = observation.openScope()) { Response response = client.execute(request, options); - finalizeObservation(feignContext, observation, null, response); + feignContext.setResponse(response); return response; - } catch (FeignException ex) { - finalizeObservation(feignContext, observation, ex, null); + } catch (Throwable ex) { + observation.error(ex); throw ex; + } finally { + observation.stop(); } }; } @@ -80,24 +82,29 @@ public AsyncClient enrich(AsyncClient client) { this.observationRegistry) .start(); - try { + try (Observation.Scope scope = observation.openScope()) { return client .execute(feignContext.getCarrier(), options, context) - .whenComplete((r, ex) -> finalizeObservation(feignContext, observation, ex, r)); - } catch (FeignException ex) { - finalizeObservation(feignContext, observation, ex, null); - + .whenComplete( + (response, ex) -> { + feignContext.setResponse(response); + if (ex != null) { + observation.error(unwrap(ex)); + } + observation.stop(); + }); + } catch (Throwable ex) { + observation.error(ex); + observation.stop(); throw ex; } }; } - private void finalizeObservation( - FeignContext feignContext, Observation observation, Throwable ex, Response response) { - feignContext.setResponse(response); - if (ex != null) { - observation.error(ex); + private static Throwable unwrap(Throwable ex) { + if (ex instanceof CompletionException && ex.getCause() != null) { + return ex.getCause(); } - observation.stop(); + return ex; } } diff --git a/micrometer/src/test/java/feign/micrometer/MicrometerObservationCapabilityTest.java b/micrometer/src/test/java/feign/micrometer/MicrometerObservationCapabilityTest.java new file mode 100644 index 0000000000..b2f75d09ef --- /dev/null +++ b/micrometer/src/test/java/feign/micrometer/MicrometerObservationCapabilityTest.java @@ -0,0 +1,302 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed 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 feign.micrometer; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import feign.AsyncClient; +import feign.AsyncFeign; +import feign.Client; +import feign.Feign; +import feign.RequestLine; +import feign.Retryer; +import feign.Target.HardCodedTarget; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationHandler; +import io.micrometer.observation.tck.TestObservationRegistry; +import io.micrometer.observation.tck.TestObservationRegistryAssert; +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Behavioural tests for {@link MicrometerObservationCapability} focusing on the cases that the + * existing capability did not cover: + * + *
    + *
  • The observation is current (via {@code Observation.Scope}) while the client executes, so + * downstream handlers — most notably tracing handlers that propagate trace/span ids into the + * MDC — see the Feign-scoped observation rather than the parent. + *
  • Exceptions thrown by the underlying client that are not {@code FeignException} — + * {@code IOException}, {@code SocketTimeoutException}, runtime exceptions thrown by a custom + * {@code ErrorDecoder}, and so on — still close the observation and record the error. + *
+ */ +class MicrometerObservationCapabilityTest { + + interface TestClient { + @RequestLine("GET /") + String get(); + } + + interface AsyncTestClient { + @RequestLine("GET /") + CompletableFuture get(); + } + + private TestObservationRegistry observationRegistry; + + @BeforeEach + void setUp() { + this.observationRegistry = TestObservationRegistry.create(); + } + + @Test + void scopeIsOpenDuringClientExecution() { + AtomicReference observedDuringExecute = new AtomicReference<>(); + Client client = + (request, options) -> { + observedDuringExecute.set(observationRegistry.getCurrentObservation()); + return feign.Response.builder() + .status(200) + .reason("OK") + .request(request) + .headers(java.util.Collections.emptyMap()) + .build(); + }; + + TestClient feignClient = + Feign.builder() + .client(client) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(TestClient.class, "http://localhost")); + + feignClient.get(); + + assertThat(observedDuringExecute.get()) + .as("observation must be active (scoped) while the underlying client executes") + .isNotNull(); + assertThat(observationRegistry.getCurrentObservation()) + .as("observation must no longer be current after the call returns") + .isNull(); + } + + @Test + void recordsNonFeignExceptionThrownByClient() { + // Feign's SynchronousMethodHandler wraps IOExceptions, so callers don't see the original + // throwable. The observation captured by the capability sits below that wrapping, and is the + // only place the underlying error is recorded against the trace. + SocketTimeoutException underlying = new SocketTimeoutException("connect timed out"); + + Client client = + (request, options) -> { + throw underlying; + }; + + TestClient feignClient = + Feign.builder() + .client(client) + .retryer(Retryer.NEVER_RETRY) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(TestClient.class, "http://localhost")); + + assertThatThrownBy(feignClient::get).isInstanceOf(Exception.class); + + TestObservationRegistryAssert.assertThat(observationRegistry) + .hasSingleObservationThat() + .hasBeenStopped() + .hasError(underlying); + } + + @Test + void recordsRuntimeExceptionThrownByClient() { + RuntimeException underlying = new RuntimeException("boom"); + + Client client = + (request, options) -> { + throw underlying; + }; + + TestClient feignClient = + Feign.builder() + .client(client) + .retryer(Retryer.NEVER_RETRY) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(TestClient.class, "http://localhost")); + + assertThatThrownBy(feignClient::get).isInstanceOf(Exception.class); + + TestObservationRegistryAssert.assertThat(observationRegistry) + .hasSingleObservationThat() + .hasBeenStopped() + .hasError(underlying); + } + + @Test + void asyncScopeIsOpenDuringClientExecution() { + AtomicReference observedDuringExecute = new AtomicReference<>(); + AsyncClient client = + (request, options, context) -> { + observedDuringExecute.set(observationRegistry.getCurrentObservation()); + return CompletableFuture.completedFuture( + feign.Response.builder() + .status(200) + .reason("OK") + .request(request) + .headers(java.util.Collections.emptyMap()) + .build()); + }; + + AsyncTestClient feignClient = + AsyncFeign.builder() + .client(client) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(AsyncTestClient.class, "http://localhost")); + + feignClient.get().join(); + + assertThat(observedDuringExecute.get()) + .as("observation must be active while the async client kicks off the request") + .isNotNull(); + } + + @Test + void asyncRecordsNonFeignExceptionFromFailedFuture() { + IOException underlying = new IOException("connection reset"); + + AsyncClient client = + (request, options, context) -> { + CompletableFuture failed = new CompletableFuture<>(); + failed.completeExceptionally(underlying); + return failed; + }; + + AsyncTestClient feignClient = + AsyncFeign.builder() + .client(client) + .retryer(Retryer.NEVER_RETRY) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(AsyncTestClient.class, "http://localhost")); + + assertThatThrownBy(() -> feignClient.get().join()).isInstanceOf(CompletionException.class); + + TestObservationRegistryAssert.assertThat(observationRegistry) + .hasSingleObservationThat() + .hasBeenStopped() + .hasError(underlying); + } + + @Test + void asyncRecordsSynchronousExceptionFromClient() { + RuntimeException underlying = new RuntimeException("immediate failure"); + + AsyncClient client = + (request, options, context) -> { + throw underlying; + }; + + AsyncTestClient feignClient = + AsyncFeign.builder() + .client(client) + .retryer(Retryer.NEVER_RETRY) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(AsyncTestClient.class, "http://localhost")); + + assertThatThrownBy(feignClient::get).isInstanceOf(RuntimeException.class); + + TestObservationRegistryAssert.assertThat(observationRegistry) + .hasSingleObservationThat() + .hasBeenStopped() + .hasError(underlying); + } + + @Test + void parentObservationIsRestoredAfterCall() { + Observation parent = Observation.start("parent", observationRegistry); + try (Observation.Scope ignored = parent.openScope()) { + + Client client = + (request, options) -> + feign.Response.builder() + .status(200) + .reason("OK") + .request(request) + .headers(java.util.Collections.emptyMap()) + .build(); + + TestClient feignClient = + Feign.builder() + .client(client) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(TestClient.class, "http://localhost")); + + feignClient.get(); + + assertThat(observationRegistry.getCurrentObservation()) + .as("parent observation must still be current after the Feign call completes") + .isSameAs(parent); + } finally { + parent.stop(); + } + } + + @Test + @SuppressWarnings("unchecked") + void scopeReceivesObservationCarryingFeignContext() { + AtomicReference seenContext = new AtomicReference<>(); + + observationRegistry + .observationConfig() + .observationHandler( + new ObservationHandler() { + @Override + public void onScopeOpened(Observation.Context context) { + seenContext.set(context); + } + + @Override + public boolean supportsContext(Observation.Context context) { + return true; + } + }); + + Client client = + (request, options) -> + feign.Response.builder() + .status(200) + .reason("OK") + .request(request) + .headers(java.util.Collections.emptyMap()) + .build(); + + TestClient feignClient = + Feign.builder() + .client(client) + .addCapability(new MicrometerObservationCapability(observationRegistry)) + .target(new HardCodedTarget<>(TestClient.class, "http://localhost")); + + feignClient.get(); + + assertThat(seenContext.get()) + .as("scope must propagate the FeignContext to ObservationHandler#onScopeOpened") + .isInstanceOf(FeignContext.class); + } +} diff --git a/pom.xml b/pom.xml index 2e032f796d..f85a2977da 100644 --- a/pom.xml +++ b/pom.xml @@ -165,18 +165,18 @@ ${main.java.version} ${main.java.version} - 5.3.2 + 5.4.0 33.6.0-jre 2.1.0 2.14.0 1.15.2 2.0.18 20260522 - 4.0.6 + 4.1.0 6.1.0 - 2.21.3 - 3.1.3 + 2.22.0 + 3.2.0 3.27.7 5.23.0 2.0.61.android8 @@ -192,7 +192,7 @@ 3.3.1 6.0.2 0.1.1 - 0.10.0 + 0.11.0 3.5.6 0.300.0 file://${project.basedir}/src/config/bom.xml @@ -206,9 +206,9 @@ 3.2.0 1.2.8 4.0.0 - 6.41.0 - 3.37.0 - 3.36.0 + 6.42.0 + 3.40.0 + 3.38.0 0.26.1 1.0 @@ -221,7 +221,7 @@ 1.15.0 0.23.0 26.0 - 4.5.1 + 4.5.2 4.5.14 5.6.1 1.13.0 @@ -238,8 +238,8 @@ 2.3.3 2.0.1.Final 3.1.1 - 6.2.5.Final - 8.0.1.Final + 6.2.5.Final + 9.1.0.Final 3.0.4 6.0.0 1.19.4 @@ -252,10 +252,9 @@ 2.1.1 1.5.3 3.0.2 - 2025.1.1 - 5.0.1 - 7.0.7 - 7.0.7 + 2025.1.2 + 5.0.2 + 7.0.8 2.4.1.Final 1.18.0 diff --git a/reactive/pom.xml b/reactive/pom.xml index dd8b4e40a1..4d549d606b 100644 --- a/reactive/pom.xml +++ b/reactive/pom.xml @@ -29,7 +29,7 @@ Reactive Wrapper for Feign Clients - 3.8.5 + 3.8.6 1.0.4 2.2.21 diff --git a/spring/pom.xml b/spring/pom.xml index 0056fe32a0..340ae2716e 100644 --- a/spring/pom.xml +++ b/spring/pom.xml @@ -31,7 +31,6 @@ 17 - 7.0.7 diff --git a/spring4/pom.xml b/spring4/pom.xml index cb063d459b..b5486a0546 100644 --- a/spring4/pom.xml +++ b/spring4/pom.xml @@ -36,10 +36,6 @@ - - 4.3.30.RELEASE - - io.github.openfeign diff --git a/validation-jakarta/pom.xml b/validation-jakarta/pom.xml index 6641433959..8742b86a3f 100644 --- a/validation-jakarta/pom.xml +++ b/validation-jakarta/pom.xml @@ -63,7 +63,7 @@ org.hibernate.validator hibernate-validator - ${hibernate-validator-8.version} + ${hibernate-validator-jakarta.version} test diff --git a/validation/pom.xml b/validation/pom.xml index 52efe99814..c7257195fd 100644 --- a/validation/pom.xml +++ b/validation/pom.xml @@ -57,7 +57,7 @@ org.hibernate.validator hibernate-validator - ${hibernate-validator-6.version} + ${hibernate-validator-javax.version} test diff --git a/vertx/feign-vertx/pom.xml b/vertx/feign-vertx/pom.xml index 01257476d5..f63715516e 100644 --- a/vertx/feign-vertx/pom.xml +++ b/vertx/feign-vertx/pom.xml @@ -31,7 +31,7 @@ 11 - 2.21.3 + 2.22.0 diff --git a/vertx/feign-vertx5-test/pom.xml b/vertx/feign-vertx5-test/pom.xml index 9f26531b02..c0431a64df 100644 --- a/vertx/feign-vertx5-test/pom.xml +++ b/vertx/feign-vertx5-test/pom.xml @@ -30,7 +30,7 @@ Tests with Vertx 5.x. - 5.1.1 + 5.1.3