RFC 7230: treat presence of a userinfo component in request URI as an HTTP protocol violation

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/AbstractHttpAsyncClientAuthentication.java

@@ -30,6 +30,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Queue;
 import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.Collectors;
@@ -55,7 +56,6 @@ import org.apache.hc.client5.testing.BasicTestAuthenticator;
 import org.apache.hc.client5.testing.auth.Authenticator;
 import org.apache.hc.core5.function.Decorator;
 import org.apache.hc.core5.http.ContentType;
-import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequestInterceptor;
@@ -63,15 +63,14 @@ import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.HttpResponseInterceptor;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.http.HttpVersion;
+import org.apache.hc.core5.http.ProtocolException;
 import org.apache.hc.core5.http.URIScheme;
 import org.apache.hc.core5.http.config.Http1Config;
 import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.config.Registry;
 import org.apache.hc.core5.http.config.RegistryBuilder;
 import org.apache.hc.core5.http.impl.HttpProcessors;
-import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.nio.AsyncServerExchangeHandler;
-import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.http.support.BasicResponseBuilder;
 import org.apache.hc.core5.http2.config.H2Config;
 import org.apache.hc.core5.http2.impl.H2Processors;
@@ -348,24 +347,6 @@ public abstract class AbstractHttpAsyncClientAuthentication<T extends CloseableH
                 CoreMatchers.equalTo(Arrays.asList(401, 200, 401, 200, 401, 200)));
     }
 
-    @Test
-    public void testAuthenticationUserinfoInRequestSuccess() throws Exception {
-        server.register("*", AsyncEchoHandler::new);
-        final HttpHost target = start();
-
-        final HttpClientContext context = HttpClientContext.create();
-        final Future<SimpleHttpResponse> future = httpclient.execute(
-                SimpleRequestBuilder.get()
-                        .setScheme(target.getSchemeName())
-                        .setAuthority(new URIAuthority("test:test", target.getHostName(), target.getPort()))
-                        .setPath("/")
-                        .build(), context, null);
-        final SimpleHttpResponse response = future.get();
-
-        Assert.assertNotNull(response);
-        Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-    }
-
     @Test
     public void testAuthenticationUserinfoInRequestFailure() throws Exception {
         server.register("*", AsyncEchoHandler::new);
@@ -374,41 +355,11 @@ public abstract class AbstractHttpAsyncClientAuthentication<T extends CloseableH
         final HttpClientContext context = HttpClientContext.create();
         final Future<SimpleHttpResponse> future = httpclient.execute(SimpleRequestBuilder.get()
                         .setScheme(target.getSchemeName())
-                        .setAuthority(new URIAuthority("test:all-worng", target.getHostName(), target.getPort()))
+                        .setAuthority(new URIAuthority("test:test", target.getHostName(), target.getPort()))
                         .setPath("/")
                         .build(), context, null);
-        final SimpleHttpResponse response = future.get();
+        final ExecutionException exception = Assert.assertThrows(ExecutionException.class, () -> future.get());
-
+        MatcherAssert.assertThat(exception.getCause(), CoreMatchers.instanceOf(ProtocolException.class));
-        Assert.assertNotNull(response);
-        Assert.assertEquals(HttpStatus.SC_UNAUTHORIZED, response.getCode());
-    }
-
-    @Test
-    public void testAuthenticationUserinfoInRedirectSuccess() throws Exception {
-        server.register("*", AsyncEchoHandler::new);
-        final HttpHost target = start();
-        server.register("/thatway", () -> new AbstractSimpleServerExchangeHandler() {
-
-            @Override
-            protected SimpleHttpResponse handle(
-                    final SimpleHttpRequest request, final HttpCoreContext context) throws HttpException {
-                final SimpleHttpResponse response = new SimpleHttpResponse(HttpStatus.SC_MOVED_PERMANENTLY);
-                response.addHeader(new BasicHeader("Location", target.getSchemeName() + "://test:test@" + target.toHostString() + "/"));
-                return response;
-            }
-        });
-
-        final HttpClientContext context = HttpClientContext.create();
-        final Future<SimpleHttpResponse> future = httpclient.execute(
-                SimpleRequestBuilder.get()
-                        .setScheme(target.getSchemeName())
-                        .setAuthority(new URIAuthority("test:test", target.getHostName(), target.getPort()))
-                        .setPath("/thatway")
-                        .build(), context, null);
-        final SimpleHttpResponse response = future.get();
-
-        Assert.assertNotNull(response);
-        Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
     }
 
     @Test

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientAuthentication.java

