HTTPCLIENT-2045: BASIC auth scheme conformance to RFC 7617

httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java

@@ -0,0 +1,50 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl.auth;
+
+import java.nio.charset.Charset;
+import java.nio.charset.UnsupportedCharsetException;
+
+import org.apache.hc.client5.http.auth.AuthenticationException;
+import org.apache.hc.core5.annotation.Internal;
+
+/**
+ * @since 5.2
+ */
+@Internal
+public class AuthSchemeSupport {
+
+    public static Charset parseCharset(
+            final String charsetName, final Charset defaultCharset) throws AuthenticationException {
+        try {
+            return charsetName != null ? Charset.forName(charsetName) : defaultCharset;
+        } catch (final UnsupportedCharsetException ex) {
+            throw new AuthenticationException("Unsupported charset: " + charsetName);
+        }
+    }
+
+}

httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java

@@ -42,13 +42,13 @@ import java.util.Map;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.hc.client5.http.auth.AuthChallenge;
 import org.apache.hc.client5.http.auth.AuthScheme;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.AuthScope;
 import org.apache.hc.client5.http.auth.AuthStateCacheable;
 import org.apache.hc.client5.http.auth.AuthenticationException;
 import org.apache.hc.client5.http.auth.Credentials;
 import org.apache.hc.client5.http.auth.CredentialsProvider;
 import org.apache.hc.client5.http.auth.MalformedChallengeException;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.client5.http.utils.ByteArrayBuilder;
 import org.apache.hc.core5.http.HttpHost;
