Affected version
3.5.3 and master — the file is identical on both branches.
Bug description
PUT sends credentials preemptively whether or not usePreemptive is configured, and the code says as much. AbstractHttpClientWagon.put() at :688:
// preemptive for put
// TODO: is it a good idea, though? 'Expect-continue' handshake would serve much better
// FIXME Perform only when preemptive has been configured
Repository repo = getRepository();
HttpHost targetHost = new HttpHost(repo.getHost(), repo.getPort(), repo.getProtocol());
AuthScope targetScope = getBasicAuthScope().getScope(targetHost);
if (credentialsProvider.getCredentials(targetScope) != null) {
BasicScheme targetAuth = new BasicScheme(StandardCharsets.UTF_8);
authCache.put(targetHost, targetAuth);
}The same body appears in execute() at :874, correctly guarded:
if (config != null && config.isUsePreemptive()) {So every mvn deploy to an HTTP repository with credentials sends Authorization: Basic on the first PUT without being challenged. The impact is mild — the credentials go to the server they were configured for — but usePreemptive=false is not honoured, and over plain http:// it is an unrequested plaintext credential send.
A fix must also change a test
The suite asserts the current behaviour, so removing the unguarded block will look like it broke the build unless the test moves with it:
HttpWagonTest.java:51-53 declares supportPreemptiveAuthenticationPut() returning true, and that class never sets usePreemptive.HttpWagonTestCase.java:1562 and :1629 feed that into testPreemptiveAuthenticationPut.HttpWagonTestCase.java:1654-1666, with preemptive true, asserts exactly one request/response pair — i.e. that no challenge round trip happened.
Worth deciding at the same time
The TODO proposes expect-continue as the better mechanism, and execute() already enables it unconditionally for PUT at :859. Both entered in the same 2017 commit (8fadff84, WAGON-488), so neither superseded the other; they simply coexist. Whoever fixes this should decide whether the unguarded preemptive block is still wanted at all now that expect-continue is always on.
Related to #906, which is the same setting being lost through a different route.
Found while writing the HTTP configuration guide in #905.
Affected version
3.5.3 and
master— the file is identical on both branches.Bug description
PUT sends credentials preemptively whether or not
usePreemptiveis configured, and the code says as much.AbstractHttpClientWagon.put()at :688:The same body appears in
execute()at :874, correctly guarded:So every
mvn deployto an HTTP repository with credentials sendsAuthorization: Basicon the first PUT without being challenged. The impact is mild — the credentials go to the server they were configured for — butusePreemptive=falseis not honoured, and over plainhttp://it is an unrequested plaintext credential send.A fix must also change a test
The suite asserts the current behaviour, so removing the unguarded block will look like it broke the build unless the test moves with it:
HttpWagonTest.java:51-53declaressupportPreemptiveAuthenticationPut()returningtrue, and that class never setsusePreemptive.HttpWagonTestCase.java:1562and:1629feed that intotestPreemptiveAuthenticationPut.HttpWagonTestCase.java:1654-1666, with preemptive true, asserts exactly one request/response pair — i.e. that no challenge round trip happened.Worth deciding at the same time
The TODO proposes expect-continue as the better mechanism, and
execute()already enables it unconditionally for PUT at :859. Both entered in the same 2017 commit (8fadff84, WAGON-488), so neither superseded the other; they simply coexist. Whoever fixes this should decide whether the unguarded preemptive block is still wanted at all now that expect-continue is always on.Related to #906, which is the same setting being lost through a different route.
Found while writing the HTTP configuration guide in #905.