@@ -28,7 +28,6 @@ package org.apache.hc.client5.testing.sync;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
@@ -37,6 +36,7 @@ import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.Collectors;
 
+import org.apache.hc.client5.http.ClientProtocolException;
 import org.apache.hc.client5.http.auth.AuthCache;
 import org.apache.hc.client5.http.auth.AuthScheme;
 import org.apache.hc.client5.http.auth.AuthSchemeFactory;
@@ -61,7 +61,6 @@ import org.apache.hc.client5.testing.classic.AuthenticatingDecorator;
 import org.apache.hc.client5.testing.classic.EchoHandler;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
-import org.apache.hc.core5.http.EndpointDetails;
 import org.apache.hc.core5.http.HeaderElements;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpException;
@@ -76,9 +75,7 @@ import org.apache.hc.core5.http.io.HttpRequestHandler;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.apache.hc.core5.http.io.entity.InputStreamEntity;
 import org.apache.hc.core5.http.io.entity.StringEntity;
-import org.apache.hc.core5.http.message.BasicHeader;
 import org.apache.hc.core5.http.protocol.HttpContext;
-import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.http.support.BasicResponseBuilder;
 import org.apache.hc.core5.net.URIAuthority;
 import org.hamcrest.CoreMatchers;
@@ -434,66 +431,13 @@ public class TestClientAuthentication extends LocalServerTestBase {
     }
 
     @Test
