BasicAuthScope ignores realm=ANY because both branches of the test are identical

[slachiewicz]

Affected version

3.5.3 and master — BasicAuthScope.java is identical on both branches.

Bug description

<realm>ANY</realm> does not mean "any realm". It sets the realm to the literal four-character string ANY, which then matches only a server whose realm is actually called ANY. Host and port do honour ANY; realm does not.

The cause is a branch whose two arms are the same, BasicAuthScope.java:105-112:

String scopeRealm = AuthScope.ANY_REALM;
if (getRealm() != null) {
    if ("ANY".compareTo(getRealm()) != 0) {
        scopeRealm = getRealm();
    } else {
        scopeRealm = getRealm();     // <- should be AuthScope.ANY_REALM
    }
}

Compare host and port immediately above, which get it right — :86-90 maps ANY to AuthScope.ANY_HOST and :97-102 maps it to AuthScope.ANY_PORT. In HttpClient 4.5.14 ANY_HOST, ANY_REALM and ANY_SCHEME are all null and ANY_PORT is -1, so the realm case ends up strictly narrower than the default it started from.

The way to get "any realm" today is to omit realm, because the initialiser at :105 already starts at AuthScope.ANY_REALM.

Why it matters more than the size of the fix suggests

Someone reaches for this setting precisely because credentials are not being sent. Of the values available, ANY is the one that reads like "stop being fussy" — and it makes the scope narrower instead of wider, silently. There is no error; the credentials simply continue not to be sent.

The one case that does work

All three set to ANY behaves correctly, but only by accident of the early return at :76-83, which short-circuits to AuthScope.ANY before any of this code runs.

Test coverage

BasicAuthScopeTest.testGetScopeAllAny asserts assertEquals(AuthScope.ANY_REALM, authScope.getRealm()) and passes — but it sets all three to ANY, so it returns through the early return and never reaches the realm branch. It appears to cover this and does not. There is no test for realm=ANY on its own.

Note for whoever fixes it

Deleting the dead else is a one-line change, but it is a behaviour change, not a cleanup: anyone who wrote <realm>ANY</realm> and unknowingly depends on the literal match would see their scope widen. Worth a release note.

The javadoc on that method also documented three element names that match nothing — /server/proxyBasicAuth, /server/basicAuthentication/realm and /repository/password. That part is corrected in #905, which documents current behaviour including this asymmetry; this issue is about the code.