Treat Expires: as 0 (issue735041)

[jo...@gmail.com]

Reviewers: shindig.remailer_gmail.com,

Description:
Per RFC2616, section 14.21:

"HTTP/1.1 clients and caches MUST treat other invalid date formats,
especially including the value "0", as in the past (i.e., "already
expired")."

So if you see a Date header but you can't parse it you should return 0
not -1 from getExpiresTime. Unless parseRfc1123Date always returns
zero for dates it can't parse in which case you're already set.

Please review this at http://codereview.appspot.com/735041/show

Affected files:
    
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
    
java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java


Index:  
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java
===================================================================
---  
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java	 
(revision 922057)
+++  
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponse.java	 
(working copy)
@@ -391,6 +391,12 @@
        Date expiresDate = DateUtil.parseRfc1123Date(expires);
        if (expiresDate != null) {
          return expiresDate.getTime();
+      } else {
+        // Per RFC2616, 14.21  
(http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.21):
+        // "HTTP/1.1 clients and caches MUST treat other invalid date  
formats,
+        // especially including the value "0", as in the past  
(i.e., "already
+        // expired")."
+        return 0;
        }
      }
      return -1;
Index:  
java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java
===================================================================
---  
java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java	 
(revision 922057)
+++  
java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpResponseTest.java	 
(working copy)
@@ -289,6 +289,18 @@
      // 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();
+    assertEquals(0, roundToSeconds(response.getCacheExpiration()));
+  }

    @Test
    public void testMaxAgeNoDate() throws Exception {
@@ -388,16 +400,13 @@
    @Test
    public void testRetryAfter() {
      HttpResponse response;
-    String nowPlus60 =  
DateUtil.formatRfc1123Date(System.currentTimeMillis() + 60 * 1000L);
-    for (String date : Arrays.asList("60", nowPlus60)) {
-      for (int rc : Arrays.asList(HttpResponse.SC_INTERNAL_SERVER_ERROR,  
HttpResponse.SC_GATEWAY_TIMEOUT, HttpResponse.SC_BAD_REQUEST)) {
-        response = new HttpResponseBuilder()
-            .setHttpStatusCode(rc)
-            .setHeader("Retry-After","60")
-            .create();
-        long ttl = response.getCacheTtl();
-        assertTrue(ttl <= 60 * 1000L && ttl > 0);
-      }
+    for (int rc : Arrays.asList(HttpResponse.SC_INTERNAL_SERVER_ERROR,  
HttpResponse.SC_GATEWAY_TIMEOUT, HttpResponse.SC_BAD_REQUEST)) {
+      response = new HttpResponseBuilder()
+          .setHttpStatusCode(rc)
+          .setHeader("Retry-After","60")
+          .create();
+      long ttl = response.getCacheTtl();
+      assertTrue(ttl <= 60 * 1000L && ttl > 0);
      }
    }