-    public void testAuthenticationUserinfoInRequestSuccess() throws Exception {
+    public void testAuthenticationUserinfoInRequest() throws Exception {
         this.server.registerHandler("*", new EchoHandler());
         final HttpHost target = start();
         final HttpGet httpget = new HttpGet("http://test:test@" +  target.toHostString() + "/");
 
         final HttpClientContext context = HttpClientContext.create();
-        try (final ClassicHttpResponse response = this.httpclient.execute(target, httpget, context)) {
+        Assert.assertThrows(ClientProtocolException.class, () -> this.httpclient.execute(target, httpget, context));
-            final HttpEntity entity = response.getEntity();
-            Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertNotNull(entity);
-            EntityUtils.consume(entity);
-        }
-    }
-
-    @Test
-    public void testAuthenticationUserinfoInRequestFailure() throws Exception {
-        this.server.registerHandler("*", new EchoHandler());
-        final HttpHost target = start();
-        final HttpGet httpget = new HttpGet("http://test:all-wrong@" +  target.toHostString() + "/");
-
-        final HttpClientContext context = HttpClientContext.create();
-        try (final ClassicHttpResponse response = this.httpclient.execute(target, httpget, context)) {
-            final HttpEntity entity = response.getEntity();
-            Assert.assertEquals(HttpStatus.SC_UNAUTHORIZED, response.getCode());
-            Assert.assertNotNull(entity);
-            EntityUtils.consume(entity);
-        }
-    }
-
-    @Test
-    public void testAuthenticationUserinfoInRedirectSuccess() throws Exception {
-        this.server.registerHandler("/*", new EchoHandler());
-        this.server.registerHandler("/thatway", (request, response, context) -> {
-            final EndpointDetails endpoint = (EndpointDetails) context.getAttribute(HttpCoreContext.CONNECTION_ENDPOINT);
-            final InetSocketAddress socketAddress = (InetSocketAddress) endpoint.getLocalAddress();
-            final int port = socketAddress.getPort();
-            response.setCode(HttpStatus.SC_MOVED_PERMANENTLY);
-            response.addHeader(new BasicHeader("Location", "http://test:test@localhost:" + port + "/secure"));
-        });
-
-        final HttpHost target = start(new BasicTestAuthenticator("test:test", "test realm") {
-
-            @Override
-            public boolean authenticate(final URIAuthority authority, final String requestUri, final String credentials) {
-                if (requestUri.equals("/secure") || requestUri.startsWith("/secure/")) {
-                    return super.authenticate(authority, requestUri, credentials);
-                }
-                return true;
-            }
-        });
-
-        final HttpGet httpget = new HttpGet("/thatway");
-        final HttpClientContext context = HttpClientContext.create();
-
-        try (final ClassicHttpResponse response = this.httpclient.execute(target, httpget, context)) {
-            final HttpEntity entity = response.getEntity();
-            Assert.assertEquals(HttpStatus.SC_OK, response.getCode());
-            Assert.assertNotNull(entity);
-            EntityUtils.consume(entity);
-        }
     }
 
     @Test

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

@@ -1,85 +0,0 @@
-/*
- * ====================================================================
- * 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;
-
-import org.apache.hc.client5.http.HttpRoute;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
-import org.apache.hc.client5.http.auth.AuthScope;
-import org.apache.hc.client5.http.auth.CredentialsStore;
-import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
-import org.apache.hc.core5.annotation.Internal;
-import org.apache.hc.core5.http.HttpHost;
-import org.apache.hc.core5.http.HttpRequest;
-import org.apache.hc.core5.net.URIAuthority;
-import org.apache.hc.core5.util.Args;
-
-/**
- * Authentication support methods.
- *
- * @since 5.0
- */
-@Internal
-public class AuthSupport {
-
-    public static void extractFromAuthority(
-            final String scheme,
-            final URIAuthority authority,
-            final CredentialsStore credentialsStore) {
-        Args.notNull(credentialsStore, "Credentials store");
-        if (authority == null) {
-            return;
-        }
-        final String userInfo = authority.getUserInfo();
-        if (userInfo == null) {
-            return;
-        }
-        final int atColon = userInfo.indexOf(':');
-        final String userName = atColon >= 0 ? userInfo.substring(0, atColon) : userInfo;
-        final char[] password = atColon >= 0 ? userInfo.substring(atColon + 1).toCharArray() : null;
-
-        credentialsStore.setCredentials(
-                new AuthScope(scheme, authority.getHostName(), authority.getPort(), null, StandardAuthScheme.BASIC),
-                new UsernamePasswordCredentials(userName, password));
-    }
-
-    public static HttpHost resolveAuthTarget(final HttpRequest request, final HttpRoute route) {
-        Args.notNull(request, "Request");
-        Args.notNull(route, "Route");
-        final URIAuthority authority = request.getAuthority();
-        final String scheme = request.getScheme();
-        final HttpHost target = authority != null ? new HttpHost(scheme, authority) : route.getTargetHost();
-        if (target.getPort() < 0) {
-            return new HttpHost(
-                    target.getSchemeName(),
-                    target.getHostName(),
-                    route.getTargetHost().getPort());
-        }
-        return target;
-    }
-
-}

httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java

@@ -39,10 +39,7 @@ import org.apache.hc.client5.http.async.AsyncExecChainHandler;
 import org.apache.hc.client5.http.async.AsyncExecRuntime;
 import org.apache.hc.client5.http.auth.AuthExchange;
 import org.apache.hc.client5.http.auth.ChallengeType;
-import org.apache.hc.client5.http.auth.CredentialsProvider;
-import org.apache.hc.client5.http.auth.CredentialsStore;
 import org.apache.hc.client5.http.config.RequestConfig;
-import org.apache.hc.client5.http.impl.AuthSupport;
 import org.apache.hc.client5.http.impl.RequestSupport;
 import org.apache.hc.client5.http.impl.auth.AuthCacheKeeper;
 import org.apache.hc.client5.http.impl.auth.HttpAuthenticator;
@@ -143,9 +140,8 @@ public final class AsyncProtocolExec implements AsyncExecChainHandler {
         }
 
         final URIAuthority authority = request.getAuthority();
-        final CredentialsProvider credsProvider = clientContext.getCredentialsProvider();
+        if (authority.getUserInfo() != null) {
-        if (credsProvider instanceof CredentialsStore) {
+            throw new ProtocolException("Request URI authority contains deprecated userinfo component");
-            AuthSupport.extractFromAuthority(request.getScheme(), authority, (CredentialsStore) credsProvider);
         }
 
         final HttpHost target = new HttpHost(request.getScheme(), request.getAuthority());

httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java

@@ -35,13 +35,10 @@ import org.apache.hc.client5.http.HttpRoute;
 import org.apache.hc.client5.http.SchemePortResolver;
 import org.apache.hc.client5.http.auth.AuthExchange;
 import org.apache.hc.client5.http.auth.ChallengeType;
-import org.apache.hc.client5.http.auth.CredentialsProvider;
-import org.apache.hc.client5.http.auth.CredentialsStore;
 import org.apache.hc.client5.http.classic.ExecChain;
 import org.apache.hc.client5.http.classic.ExecChainHandler;
 import org.apache.hc.client5.http.classic.ExecRuntime;
 import org.apache.hc.client5.http.config.RequestConfig;
-import org.apache.hc.client5.http.impl.AuthSupport;
 import org.apache.hc.client5.http.impl.RequestSupport;
 import org.apache.hc.client5.http.impl.auth.AuthCacheKeeper;
 import org.apache.hc.client5.http.impl.auth.HttpAuthenticator;
@@ -146,9 +143,8 @@ public final class ProtocolExec implements ExecChainHandler {
             }
 
             final URIAuthority authority = request.getAuthority();
-            final CredentialsProvider credsProvider = context.getCredentialsProvider();
+            if (authority.getUserInfo() != null) {
-            if (credsProvider instanceof CredentialsStore) {
+                throw new ProtocolException("Request URI authority contains deprecated userinfo component");
-                AuthSupport.extractFromAuthority(request.getScheme(), authority, (CredentialsStore) credsProvider);
             }
 
             final HttpHost target = new HttpHost(request.getScheme(), request.getAuthority());

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

@@ -1,52 +0,0 @@
-/*
- * ====================================================================
- * 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;
-
-import org.apache.hc.client5.http.auth.AuthScope;
-import org.apache.hc.client5.http.auth.Credentials;
-import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
-import org.apache.hc.core5.net.URIAuthority;
-import org.junit.Assert;
-import org.junit.Test;
-
-/**
- * Simple tests for {@link AuthSupport}.
- */
-public class TestAuthSupport {
-
-    @Test
-    public void testExtractFromAuthority() {
-        final URIAuthority uriAuthority = new URIAuthority("testUser", "localhost", 8080);
-        final BasicCredentialsProvider basicCredentialsProvider = new BasicCredentialsProvider();
-
-        AuthSupport.extractFromAuthority("http", uriAuthority, basicCredentialsProvider);
-
-        final Credentials credentials = basicCredentialsProvider.getCredentials(new AuthScope("localhost", 8080), null);
-        Assert.assertEquals("testUser", credentials.getUserPrincipal().getName());
-        Assert.assertNull(credentials.getPassword());
-    }
-}

httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java

@@ -30,7 +30,6 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.URI;
 import java.util.Collections;
 
 import org.apache.hc.client5.http.AuthenticationStrategy;
@@ -38,15 +37,12 @@ import org.apache.hc.client5.http.HttpRoute;
 import org.apache.hc.client5.http.auth.AuthExchange;
 import org.apache.hc.client5.http.auth.AuthScope;
 import org.apache.hc.client5.http.auth.ChallengeType;
-import org.apache.hc.client5.http.auth.Credentials;
-import org.apache.hc.client5.http.auth.CredentialsProvider;
 import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.classic.ExecChain;
 import org.apache.hc.client5.http.classic.ExecRuntime;
 import org.apache.hc.client5.http.classic.methods.HttpGet;
 import org.apache.hc.client5.http.classic.methods.HttpPost;
 import org.apache.hc.client5.http.entity.EntityBuilder;
-import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
 import org.apache.hc.client5.http.impl.auth.BasicScheme;
 import org.apache.hc.client5.http.impl.auth.CredentialsProviderBuilder;
 import org.apache.hc.client5.http.impl.auth.NTLMScheme;
@@ -57,6 +53,7 @@ import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpResponse;
+import org.apache.hc.core5.http.ProtocolException;
 import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
 import org.junit.Assert;
@@ -123,20 +120,9 @@ public class TestProtocolExec {
         final HttpRoute route = new HttpRoute(new HttpHost("somehost", 8080));
         final ClassicHttpRequest request = new HttpGet("http://somefella:secret@bar/test");
         final HttpClientContext context = HttpClientContext.create();
-        context.setCredentialsProvider(new BasicCredentialsProvider());
-
-        final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
-        Mockito.when(chain.proceed(
-                Mockito.any(),
-                Mockito.any())).thenReturn(response);
 
         final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context);
-        protocolExec.execute(request, scope, chain);
+        Assert.assertThrows(ProtocolException.class, () -> protocolExec.execute(request, scope, chain));
-        Assert.assertEquals(new URI("http://bar/test"), request.getUri());
-        final CredentialsProvider credentialsProvider = context.getCredentialsProvider();
-        final Credentials creds = credentialsProvider.getCredentials(new AuthScope(null, "bar", -1, null, null), null);
-        Assert.assertNotNull(creds);
-        Assert.assertEquals("somefella", creds.getUserPrincipal().getName());
     }
 
     @Test