You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "dhruv9b (via GitHub)" <gi...@apache.org> on 2026/10/06 16:58:26 UTC
[PR] [MRESOLVER-1736] Re-enable expect-continue on JDK 17.0.17+ [maven-resolver]
dhruv9b opened a new pull request, #2180:
URL: https://github.com/apache/maven-resolver/pull/2180
## Description
OpenJDK fixed the `Expect: 100-continue` issue tracked by JDK-8286171 in JDK 17.0.17.
Maven Resolver currently disables `Expect-Continue` for all Java versions below 20 based only on the Java feature version. As a result, `Expect-Continue` remains disabled on JDK 17.0.17 and later, even though the underlying issue has been fixed.
This change makes the Java version check version-aware by using `Runtime.Version`:
* JDK 17.0.0–17.0.16: `Expect-Continue` disabled
* JDK 17.0.17+: `Expect-Continue` enabled
* JDK 18.x–19.x: `Expect-Continue` disabled
* JDK 20+: `Expect-Continue` enabled
A regression test was added to verify the version-specific behavior and to ensure that an explicitly enabled `Expect: 100-continue` header is sent on JDK 17.0.17+.
The version matrix test covers JDK 11, 17.0.16, 17.0.17, 17.0.18, 18, 19, 20, and 21.
The regression test fails with the previous implementation on JDK 17.0.17 and passes with this change.
- [ ] I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004
- [ ] I hereby declare that I have filed an Apache Individual Contributor License Agreement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MRESOLVER-1736] Re-enable expect-continue on JDK 17.0.17+ [maven-resolver]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #2180:
URL: https://github.com/apache/maven-resolver/pull/2180#issuecomment-6130575439
## CI failure analysis
The two macOS failures are **not caused by this PR**. Here's the breakdown:
### What failed
Both failures are in `maven-resolver-transport-jetty` — a completely separate module from the JDK transport code this PR touches:
| Job | Test | Assertion |
|---|---|---|
| macOS + JDK 25 + 4.0.0-rc-6 | `JettyTransporterTest.testPut_ProxyAuthenticated` | `progressedCount > 0` → got `0` |
| macOS + JDK 26 + 4.0.0-rc-6 | `JettyTransporterTest.testPut_SSL` | `progressedCount > 0` → got `0` |
### Why these are pre-existing flakies
This PR modifies exactly 3 files:
- `JdkTransporter.java`
- `JdkTransporterFactory.java`
- `JdkTransporterTest.java`
None of these touch `maven-resolver-transport-jetty` or `maven-resolver-test-http`. The `progressedCount = 0` failure is a known race condition on macOS where progress listener callbacks are not fired during proxy auth challenges or TLS handshakes in the embedded test server. Linux and Windows runners pass cleanly.
I've filed https://github.com/apache/maven-resolver/issues/2203 to track the flakiness.
### Recommendation
Re-run the two failed macOS jobs. They will pass on a clean run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MRESOLVER-1736] Re-enable expect-continue on JDK 17.0.17+ [maven-resolver]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #2180:
URL: https://github.com/apache/maven-resolver/pull/2180#discussion_r4227160215
##########
maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java:
##########
@@ -217,13 +229,13 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
this.connectTimeout = HttpTransporterUtils.getHttpConnectTimeout(session, repository);
this.requestTimeout = HttpTransporterUtils.getHttpRequestTimeout(session, repository);
Optional<Boolean> expectContinue = HttpTransporterUtils.getHttpExpectContinue(session, repository);
- if (javaVersion > 19) {
+ if (isExpectContinueSupported(javaVersion)) {
this.expectContinue = expectContinue.orElse(null);
} else {
this.expectContinue = null;
if (expectContinue.isPresent()) {
LOGGER.warn(
- "Configuration for Expect-Continue set but is ignored on Java versions below 20 (current java version is {}) due https://bugs.openjdk.org/browse/JDK-8286171",
+ "Configuration for Expect-Continue set but is ignored on Java version {} due https://bugs.openjdk.org/browse/JDK-8286171",
Review Comment:
⚠️ **Medium — warning message lost threshold context**
The previous message:
> `"Configuration for Expect-Continue set but is ignored on Java versions below 20 (current java version is {}) due https://bugs.openjdk.org/browse/JDK-8286171"`
The new message:
> `"Configuration for Expect-Continue set but is ignored on Java version {} due https://bugs.openjdk.org/browse/JDK-8286171"`
The new message is ambiguous — it no longer tells users *why* their version is unsupported. Consider something like:
```suggestion
"Configuration for Expect-Continue set but is ignored on Java version {} (supported on 17.0.17+ and 20+) due https://bugs.openjdk.org/browse/JDK-8286171",
```
##########
maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/test/java/org/eclipse/aether/transport/jdk/JdkTransporterTest.java:
##########
@@ -172,4 +178,41 @@ void testMaximumHttpVersionAtRuntime() throws Exception {
assertEquals(Version.HTTP_2, jdkTransporter.getHttpVersion(session, remoteRepository));
}
}
+
+ /*
+ @ParameterizedTest
+ @CsvSource({
+ "11.0.20, false",
+ "17.0.16, false",
+ "17.0.17, true",
+ "17.0.18, true",
+ "18.0.2, false",
+ "19.0.2, false",
+ "20, true",
+ "21.0.2, true"
+ })
+ void testIsExpectContinueSupported(String versionStr, boolean expected) {
+ assertEquals(expected, JdkTransporter.isExpectContinueSupported(Runtime.Version.parse(versionStr)));
+ }
+ */
+
+ @Test
+ void testPut_ExpectContinueExplicitlyEnabledOnJava17() throws Exception {
+ Runtime.Version version = Runtime.version();
+ Assumptions.assumeTrue(version.feature() == 17 && version.compareTo(Runtime.Version.parse("17.0.17")) >= 0);
Review Comment:
⚠️ **High — commented-out parametrized test is the only boundary-case coverage**
The `/* ... */` block is the only test that exercises all version boundaries for `isExpectContinueSupported` (11, 17.0.16, 17.0.17, 18, 19, 20, 21). Please uncomment it and convert it to a proper `@ParameterizedTest`.
The active integration test below (`testPut_ExpectContinueExplicitlyEnabledOnJava17`) is valid as a regression test but has narrow coverage: it skips entirely on JDK 18, 19, 20, and 21 via the `Assumptions.assumeTrue(version.feature() == 17 && ...)` guard. It doesn't exercise the JDK 20+ path at all.
##########
maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java:
##########
@@ -134,6 +134,18 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
"EEE, dd MMM yyyy HH:mm:ss z", Locale.ENGLISH)
.withZone(ZoneId.of("GMT"));
+ private static final Runtime.Version JAVA_17_0_17 = Runtime.Version.parse("17.0.17");
+
+ static boolean isExpectContinueSupported(Runtime.Version version) {
Review Comment:
💡 **Low — missing Javadoc on `isExpectContinueSupported`**
This method is package-private and has non-trivial logic (JDK 17.0.17 boundary). A short Javadoc explaining the version matrix would help future readers:
```java
/**
* Returns {@code true} if the given JVM version supports the {@code Expect: 100-continue}
* header. Disabled on JDK 18.x and 19.x (unfixed JDK-8286171), and on JDK 17 before
* 17.0.17 (backport JDK-8364017). Enabled on 17.0.17+, and all versions 20+.
*/
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MRESOLVER-1736] Re-enable expect-continue on JDK 17.0.17+ [maven-resolver]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #2180:
URL: https://github.com/apache/maven-resolver/pull/2180
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MRESOLVER-1736] Re-enable expect-continue on JDK 17.0.17+ [maven-resolver]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #2180:
URL: https://github.com/apache/maven-resolver/pull/2180#issuecomment-6128776731
Reformat needed. Pls run `mvn spotless:apply` and commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org