@@ -72,7 +72,7 @@ public class BasicScheme implements AuthScheme, Serializable {
     private static final Logger LOG = LoggerFactory.getLogger(BasicScheme.class);
 
     private final Map<String, String> paramMap;
-    private transient Charset charset;
+    private transient Charset defaultCharset;
     private transient ByteArrayBuilder buffer;
     private transient Base64 base64codec;
     private boolean complete;
@@ -85,7 +85,7 @@ public class BasicScheme implements AuthScheme, Serializable {
      */
     public BasicScheme(final Charset charset) {
         this.paramMap = new HashMap<>();
-        this.charset = charset != null ? charset : StandardCharsets.US_ASCII;
+        this.defaultCharset = charset != null ? charset : StandardCharsets.US_ASCII;
         this.complete = false;
     }
 
@@ -93,13 +93,21 @@ public class BasicScheme implements AuthScheme, Serializable {
         this(StandardCharsets.US_ASCII);
     }
 
+    private void applyCredentials(final Credentials credentials) {
+        this.username = credentials.getUserPrincipal().getName();
+        this.password = credentials.getPassword();
+    }
+
+    private void clearCredentials() {
+        this.username = null;
+        this.password = null;
+    }
+
     public void initPreemptive(final Credentials credentials) {
         if (credentials != null) {
-            this.username = credentials.getUserPrincipal().getName();
+            applyCredentials(credentials);
-            this.password = credentials.getPassword();
         } else {
-            this.username = null;
+            clearCredentials();
-            this.password = null;
         }
     }
 
@@ -150,8 +158,7 @@ public class BasicScheme implements AuthScheme, Serializable {
         final Credentials credentials = credentialsProvider.getCredentials(
                 authScope, context);
         if (credentials != null) {
-            this.username = credentials.getUserPrincipal().getName();
+            applyCredentials(credentials);
-            this.password = credentials.getPassword();
             return true;
         }
 
@@ -160,8 +167,7 @@ public class BasicScheme implements AuthScheme, Serializable {
             final String exchangeId = clientContext.getExchangeId();
             LOG.debug("{} No credentials found for auth scope [{}]", exchangeId, authScope);
         }
-        this.username = null;
+        clearCredentials();
-        this.password = null;
         return false;
     }
 
@@ -170,16 +176,34 @@ public class BasicScheme implements AuthScheme, Serializable {
         return null;
     }
 
+    private void validateUsername() throws AuthenticationException {
+        if (username == null) {
+            throw new AuthenticationException("User credentials not set");
+        }
+        for (int i = 0; i < username.length(); i++) {
+            final char ch = username.charAt(i);
+            if (Character.isISOControl(ch)) {
+                throw new AuthenticationException("Username must not contain any control characters");
+            }
+            if (ch == ':') {
+                throw new AuthenticationException("Username contains a colon character and is invalid");
+            }
+        }
+    }
+
     @Override
     public String generateAuthResponse(
             final HttpHost host,
             final HttpRequest request,
             final HttpContext context) throws AuthenticationException {
+        validateUsername();
         if (this.buffer == null) {
-            this.buffer = new ByteArrayBuilder(64).charset(this.charset);
+            this.buffer = new ByteArrayBuilder(64);
         } else {
             this.buffer.reset();
         }
+        final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), defaultCharset);
+        this.buffer.charset(charset);
         this.buffer.append(this.username).append(":").append(this.password);
         if (this.base64codec == null) {
             this.base64codec = new Base64(0);
@@ -191,16 +215,16 @@ public class BasicScheme implements AuthScheme, Serializable {
 
     private void writeObject(final ObjectOutputStream out) throws IOException {
         out.defaultWriteObject();
-        out.writeUTF(this.charset.name());
+        out.writeUTF(this.defaultCharset.name());
     }
 
     @SuppressWarnings("unchecked")
     private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException {
         in.defaultReadObject();
         try {
-            this.charset = Charset.forName(in.readUTF());
+            this.defaultCharset = Charset.forName(in.readUTF());
         } catch (final UnsupportedCharsetException ex) {
-            this.charset = StandardCharsets.US_ASCII;
+            this.defaultCharset = StandardCharsets.US_ASCII;
         }
     }
 

httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java

@@ -32,7 +32,6 @@ import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.nio.charset.UnsupportedCharsetException;
 import java.security.MessageDigest;
 import java.security.Principal;
 import java.security.SecureRandom;
@@ -275,14 +274,7 @@ public class DigestScheme implements AuthScheme, Serializable {
             throw new AuthenticationException("None of the qop methods is supported: " + qoplist);
         }
 
-        final String charsetName = this.paramMap.get("charset");
+        final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), defaultCharset);
-        final Charset charset;
-        try {
-            charset = charsetName != null ? Charset.forName(charsetName) : defaultCharset;
-        } catch (final UnsupportedCharsetException ex) {
-            throw new AuthenticationException("Unsupported charset: " + charsetName);
-        }
-
         String digAlg = algorithm;
         if (digAlg.equalsIgnoreCase("MD5-sess")) {
             digAlg = "MD5";

httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java

@@ -36,9 +36,10 @@ import java.util.List;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.hc.client5.http.auth.AuthChallenge;
 import org.apache.hc.client5.http.auth.AuthScheme;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.AuthScope;
+import org.apache.hc.client5.http.auth.AuthenticationException;
 import org.apache.hc.client5.http.auth.ChallengeType;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
@@ -72,30 +73,9 @@ public class TestBasicScheme {
         Assert.assertNull(authscheme.getRealm());
     }
 
-    @Test
-    public void testBasicAuthenticationWith88591Chars() throws Exception {
-        final int[] germanChars = { 0xE4, 0x2D, 0xF6, 0x2D, 0xFc };
-        final StringBuilder buffer = new StringBuilder();
-        for (final int germanChar : germanChars) {
-            buffer.append((char)germanChar);
-        }
-
-        final HttpHost host  = new HttpHost("somehost", 80);
-        final AuthScope authScope = new AuthScope(host, "some realm", null);
-        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("dh", buffer.toString().toCharArray());
-        final BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
-        credentialsProvider.setCredentials(authScope, creds);
-        final BasicScheme authscheme = new BasicScheme(StandardCharsets.ISO_8859_1);
-
-        Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null));
-        final HttpRequest request = new BasicHttpRequest("GET", "/");
-        final String authResponse = authscheme.generateAuthResponse(host, request, null);
-        Assert.assertEquals(StandardAuthScheme.BASIC + " ZGg65C32Lfw=", authResponse);
-    }
-
     @Test
     public void testBasicAuthentication() throws Exception {
-        final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\"");
+        final AuthChallenge authChallenge = parse("Basic realm=\"test\"");
 
         final BasicScheme authscheme = new BasicScheme();
         authscheme.processChallenge(authChallenge, null);
@@ -110,7 +90,7 @@ public class TestBasicScheme {
         Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null));
         final String authResponse = authscheme.generateAuthResponse(host, request, null);
 
