You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by zh...@apache.org on 2010/11/11 19:55:31 UTC
svn commit: r1034046 - in /shindig/trunk/java/gadgets/src:
main/java/org/apache/shindig/gadgets/http/
test/java/org/apache/shindig/gadgets/http/
Author: zhoresh
Date: Thu Nov 11 18:55:30 2010
New Revision: 1034046
URL: http://svn.apache.org/viewvc?rev=1034046&view=rev
Log:
ref http://codereview.appspot.com/2993042/
Move the drift time correction from http response to pipeline
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java?rev=1034046&r1=1034045&r2=1034046&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java Thu Nov 11 18:55:30 2010
@@ -26,12 +26,17 @@ import org.apache.shindig.common.Nullabl
import com.google.inject.name.Named;
+import org.apache.shindig.common.servlet.HttpUtil;
+import org.apache.shindig.common.util.DateUtil;
import org.apache.shindig.common.util.Utf8UrlCoder;
import org.apache.shindig.gadgets.GadgetException;
import org.apache.shindig.gadgets.oauth.OAuthRequest;
import org.apache.shindig.gadgets.rewrite.ResponseRewriterRegistry;
import org.apache.shindig.gadgets.rewrite.RewritingException;
+import java.util.Collection;
+import java.util.Date;
+
/**
* A standard implementation of a request pipeline. Performs request caching and
* signing on top of standard HTTP requests.
@@ -45,6 +50,13 @@ public class DefaultRequestPipeline impl
private final InvalidationService invalidationService;
private final HttpResponseMetadataHelper metadataHelper;
+ // At what point you don't trust remote server date stamp on response (in milliseconds)
+ // (Should be less then DEFAULT_TTL)
+ static final long DEFAULT_DRIFT_LIMIT_MS = 3L * 60L * 1000L;
+
+ @Inject(optional = true) @Named("shindig.http.date-drift-limit-ms")
+ private static long responseDateDriftLimit = DEFAULT_DRIFT_LIMIT_MS;
+
@Inject
public DefaultRequestPipeline(HttpFetcher httpFetcher,
HttpCache httpCache,
@@ -107,10 +119,12 @@ public class DefaultRequestPipeline impl
if (fetchedResponse.getHttpStatusCode() >= 500 && staleResponse != null) {
// If we have trouble accessing the remote server,
- // Lets try the latest good but staled result
+ // Lets try the latest good but staled result
return staleResponse;
}
-
+
+ fetchedResponse = maybeFixDriftTime(fetchedResponse);
+
if (!fetchedResponse.isError() && !request.getIgnoreCache() && request.getCacheTtl() != 0) {
try {
fetchedResponse = responseRewriterRegistry.rewriteHttpResponse(request, fetchedResponse);
@@ -118,7 +132,7 @@ public class DefaultRequestPipeline impl
throw new GadgetException(GadgetException.Code.INTERNAL_SERVER_ERROR, e, e.getHttpStatusCode());
}
}
-
+
// Set response hash value in metadata (used for url versioning)
fetchedResponse = HttpResponseMetadataHelper.updateHash(fetchedResponse, metadataHelper);
if (!request.getIgnoreCache()) {
@@ -145,4 +159,32 @@ public class DefaultRequestPipeline impl
HttpResponse.SC_BAD_REQUEST);
}
}
+
+ /**
+ * Verify response time, and if response time is off from current time by more then
+ * speficied time change response time to be current time.
+ * The function resolve cases that remote server time is wrong, which can cause
+ * resources to expire prematurly or served after they should be expired.
+ * The allowd drift time is configured by responseDateDriftLimit.
+ * @param response the response to fix
+ * @return new response with fix date or original reesponse
+ */
+ public static HttpResponse maybeFixDriftTime(HttpResponse response) {
+ Collection<String> dates = response.getHeaders("Date");
+
+ if (!dates.isEmpty()) {
+ Date d = DateUtil.parseRfc1123Date(dates.iterator().next());
+ if (d != null) {
+ long timestamp = d.getTime();
+ long currentTime = HttpUtil.getTimeSource().currentTimeMillis();
+ if (Math.abs(currentTime - timestamp) > responseDateDriftLimit) {
+ // Do not trust the date from response if it is too old (server time out of sync)
+ HttpResponseBuilder builder = new HttpResponseBuilder(response);
+ builder.setHeader("Date", DateUtil.formatRfc1123Date(currentTime));
+ response = builder.create();
+ }
+ }
+ }
+ return response;
+ }
}
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java?rev=1034046&r1=1034045&r2=1034046&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java Thu Nov 11 18:55:30 2010
@@ -129,10 +129,6 @@ public final class HttpResponse implemen
static final Charset DEFAULT_ENCODING = Charset.forName("UTF-8");
- // At what point you don't trust remote server date stamp on response (in milliseconds)
- // (Should be less then DEFAULT_TTL)
- static final long DEFAULT_DRIFT_LIMIT_MS = 3L * 60L * 1000L;
-
@Inject(optional = true) @Named("shindig.cache.http.negativeCacheTtl")
private static long negativeCacheTtl = DEFAULT_NEGATIVE_CACHE_TTL;
@@ -147,9 +143,6 @@ public final class HttpResponse implemen
private static EncodingDetector.FallbackEncodingDetector customEncodingDetector =
new EncodingDetector.FallbackEncodingDetector();
- @Inject(optional = true) @Named("shindig.http.date-drift-limit-ms")
- private static long responseDateDriftLimit = DEFAULT_DRIFT_LIMIT_MS;
-
public static void setTimeSource(TimeSource timeSource) {
HttpUtil.setTimeSource(timeSource);
}
@@ -468,10 +461,6 @@ public final class HttpResponse implemen
Date d = DateUtil.parseRfc1123Date(dates.iterator().next());
if (d != null) {
timestamp = d.getTime();
- if (Math.abs(currentTime - timestamp) > responseDateDriftLimit) {
- // Do not trust the date from response if it is too old (server time out of sync)
- timestamp = -1;
- }
}
}
if (timestamp == -1) {
Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java?rev=1034046&r1=1034045&r2=1034046&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java Thu Nov 11 18:55:30 2010
@@ -18,15 +18,18 @@
package org.apache.shindig.gadgets.http;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
import com.google.common.collect.Maps;
import com.google.inject.Provider;
import org.apache.shindig.common.uri.Uri;
+import org.apache.shindig.common.util.DateUtil;
import org.apache.shindig.gadgets.AuthType;
import org.apache.shindig.gadgets.GadgetException;
import org.apache.shindig.gadgets.oauth.OAuthRequest;
import org.apache.shindig.gadgets.rewrite.DefaultResponseRewriterRegistry;
+import org.junit.Before;
import org.junit.Test;
import java.util.Map;
@@ -47,6 +50,11 @@ public class DefaultRequestPipelineTest
private final RequestPipeline pipeline = new DefaultRequestPipeline(fetcher, cache, oauth,
new DefaultResponseRewriterRegistry(null, null), new NoOpInvalidationService(), helper);
+ @Before
+ public void setUp() {
+ HttpResponseTest.setHttpTimeSource();
+ }
+
@Test
public void authTypeNoneNotCached() throws Exception {
HttpRequest request = new HttpRequest(DEFAULT_URI)
@@ -72,7 +80,14 @@ public class DefaultRequestPipelineTest
HttpRequest request = new HttpRequest(DEFAULT_URI)
.setAuthType(AuthType.NONE);
- fetcher.response = new HttpResponse("response");
+ int time = roundToSeconds(HttpResponseTest.timeSource.currentTimeMillis()) - 10;
+ String date = DateUtil.formatRfc1123Date(1000L * time);
+ HttpResponseBuilder builder = new HttpResponseBuilder()
+ .setCacheTtl(100)
+ .addHeader("Date", date);
+ builder.setContent("response");
+
+ fetcher.response = builder.create();
RequestPipeline pipeline = new DefaultRequestPipeline(fetcher, cache, oauth,
new DefaultResponseRewriterRegistry(null, null), new NoOpInvalidationService(),
@@ -81,9 +96,36 @@ public class DefaultRequestPipelineTest
assertEquals(1, response.getMetadata().size());
assertEquals("q7u8tbpmidtu1gtqhjv0kb0rvo",
response.getMetadata().get(HttpResponseMetadataHelper.DATA_HASH));
+ assertEquals(date, response.getHeader("Date"));
+ assertEquals(roundToSeconds(90000 - 1), roundToSeconds(response.getCacheTtl() - 1));
}
@Test
+ public void verifyFixedDate() throws Exception {
+ HttpRequest request = new HttpRequest(DEFAULT_URI)
+ .setAuthType(AuthType.NONE);
+
+ int time = roundToSeconds(HttpResponseTest.timeSource.currentTimeMillis());
+ String date = DateUtil.formatRfc1123Date(1000L * time
+ - 1000 - DefaultRequestPipeline.DEFAULT_DRIFT_LIMIT_MS);
+ HttpResponseBuilder builder = new HttpResponseBuilder()
+ .setCacheTtl(100)
+ .addHeader("Date", date);
+ builder.setContent("response");
+
+ fetcher.response = builder.create();
+
+ RequestPipeline pipeline = new DefaultRequestPipeline(fetcher, cache, oauth,
+ new DefaultResponseRewriterRegistry(null, null), new NoOpInvalidationService(),
+ new HttpResponseMetadataHelper());
+ HttpResponse response = pipeline.execute(request);
+ // Verify time is current time instead of expired
+ assertEquals(DateUtil.formatRfc1123Date(1000L * time), response.getHeader("Date"));
+ assertEquals(roundToSeconds(100000 - 1), roundToSeconds(response.getCacheTtl() - 1));
+ }
+
+
+ @Test
public void authTypeNoneWasCached() throws Exception {
HttpRequest request = new HttpRequest(DEFAULT_URI)
.setAuthType(AuthType.NONE);
@@ -220,6 +262,55 @@ public class DefaultRequestPipelineTest
assertEquals(0, cache.writeCount);
}
+ private static int roundToSeconds(long ts) {
+ return (int)(ts / 1000);
+ }
+
+ @Test
+ public void testFixedDateOk() throws Exception {
+ int time = roundToSeconds(HttpResponseTest.timeSource.currentTimeMillis());
+ HttpResponse response = new HttpResponseBuilder()
+ .addHeader("Date", DateUtil.formatRfc1123Date(1000L * time
+ + 1000 - DefaultRequestPipeline.DEFAULT_DRIFT_LIMIT_MS))
+ .setCacheTtl(100)
+ .create();
+
+ HttpResponse newResponse = DefaultRequestPipeline.maybeFixDriftTime(response);
+ assertSame(response, newResponse);
+ }
+
+ @Test
+ public void testFixedDateOld() throws Exception {
+ int time = roundToSeconds(HttpResponseTest.timeSource.currentTimeMillis());
+ HttpResponse response = new HttpResponseBuilder()
+ .addHeader("Date", DateUtil.formatRfc1123Date(1000L * time
+ - 1000 - DefaultRequestPipeline.DEFAULT_DRIFT_LIMIT_MS))
+ .setCacheTtl(100)
+ .create();
+
+ response = DefaultRequestPipeline.maybeFixDriftTime(response);
+ // Verify that the old time is ignored:
+ assertEquals(time + 100, roundToSeconds(response.getCacheExpiration()));
+ assertEquals(DateUtil.formatRfc1123Date(HttpResponseTest.timeSource.currentTimeMillis()),
+ response.getHeader("Date"));
+ }
+
+ @Test
+ public void testFixedDateNew() throws Exception {
+ int time = roundToSeconds(HttpResponseTest.timeSource.currentTimeMillis());
+ HttpResponse response = new HttpResponseBuilder()
+ .addHeader("Date", DateUtil.formatRfc1123Date(1000L * time
+ + 1000 + DefaultRequestPipeline.DEFAULT_DRIFT_LIMIT_MS))
+ .setCacheTtl(100)
+ .create();
+
+ response = DefaultRequestPipeline.maybeFixDriftTime(response);
+ // Verify that the old time is ignored:
+ assertEquals(time + 100, roundToSeconds(response.getCacheExpiration()));
+ assertEquals(DateUtil.formatRfc1123Date(HttpResponseTest.timeSource.currentTimeMillis()),
+ response.getHeader("Date"));
+ }
+
public static class FakeHttpFetcher implements HttpFetcher {
protected HttpRequest request;
protected HttpResponse response;
Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java?rev=1034046&r1=1034045&r2=1034046&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java Thu Nov 11 18:55:30 2010
@@ -56,10 +56,13 @@ public class HttpResponseTest extends As
return (int)(ts / 1000);
}
- private static FakeTimeSource timeSource = new FakeTimeSource(System.currentTimeMillis());
+ public static FakeTimeSource timeSource = new FakeTimeSource(System.currentTimeMillis());
+ public static void setHttpTimeSource() {
+ HttpResponse.setTimeSource(timeSource);
+ }
@Before
public void setUp() {
- HttpResponse.setTimeSource(timeSource);
+ setHttpTimeSource();
}
@Test
@@ -374,34 +377,6 @@ public class HttpResponseTest extends As
}
@Test
- public void testFixedDateOld() throws Exception {
- int time = roundToSeconds(timeSource.currentTimeMillis());
- HttpResponse response = new HttpResponseBuilder()
- .addHeader("Date", DateUtil.formatRfc1123Date(1000L * time
- - 1000 - HttpResponse.DEFAULT_DRIFT_LIMIT_MS))
- .create();
- // Verify that the old time is ignored:
- assertEquals(time + roundToSeconds(response.getDefaultTtl()),
- roundToSeconds(response.getCacheExpiration()));
- assertEquals(DateUtil.formatRfc1123Date(timeSource.currentTimeMillis()),
- response.getHeader("Date"));
- assertTtlOk(roundToSeconds(response.getDefaultTtl()), response);
- }
-
- @Test
- public void testFixedDateNew() throws Exception {
- int time = roundToSeconds(timeSource.currentTimeMillis());
- HttpResponse response = new HttpResponseBuilder()
- .addHeader("Date", DateUtil.formatRfc1123Date(1000L * time
- + 1000 + HttpResponse.DEFAULT_DRIFT_LIMIT_MS))
- .create();
- // Verify that the old time is ignored:
- assertEquals(time + roundToSeconds(response.getDefaultTtl()),
- roundToSeconds(response.getCacheExpiration()));
- assertTtlOk(roundToSeconds(response.getDefaultTtl()), response);
- }
-
- @Test
public void testNegativeCaching() {
assertTrue("Bad HTTP responses must be cacheable!",
HttpResponse.error().getCacheExpiration() > timeSource.currentTimeMillis());