hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Fran├žois-Xavier Bonnet <francois-xavier.bon...@centraliens.net>
Subject Re: svn commit: r1448936
Date Fri, 22 Feb 2013 10:19:15 GMT
Oups... Sorry, I will do a diff before committing next time.

On 22/02/2013 11:13, Oleg Kalnichevski wrote:
> On Fri, 2013-02-22 at 08:36 +0000, fx@apache.org wrote:
>> Author: fx
>> Date: Fri Feb 22 08:36:36 2013
>> New Revision: 1448936
>>
>> URL: http://svn.apache.org/r1448936
>> Log:
>> HTTPCLIENT-1325: Using a HttpRequest that is not an HttpUriRequest when host in URI
is not target host results in an invalid request
>>
> Francois-Xavier
>
> I do not mean to be nasty, but please avoid mixing functional and
> formatting changes in the same commit. It makes it more difficult to
> review the changes. It happens to me all the time but generally this is
> something we should _try_ to avoid. You should probably disable code
> formatting on save option or some such in your Eclipse.
>
> I also tend to do 'git diff --staged' or 'svn diff' prior to committing
> local changes to make sure there is no unnecessary noise (whitespace
> changes, formatting, etc).
>
> Oleg
>
>
>> Added:
>>      httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestProtocolExec.java
  (with props)
