Skip to content

Commit

Permalink
[MRESOLVER-427] Config for expect/continue inhibition (#363)
Browse files Browse the repository at this point in the history
Add configuration to inhibit expect/continue handshake. Currently supported by "apache" and "jdk" HTTP transports.

---

https://issues.apache.org/jira/browse/MRESOLVER-427
  • Loading branch information
cstamas committed Nov 14, 2023
1 parent 2fa9af7 commit 178cfba
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,22 @@ public final class ConfigurationProperties {
*/
public static final boolean DEFAULT_HTTP_PREEMPTIVE_PUT_AUTH = true;

/**
* Boolean flag should the HTTP transport use expect-continue handshake for PUT requests. Not all transport support
* this option. This option may be needed for some broken HTTP servers.
*
* @see #DEFAULT_HTTP_EXPECT_CONTINUE
* @since 2.0.0
*/
public static final String HTTP_EXPECT_CONTINUE = PREFIX_CONNECTOR + "http.expectContinue";

/**
* Default value if {@link #HTTP_EXPECT_CONTINUE} is not set: {@code true}.
*
* @since 2.0.0
*/
public static final boolean DEFAULT_HTTP_EXPECT_CONTINUE = true;

/**
* The mode that sets HTTPS transport "security mode": to ignore any SSL errors (certificate validity checks,
* hostname verification). The default value is {@link #HTTPS_SECURITY_MODE_DEFAULT}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,15 @@ final class HttpTransporter extends AbstractTransporter {
builder.useSystemProperties();
}

final boolean expectContinue = ConfigUtils.getBoolean(
session,
ConfigurationProperties.DEFAULT_HTTP_EXPECT_CONTINUE,
ConfigurationProperties.HTTP_EXPECT_CONTINUE + "." + repository.getId(),
ConfigurationProperties.HTTP_EXPECT_CONTINUE);
if (expectContinue != ConfigurationProperties.DEFAULT_HTTP_EXPECT_CONTINUE) {
state.setExpectContinue(expectContinue);
}

final boolean reuseConnections = ConfigUtils.getBoolean(
session,
ConfigurationProperties.DEFAULT_HTTP_REUSE_CONNECTIONS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,27 @@ void testPut_Authenticated_ExpectContinueRejected() throws Exception {
assertEquals("upload", TestFileUtils.readString(new File(repoDir, "file.txt")));
}

@Test
void testPut_Authenticated_ExpectContinueDisabled() throws Exception {
session.setConfigProperty(ConfigurationProperties.HTTP_EXPECT_CONTINUE, false);
httpServer.setAuthentication("testuser", "testpass");
httpServer.setExpectSupport(HttpServer.ExpectContinue.FAIL); // if transport tries Expect/Continue explode
auth = new AuthenticationBuilder()
.addUsername("testuser")
.addPassword("testpass")
.build();
newTransporter(httpServer.getHttpUrl());
RecordingTransportListener listener = new RecordingTransportListener();
PutTask task =
new PutTask(URI.create("repo/file.txt")).setListener(listener).setDataString("upload");
transporter.put(task);
assertEquals(0L, listener.dataOffset);
assertEquals(6L, listener.dataLength);
assertEquals(1, listener.startedCount); // w/ expectContinue enabled would have here 2
assertTrue(listener.progressedCount > 0, "Count: " + listener.progressedCount);
assertEquals("upload", TestFileUtils.readString(new File(repoDir, "file.txt")));
}

@Test
void testPut_Authenticated_ExpectContinueRejected_ExplicitlyConfiguredHeader() throws Exception {
Map<String, String> headers = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ final class JdkHttpTransporter extends AbstractTransporter {

private final int requestTimeout;

private final boolean expectContinue;

JdkHttpTransporter(RepositorySystemSession session, RemoteRepository repository) throws NoTransporterException {
try {
URI uri = new URI(repository.getUrl()).parseServerAuthority();
Expand Down Expand Up @@ -149,6 +151,11 @@ final class JdkHttpTransporter extends AbstractTransporter {
ConfigurationProperties.DEFAULT_REQUEST_TIMEOUT,
ConfigurationProperties.REQUEST_TIMEOUT + "." + repository.getId(),
ConfigurationProperties.REQUEST_TIMEOUT);
this.expectContinue = ConfigUtils.getBoolean(
session,
ConfigurationProperties.DEFAULT_HTTP_EXPECT_CONTINUE,
ConfigurationProperties.HTTP_EXPECT_CONTINUE + "." + repository.getId(),
ConfigurationProperties.HTTP_EXPECT_CONTINUE);

this.headers = headers;
this.client = getOrCreateClient(session, repository);
Expand Down Expand Up @@ -280,8 +287,10 @@ protected void implGet(GetTask task) throws Exception {

@Override
protected void implPut(PutTask task) throws Exception {
HttpRequest.Builder request =
HttpRequest.newBuilder().uri(resolve(task)).timeout(Duration.ofMillis(requestTimeout));
HttpRequest.Builder request = HttpRequest.newBuilder()
.uri(resolve(task))
.timeout(Duration.ofMillis(requestTimeout))
.expectContinue(expectContinue);
headers.forEach(request::setHeader);
try (FileUtils.TempFile tempFile = FileUtils.newTempFile()) {
utilPut(task, Files.newOutputStream(tempFile.getPath()), true);
Expand Down
1 change: 1 addition & 0 deletions src/site/markdown/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Option | Type | Description | Default Value | Supports Repo ID Suffix
`aether.connector.http.cacheState` | boolean | Flag indicating whether a memory-based cache is used for user tokens, connection managers, expect continue requests and authentication schemes. | `true` | no
`aether.connector.http.connectionMaxTtl` | int | Total time to live in seconds for an HTTP connection, after that time, the connection will be dropped (no matter for how long it was idle). | `300` | yes
`aether.connector.http.credentialEncoding` | String | The encoding/charset to use when exchanging credentials with HTTP servers. | `"ISO-8859-1"` | yes
`aether.connector.http.expectContinue` | boolean | Whether to use expect/continue handshake during PUTs. Some broken HTTP servers needs this disabled. | `true` | yes
`aether.connector.http.headers` | `Map<String, String>` | The request headers to use for HTTP-based repository connectors. The headers are specified using a map of strings mapping a header name to its value. The repository-specific headers map is supposed to be complete, i.e. is not merged with the general headers map. | - | yes
`aether.connector.http.localAddress` | String | Set the outgoing interface (globally or per remote repository). Valid values are local accessible IP addresses or host names. The default will use the system's default route. Invalid addresses will result in HttpTransport creation failure. | - | yes
`aether.connector.http.maxConnectionsPerRoute` | int | The maximum concurrent connections per route HTTP client is allowed to use. | `50` | yes
Expand Down

0 comments on commit 178cfba

Please sign in to comment.