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/07/30 17:09:23 UTC

svn commit: r980820 - in /shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/http/HttpResponse.java test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java

Author: zhoresh
Date: Fri Jul 30 15:09:23 2010
New Revision: 980820

URL: http://svn.apache.org/viewvc?rev=980820&view=rev
Log:
Ignore bad time for http response from remote resource
http://codereview.appspot.com/1689060/show

Modified:
    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/HttpResponseTest.java

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=980820&r1=980819&r2=980820&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 Fri Jul 30 15:09:23 2010
@@ -126,6 +126,10 @@ 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;
 
@@ -139,7 +143,10 @@ public final class HttpResponse implemen
   @Inject(optional = true)
   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;
+
   // Holds character sets for fast conversion
   private static final Map<String, Charset> encodingToCharset = new MapMaker().makeMap();
 
@@ -166,7 +173,7 @@ public final class HttpResponse implemen
 
     // Always safe, HttpResponseBuilder won't modify the body.
     responseBytes = builder.getResponse();
-    
+
     // Copy headers after builder.getResponse(), since that can modify Content-Type.
     headerCopy.putAll(builder.getHeaders());
 
@@ -197,7 +204,7 @@ public final class HttpResponse implemen
   public static HttpResponse badrequest(String msg) {
     return new HttpResponse(SC_BAD_REQUEST, msg);
   }
-  
+
   public static HttpResponse timeout() {
     return new HttpResponse(SC_GATEWAY_TIMEOUT, "");
   }
@@ -223,7 +230,7 @@ public final class HttpResponse implemen
   public String getEncoding() {
     return encoding.name();
   }
-  
+
   /**
    * @return The Charset of the response body's encoding, if we were able to determine it.
    */
@@ -255,7 +262,7 @@ public final class HttpResponse implemen
   public String getResponseAsString() {
     if (responseString == null) {
       responseString = encoding.decode(ByteBuffer.wrap(responseBytes)).toString();
-      
+
       // Strip BOM if present.
       if (responseString.length() > 0 && responseString.codePointAt(0) == 0xFEFF) {
         responseString = responseString.substring(1);
@@ -443,16 +450,21 @@ public final class HttpResponse implemen
   private static long getAndUpdateDate(Multimap<String, String> headers) {
     // Validate the Date header. Must conform to the HTTP date format.
     long timestamp = -1;
+    long currentTime = System.currentTimeMillis();
     Collection<String> dates = headers.get("Date");
 
     if (!dates.isEmpty()) {
       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) {
-      timestamp = System.currentTimeMillis();
+      timestamp = currentTime;
       headers.put("Date", DateUtil.formatRfc1123Date(timestamp));
     }
     return timestamp;
@@ -464,7 +476,7 @@ public final class HttpResponse implemen
    * @return milliseconds of the ttl
    */
   public long getDefaultTtl() {
-    return defaultTtl;  
+    return defaultTtl;
   }
 
   /**
@@ -496,7 +508,7 @@ public final class HttpResponse implemen
           if (charset.length() >= 2 && charset.startsWith("\"") && charset.endsWith("\"")) {
             charset = charset.substring(1, charset.length() - 1);
           }
-          
+
           try {
             return charsetForName(charset);
           } catch (IllegalArgumentException e) {
@@ -529,10 +541,10 @@ public final class HttpResponse implemen
       charset = Charset.forName(encoding);
       encodingToCharset.put(encoding, charset);
     }
-    
+
     return charset;
   }
-  
+
   @Override
   public int hashCode() {
     return httpStatusCode
@@ -600,7 +612,7 @@ public final class HttpResponse implemen
    	 bodyLength -= cnt;
     }
     if (offset != responseBytes.length) {
-    	throw new IOException("Invalid body! Expected length = " + responseBytes.length + ", bytes readed = " + offset + '.'); 
+    	throw new IOException("Invalid body! Expected length = " + responseBytes.length + ", bytes readed = " + offset + '.');
     }
 
     date = getAndUpdateDate(headerCopy);

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=980820&r1=980819&r2=980820&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 Fri Jul 30 15:09:23 2010
@@ -124,7 +124,7 @@ public class HttpResponseTest extends As
         .create();
     assertEquals(BIG5_STRING, response.getResponseAsString());
     assertEquals("text/plain; charset=BIG5", response.getHeader("Content-Type"));
-    
+
     HttpResponseBuilder subResponseBuilder = new HttpResponseBuilder(response);
     subResponseBuilder.setContent(response.getResponseAsString());
     HttpResponse subResponse = subResponseBuilder.create();
@@ -171,7 +171,7 @@ public class HttpResponseTest extends As
         .create();
     assertEquals(HttpResponse.DEFAULT_ENCODING.name(), response.getEncoding());
   }
-  
+
   @Test
   public void testEncodingDetectionUtf8WithBomNoContentHeader() throws Exception {
     HttpResponse response = new HttpResponseBuilder()
@@ -256,7 +256,7 @@ public class HttpResponseTest extends As
     assertEquals(expected, expires);
     assertTrue(response.getCacheTtl() <= response.getDefaultTtl() && response.getCacheTtl() > 0);
   }
-  
+
   @Test
   public void testCachingHeadersIgnoredOnError() throws Exception {
     HttpResponse response = new HttpResponseBuilder()
@@ -297,13 +297,13 @@ public class HttpResponseTest extends As
     // Second rounding makes this n-1.
     assertTtlOk(maxAge, response);
   }
-  
+
   @Test
   public void testExpiresZeroValue() throws Exception {
     HttpResponse response = new HttpResponseBuilder().addHeader("Expires", "0").create();
     assertEquals(0, roundToSeconds(response.getCacheExpiration()));
   }
-  
+
   @Test
   public void testExpiresUnknownValue() throws Exception {
     HttpResponse response = new HttpResponseBuilder().addHeader("Expires", "howdy").create();
@@ -365,6 +365,32 @@ public class HttpResponseTest extends As
   }
 
   @Test
+  public void testFixedDateOld() throws Exception {
+    int time = roundToSeconds(System.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 testFixedDateNew() throws Exception {
+    int time = roundToSeconds(System.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() > System.currentTimeMillis());