-        final String expected = StandardAuthScheme.BASIC + " " + new String(
+        final String expected = "Basic " + new String(
                 Base64.encodeBase64("testuser:testpass".getBytes(StandardCharsets.US_ASCII)),
                 StandardCharsets.US_ASCII);
         Assert.assertEquals(expected, authResponse);
@@ -119,27 +99,58 @@ public class TestBasicScheme {
         Assert.assertFalse(authscheme.isConnectionBased());
     }
 
+    static final String TEST_UTF8_PASSWORD = "123\u00A3";
+
     @Test
-    public void testBasicProxyAuthentication() throws Exception {
+    public void testBasicAuthenticationDefaultCharsetASCII() throws Exception {
-        final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\"");
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
+        final BasicScheme authscheme = new BasicScheme(StandardCharsets.US_ASCII);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(creds);
+        final String authResponse = authscheme.generateAuthResponse(host, request, null);
+        Assert.assertEquals("Basic dGVzdDoxMjM/", authResponse);
+    }
+
+    @Test
+    public void testBasicAuthenticationDefaultCharsetISO88591() throws Exception {
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
+        final BasicScheme authscheme = new BasicScheme(StandardCharsets.ISO_8859_1);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(creds);
+        final String authResponse = authscheme.generateAuthResponse(host, request, null);
+        Assert.assertEquals("Basic dGVzdDoxMjOj", authResponse);
+    }
+
+    @Test
+    public void testBasicAuthenticationDefaultCharsetUTF8() throws Exception {
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
+        final BasicScheme authscheme = new BasicScheme(StandardCharsets.UTF_8);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(creds);
+        final String authResponse = authscheme.generateAuthResponse(host, request, null);
+        Assert.assertEquals("Basic dGVzdDoxMjPCow==", authResponse);
+    }
+
+    @Test
+    public void testBasicAuthenticationWithCharset() throws Exception {
+        final AuthChallenge authChallenge = parse("Basic realm=\"test\", charset=\"utf-8\"");
 
         final BasicScheme authscheme = new BasicScheme();
         authscheme.processChallenge(authChallenge, null);
 
         final HttpHost host  = new HttpHost("somehost", 80);
         final AuthScope authScope = new AuthScope(host, "test", null);
-        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("testuser", "testpass".toCharArray());
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
         final BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
         credentialsProvider.setCredentials(authScope, creds);
 
         final HttpRequest request = new BasicHttpRequest("GET", "/");
         Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null));
         final String authResponse = authscheme.generateAuthResponse(host, request, null);
-
+        Assert.assertEquals("Basic dGVzdDoxMjPCow==", authResponse);
-        final String expected = StandardAuthScheme.BASIC + " " + new String(
-                Base64.encodeBase64("testuser:testpass".getBytes(StandardCharsets.US_ASCII)),
-                StandardCharsets.US_ASCII);
-        Assert.assertEquals(expected, authResponse);
         Assert.assertEquals("test", authscheme.getRealm());
         Assert.assertTrue(authscheme.isChallengeComplete());
         Assert.assertFalse(authscheme.isConnectionBased());
@@ -147,7 +158,7 @@ public class TestBasicScheme {
 
     @Test
     public void testSerialization() throws Exception {
-        final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\"");
+        final AuthChallenge authChallenge = parse("Basic realm=\"test\"");
 
         final BasicScheme basicScheme = new BasicScheme();
         basicScheme.processChallenge(authChallenge, null);
@@ -182,4 +193,39 @@ public class TestBasicScheme {
         Assert.assertEquals(basicScheme.isChallengeComplete(), authScheme.isChallengeComplete());
     }
 
+    @Test
+    public void testBasicAuthenticationUserCredentialsMissing() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null));
+    }
+
+    @Test
+    public void testBasicAuthenticationUsernameWithBlank() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(new UsernamePasswordCredentials("blah blah", null));
+        authscheme.generateAuthResponse(host, request, null);
+    }
+
+    @Test
+    public void testBasicAuthenticationUsernameWithTab() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(new UsernamePasswordCredentials("blah\tblah", null));
+        Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null));
+    }
+
+    @Test
+    public void testBasicAuthenticationUsernameWithColon() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(new UsernamePasswordCredentials("blah:blah", null));
+        Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null));
+    }
+
 }