>> Modified:
>>      httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/ProtocolExec.java
>>
>> Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/ProtocolExec.java
>> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/ProtocolExec.java?rev=1448936&r1=1448935&r2=1448936&view=diff
>> ==============================================================================
>> --- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/ProtocolExec.java
(original)
>> +++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/execchain/ProtocolExec.java
Fri Feb 22 08:36:36 2013
>> @@ -67,18 +67,15 @@ public class ProtocolExec implements Cli
>>       private final ClientExecChain requestExecutor;
>>       private final HttpProcessor httpProcessor;
>>   
>> -    public ProtocolExec(
>> -            final ClientExecChain requestExecutor,
>> -            final HttpProcessor httpProcessor) {
>> +    public ProtocolExec(final ClientExecChain requestExecutor, final HttpProcessor
httpProcessor) {
>>           Args.notNull(requestExecutor, "HTTP client request executor");
>>           Args.notNull(httpProcessor, "HTTP protocol processor");
>>           this.requestExecutor = requestExecutor;
>>           this.httpProcessor = httpProcessor;
>>       }
>>   
>> -    private void rewriteRequestURI(
>> -            final HttpRequestWrapper request,
>> -            final HttpRoute route) throws ProtocolException {
>> +    private void rewriteRequestURI(final HttpRequestWrapper request, final HttpRoute
route)
>> +        throws ProtocolException {
>>           try {
>>               URI uri = request.getURI();
>>               if (uri != null) {
>> @@ -101,16 +98,13 @@ public class ProtocolExec implements Cli
>>                   request.setURI(uri);
>>               }
>>           } catch (final URISyntaxException ex) {
>> -            throw new ProtocolException("Invalid URI: " +
>> -                    request.getRequestLine().getUri(), ex);
>> +            throw new ProtocolException("Invalid URI: " + request.getRequestLine().getUri(),
ex);
>>           }
>>       }
>>   
>> -    public CloseableHttpResponse execute(
>> -            final HttpRoute route,
>> -            final HttpRequestWrapper request,
>> -            final HttpClientContext context,
>> -            final HttpExecutionAware execAware) throws IOException, HttpException
{
>> +    public CloseableHttpResponse execute(final HttpRoute route, final HttpRequestWrapper
request,
>> +        final HttpClientContext context, final HttpExecutionAware execAware) throws
IOException,
>> +        HttpException {
>>           Args.notNull(route, "HTTP route");
>>           Args.notNull(request, "HTTP request");
>>           Args.notNull(context, "HTTP context");
>> @@ -122,8 +116,8 @@ public class ProtocolExec implements Cli
>>               if (uri != null) {
>>                   final String userinfo = uri.getUserInfo();
>>                   if (userinfo != null) {
>> -                    targetAuthState.update(
>> -                            new BasicScheme(), new UsernamePasswordCredentials(userinfo));
>> +                    targetAuthState.update(new BasicScheme(), new UsernamePasswordCredentials(
>> +                        userinfo));
>>                   }
>>               }
>>           }
>> @@ -136,8 +130,9 @@ public class ProtocolExec implements Cli
>>           // HTTPCLIENT-1092 - add the port if necessary
>>           if (virtualHost != null && virtualHost.getPort() == -1) {
>>               final int port = route.getTargetHost().getPort();
>> -            if (port != -1){
>> -                virtualHost = new HttpHost(virtualHost.getHostName(), port, virtualHost.getSchemeName());
>> +            if (port != -1) {
>> +                virtualHost = new HttpHost(virtualHost.getHostName(), port,
>> +                    virtualHost.getSchemeName());
>>               }
>>               if (this.log.isDebugEnabled()) {
>>                   this.log.debug("Using virtual host" + virtualHost);
>> @@ -149,11 +144,20 @@ public class ProtocolExec implements Cli
>>               target = virtualHost;
>>           } else {
>>               final HttpRequest original = request.getOriginal();
>> +            URI uri = null;
>>               if (original instanceof HttpUriRequest) {
>> -                final URI uri = ((HttpUriRequest) original).getURI();
>> -                if (uri.isAbsolute()) {
>> -                    target = new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme());
>> +                uri = ((HttpUriRequest) original).getURI();
>> +            } else {
>> +                final String uriString = original.getRequestLine().getUri();
>> +                try {
>> +                    uri = URI.create(uriString);
>> +                } catch (IllegalArgumentException e) {
>> +                    log.debug("Could not parse " + uriString + " as a URI to set
Host header");
>>                   }
>> +
>> +            }
>> +            if (uri != null && uri.isAbsolute() && uri.getHost()
!= null) {
>> +                target = new HttpHost(uri.getHost(), uri.getPort(), uri.getScheme());
>>               }
>>           }
>>           if (target == null) {
>> @@ -167,7 +171,8 @@ public class ProtocolExec implements Cli
>>   
>>           this.httpProcessor.process(request, context);
>>   
>> -        final CloseableHttpResponse response = this.requestExecutor.execute(route,
request, context, execAware);
>> +        final CloseableHttpResponse response = this.requestExecutor.execute(route,
request,
>> +            context, execAware);
>>           try {
>>               // Run response protocol interceptors
>>               context.setAttribute(ExecutionContext.HTTP_RESPONSE, response);
>>
>> Added: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestProtocolExec.java
>> URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestProtocolExec.java?rev=1448936&view=auto
>> ==============================================================================
>> --- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestProtocolExec.java
(added)
>> +++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestProtocolExec.java
Fri Feb 22 08:36:36 2013
>> @@ -0,0 +1,84 @@
>> +/*
>> + * ====================================================================
>> + * 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.http.impl.execchain;
>> +
>> +import org.apache.http.HttpHost;
>> +import org.apache.http.client.methods.HttpExecutionAware;
>> +import org.apache.http.client.methods.HttpRequestWrapper;
>> +import org.apache.http.client.protocol.HttpClientContext;
>> +import org.apache.http.conn.routing.HttpRoute;
>> +import org.apache.http.message.BasicHttpRequest;
>> +import org.apache.http.params.BasicHttpParams;
>> +import org.apache.http.params.HttpParams;
>> +import org.apache.http.protocol.ExecutionContext;
>> +import org.apache.http.protocol.HttpProcessor;
>> +import org.junit.Assert;
>> +import org.junit.Before;
>> +import org.junit.Test;
>> +import org.mockito.Mockito;
>> +
>> +public class TestProtocolExec {
>> +
>> +    private ClientExecChain requestExecutor;
>> +    private HttpProcessor httpProcessor;
>> +    private ProtocolExec protocolExec;
>> +    private HttpClientContext context;
>> +    private HttpRequestWrapper request;
>> +    private HttpExecutionAware execAware;
>> +    private HttpRoute route;
>> +    private HttpParams params = new BasicHttpParams();
>> +
>> +    @Before
>> +    public void setup() throws Exception {
>> +        requestExecutor = Mockito.mock(ClientExecChain.class);
>> +        httpProcessor = Mockito.mock(HttpProcessor.class);
>> +        protocolExec = new ProtocolExec(requestExecutor, httpProcessor);
>> +        route = new HttpRoute(new HttpHost("foo", 8080));
>> +        context = new HttpClientContext();
>> +        execAware = Mockito.mock(HttpExecutionAware.class);
>> +    }
>> +
>> +    @Test
>> +    public void testHostHeaderWhenNonUriRequest() throws Exception {
>> +        request = HttpRequestWrapper.wrap(new BasicHttpRequest("GET", "http://bar/test"));
>> +        protocolExec.execute(route, request, context, execAware);
>> +        // ProtocolExect should have extracted the host from request URI
>> +        Assert.assertEquals(new HttpHost("bar", -1, "http"),
>> +            context.getAttribute(ExecutionContext.HTTP_TARGET_HOST));
>> +    }
>> +
>> +    @Test
>> +    public void testHostHeaderWhenNonUriRequestAndInvalidUri() throws Exception
{
>> +        request = HttpRequestWrapper.wrap(new BasicHttpRequest("GET", "http://bar/test|"));
>> +        protocolExec.execute(route, request, context, execAware);
>> +        // ProtocolExect should have fall back to physical host as request URI
>> +        // is not parseable
>> +        Assert.assertEquals(new HttpHost("foo", 8080, "http"),
>> +            context.getAttribute(ExecutionContext.HTTP_TARGET_HOST));
>> +    }
>> +
>> +}
>>
>> Propchange: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/execchain/TestProtocolExec.java
>> ------------------------------------------------------------------------------
>>      svn:executable = *
>>
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message