mirror of https://github.com/apache/jclouds.git
Introduce StripExpectHeader filter and a property to control it.
Some providers (specifically HP Cloud and Google Cloud Storage) do not properly support Expect: 100-continue headers. JDK7 is stricter in its handling of the Expect header than JDK6 -- in particular, it expects servers to properly respond to an expect header and times out only if a prior timeout did not exist on the underlying HTTP connection. As a result, JDK7 tests against these providers hang and fail. This commit introduces a new filter -- appropriate called StripExpectHeader -- that is controlled by the property jclouds.strip-expect-header. The property defaults to false to preserve existing behavior but allows applications to tweak Expect header handling. Tested by running HPCS live tests with JDK7 -- previously most of these tests would fail with timeouts. Closes JCLOUDS-181
This commit is contained in:
parent
1178f47cd7
commit
d52f460562
|
@ -296,4 +296,9 @@ public interface Constants {
|
|||
*/
|
||||
public static final String PROPERTY_PRETTY_PRINT_PAYLOADS = "jclouds.payloads.pretty-print";
|
||||
|
||||
/**
|
||||
* When true, strip the Expect: 100-continue header. Useful when interacting with
|
||||
* providers that don't properly support Expect headers. Defaults to false.
|
||||
*/
|
||||
public static final String PROPERTY_STRIP_EXPECT_HEADER = "jclouds.strip-expect-header";
|
||||
}
|
||||
|
|
|
@ -29,6 +29,7 @@ import static org.jclouds.Constants.PROPERTY_PRETTY_PRINT_PAYLOADS;
|
|||
import static org.jclouds.Constants.PROPERTY_SCHEDULER_THREADS;
|
||||
import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL;
|
||||
import static org.jclouds.Constants.PROPERTY_SO_TIMEOUT;
|
||||
import static org.jclouds.Constants.PROPERTY_STRIP_EXPECT_HEADER;
|
||||
import static org.jclouds.Constants.PROPERTY_USER_THREADS;
|
||||
import static org.jclouds.reflect.Reflection2.typeToken;
|
||||
|
||||
|
@ -46,7 +47,6 @@ import com.google.common.base.Optional;
|
|||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.common.reflect.TypeToken;
|
||||
import com.google.inject.Module;
|
||||
|
||||
/**
|
||||
* The BaseApiMetadata class is an abstraction of {@link ApiMetadata} to be extended by those
|
||||
* implementing ApiMetadata.
|
||||
|
@ -72,6 +72,7 @@ public abstract class BaseApiMetadata implements ApiMetadata {
|
|||
props.setProperty(PROPERTY_MAX_SESSION_FAILURES, 2 + "");
|
||||
props.setProperty(PROPERTY_SESSION_INTERVAL, 60 + "");
|
||||
props.setProperty(PROPERTY_PRETTY_PRINT_PAYLOADS, "true");
|
||||
props.setProperty(PROPERTY_STRIP_EXPECT_HEADER, "false");
|
||||
return props;
|
||||
}
|
||||
|
||||
|
|
|
@ -0,0 +1,36 @@
|
|||
/*
|
||||
* 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.
|
||||
*/
|
||||
|
||||
package org.jclouds.http.filters;
|
||||
|
||||
import org.jclouds.http.HttpException;
|
||||
import org.jclouds.http.HttpRequest;
|
||||
import org.jclouds.http.HttpRequestFilter;
|
||||
|
||||
import com.google.common.net.HttpHeaders;
|
||||
import com.google.inject.Singleton;
|
||||
|
||||
/**
|
||||
* @author Diwaker Gupta
|
||||
*/
|
||||
@Singleton
|
||||
public class StripExpectHeader implements HttpRequestFilter {
|
||||
@Override
|
||||
public HttpRequest filter(HttpRequest request) throws HttpException {
|
||||
return request.toBuilder().removeHeader(HttpHeaders.EXPECT).build();
|
||||
}
|
||||
}
|
|
@ -50,7 +50,6 @@ import java.util.Map;
|
|||
import java.util.Map.Entry;
|
||||
import java.util.NoSuchElementException;
|
||||
import java.util.Set;
|
||||
|
||||
import javax.annotation.Resource;
|
||||
import javax.inject.Named;
|
||||
import javax.ws.rs.FormParam;
|
||||
|
@ -65,6 +64,7 @@ import org.jclouds.http.HttpRequest;
|
|||
import org.jclouds.http.HttpRequestFilter;
|
||||
import org.jclouds.http.HttpUtils;
|
||||
import org.jclouds.http.Uris.UriBuilder;
|
||||
import org.jclouds.http.filters.StripExpectHeader;
|
||||
import org.jclouds.http.options.HttpRequestOptions;
|
||||
import org.jclouds.io.ContentMetadataCodec;
|
||||
import org.jclouds.io.Payload;
|
||||
|
@ -149,11 +149,13 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
|
|||
private final InputParamValidator inputParamValidator;
|
||||
private final GetAcceptHeaders getAcceptHeaders;
|
||||
private final Invocation caller;
|
||||
private final boolean stripExpectHeader;
|
||||
|
||||
@Inject
|
||||
private RestAnnotationProcessor(Injector injector, @ApiVersion String apiVersion, @BuildVersion String buildVersion,
|
||||
HttpUtils utils, ContentMetadataCodec contentMetadataCodec, InputParamValidator inputParamValidator,
|
||||
GetAcceptHeaders getAcceptHeaders, @Nullable @Named("caller") Invocation caller) {
|
||||
GetAcceptHeaders getAcceptHeaders, @Nullable @Named("caller") Invocation caller,
|
||||
@Named(Constants.PROPERTY_STRIP_EXPECT_HEADER) boolean stripExpectHeader) {
|
||||
this.injector = injector;
|
||||
this.utils = utils;
|
||||
this.contentMetadataCodec = contentMetadataCodec;
|
||||
|
@ -162,6 +164,7 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
|
|||
this.inputParamValidator = inputParamValidator;
|
||||
this.getAcceptHeaders = getAcceptHeaders;
|
||||
this.caller = caller;
|
||||
this.stripExpectHeader = stripExpectHeader;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -208,6 +211,9 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest
|
|||
}
|
||||
|
||||
requestBuilder.filters(getFiltersIfAnnotated(invocation));
|
||||
if (stripExpectHeader) {
|
||||
requestBuilder.filter(new StripExpectHeader());
|
||||
}
|
||||
|
||||
Multimap<String, Object> tokenValues = LinkedHashMultimap.create();
|
||||
|
||||
|
|
|
@ -0,0 +1,38 @@
|
|||
/*
|
||||
* 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.
|
||||
*/
|
||||
|
||||
package org.jclouds.http.filters;
|
||||
|
||||
import static org.testng.Assert.assertFalse;
|
||||
|
||||
import org.jclouds.http.HttpRequest;
|
||||
import org.testng.annotations.Test;
|
||||
|
||||
import com.google.common.net.HttpHeaders;
|
||||
|
||||
/**
|
||||
* @author Diwaker Gupta
|
||||
*/
|
||||
@Test(groups = "unit")
|
||||
public class StripExpectHeaderTest {
|
||||
public void testExpectHeaderIsStripped() {
|
||||
HttpRequest request = HttpRequest.builder().method("POST").addHeader(HttpHeaders.EXPECT, "100-Continue")
|
||||
.endpoint("http://localhost").build();
|
||||
request = new StripExpectHeader().filter(request);
|
||||
assertFalse(request.getHeaders().containsKey(HttpHeaders.EXPECT));
|
||||
}
|
||||
}
|
|
@ -24,6 +24,7 @@ import static org.jclouds.io.Payloads.newStringPayload;
|
|||
import static org.jclouds.reflect.Reflection2.method;
|
||||
import static org.testng.Assert.assertEquals;
|
||||
import static org.testng.Assert.assertNull;
|
||||
import static org.testng.Assert.assertTrue;
|
||||
import static org.testng.Assert.fail;
|
||||
|
||||
import java.io.Closeable;
|
||||
|
@ -42,9 +43,9 @@ import java.util.Collection;
|
|||
import java.util.Collections;
|
||||
import java.util.Date;
|
||||
import java.util.Map;
|
||||
import java.util.Properties;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.ExecutionException;
|
||||
|
||||
import javax.inject.Named;
|
||||
import javax.inject.Qualifier;
|
||||
import javax.inject.Singleton;
|
||||
|
@ -62,6 +63,7 @@ import javax.ws.rs.QueryParam;
|
|||
import javax.ws.rs.core.MediaType;
|
||||
|
||||
import org.eclipse.jetty.http.HttpHeaders;
|
||||
import org.jclouds.Constants;
|
||||
import org.jclouds.ContextBuilder;
|
||||
import org.jclouds.date.DateService;
|
||||
import org.jclouds.date.internal.SimpleDateFormatDateService;
|
||||
|
@ -72,6 +74,7 @@ import org.jclouds.http.HttpRequest;
|
|||
import org.jclouds.http.HttpRequestFilter;
|
||||
import org.jclouds.http.HttpResponse;
|
||||
import org.jclouds.http.IOExceptionRetryHandler;
|
||||
import org.jclouds.http.filters.StripExpectHeader;
|
||||
import org.jclouds.http.functions.ParseFirstJsonValueNamed;
|
||||
import org.jclouds.http.functions.ParseJson;
|
||||
import org.jclouds.http.functions.ParseSax;
|
||||
|
@ -851,8 +854,8 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
|
|||
Lists.<Object> newArrayList((String) null)));
|
||||
Assert.fail("call should have failed with illegal null parameter, not permitted " + request + " to be created");
|
||||
} catch (NullPointerException e) {
|
||||
Assert.assertTrue(e.toString().indexOf("postNonnull parameter 1") >= 0,
|
||||
"Error message should have referred to 'parameter 1': " + e);
|
||||
assertTrue(e.toString().indexOf("postNonnull parameter 1") >= 0,
|
||||
"Error message should have referred to 'parameter 1': " + e);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1367,6 +1370,9 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
|
|||
@OverrideRequestFilters
|
||||
@RequestFilters(TestRequestFilter2.class)
|
||||
public void getOverride(HttpRequest request);
|
||||
|
||||
@POST
|
||||
public void post();
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -1396,6 +1402,37 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
|
|||
assertEquals(request.getFilters().get(0).getClass(), TestRequestFilter2.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRequestFilterStripExpect() {
|
||||
// First, verify that by default, the StripExpectHeader filter is not applied
|
||||
Invokable<?, ?> method = method(TestRequestFilter.class, "post");
|
||||
Invocation invocation = Invocation.create(method,
|
||||
ImmutableList.<Object>of(HttpRequest.builder().method("POST").endpoint("http://localhost")
|
||||
.addHeader(HttpHeaders.EXPECT, "100-Continue").build()));
|
||||
GeneratedHttpRequest request = processor.apply(invocation);
|
||||
assertEquals(request.getFilters().size(), 1);
|
||||
assertEquals(request.getFilters().get(0).getClass(), TestRequestFilter1.class);
|
||||
|
||||
// Now let's create a new injector with the property set. Use that to create the annotation processor.
|
||||
Properties overrides = new Properties();
|
||||
overrides.setProperty(Constants.PROPERTY_STRIP_EXPECT_HEADER, "true");
|
||||
Injector injector = ContextBuilder.newBuilder(
|
||||
AnonymousProviderMetadata.forClientMappedToAsyncClientOnEndpoint(Callee.class, AsyncCallee.class,
|
||||
"http://localhost:9999"))
|
||||
.modules(ImmutableSet.<Module> of(new MockModule(), new NullLoggingModule(), new AbstractModule() {
|
||||
protected void configure() {
|
||||
bind(new TypeLiteral<Supplier<URI>>() {
|
||||
}).annotatedWith(Localhost2.class).toInstance(
|
||||
Suppliers.ofInstance(URI.create("http://localhost:1111")));
|
||||
}}))
|
||||
.overrides(overrides).buildInjector();
|
||||
RestAnnotationProcessor newProcessor = injector.getInstance(RestAnnotationProcessor.class);
|
||||
// Verify that this time the filter is indeed applied as expected.
|
||||
request = newProcessor.apply(invocation);
|
||||
assertEquals(request.getFilters().size(), 2);
|
||||
assertEquals(request.getFilters().get(1).getClass(), StripExpectHeader.class);
|
||||
}
|
||||
|
||||
public class TestEncoding {
|
||||
@GET
|
||||
@Path("/{path1}/{path2}")
|
||||
|
|
Loading…
Reference in New Issue