From a6583c01149f3d04fc196d8ffa2a209d7d2d9440 Mon Sep 17 00:00:00 2001
From: Maks Orlovich <morlovich@google.com>
Date: Thu, 18 Aug 2016 12:40:42 -0400
Subject: [PATCH] Don't include bogus last_modified = 0 in InputInfo when the
 resource didn't have a last-modified header. (Noticed when storing these in
 dependencies tables).

---
 net/instaweb/rewriter/resource.cc              |  4 +++-
 net/instaweb/rewriter/server_context_test.cc   | 11 ++++++++++-
 pagespeed/kernel/http/response_headers.cc      |  6 ++++++
 pagespeed/kernel/http/response_headers.h       |  1 +
 pagespeed/kernel/http/response_headers_test.cc |  2 ++
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/instaweb/rewriter/resource.cc b/net/instaweb/rewriter/resource.cc
index 2bb58a240..68448b8fe 100644
--- a/net/instaweb/rewriter/resource.cc
+++ b/net/instaweb/rewriter/resource.cc
@@ -207,7 +207,9 @@ void Resource::FillInPartitionInputInfo(HashHint include_content_hash,
 void Resource::FillInPartitionInputInfoFromResponseHeaders(
       const ResponseHeaders& headers,
       InputInfo* input) {
-  input->set_last_modified_time_ms(headers.last_modified_time_ms());
+  if (headers.has_last_modified_time_ms()) {
+    input->set_last_modified_time_ms(headers.last_modified_time_ms());
+  }
   input->set_expiration_time_ms(headers.CacheExpirationTimeMs());
   input->set_date_ms(headers.date_ms());
 }
diff --git a/net/instaweb/rewriter/server_context_test.cc b/net/instaweb/rewriter/server_context_test.cc
index 8be4618fd..c5b012f15 100644
--- a/net/instaweb/rewriter/server_context_test.cc
+++ b/net/instaweb/rewriter/server_context_test.cc
@@ -1745,7 +1745,8 @@ void CheckMatchesHeaders(const ResponseHeaders& headers,
   ASSERT_TRUE(input.has_type());
   EXPECT_EQ(InputInfo::CACHED, input.type());
 
-  ASSERT_TRUE(input.has_last_modified_time_ms());
+  EXPECT_EQ(headers.has_last_modified_time_ms(),
+            input.has_last_modified_time_ms());
   EXPECT_EQ(headers.last_modified_time_ms(), input.last_modified_time_ms());
 
   ASSERT_TRUE(input.has_expiration_time_ms());
@@ -1785,6 +1786,14 @@ TEST_F(ServerContextTest, FillInPartitionInputInfo) {
   ASSERT_TRUE(with_hash.has_input_content_hash());
   EXPECT_STREQ("zEEebBNnDlISRim4rIP30", with_hash.input_content_hash());
   EXPECT_FALSE(without_hash.has_input_content_hash());
+
+  resource->response_headers()->RemoveAll(HttpAttributes::kLastModified);
+  resource->response_headers()->ComputeCaching();
+  EXPECT_FALSE(resource->response_headers()->has_last_modified_time_ms());
+  InputInfo without_last_modified;
+  resource->FillInPartitionInputInfo(Resource::kOmitInputHash,
+                                     &without_last_modified);
+  CheckMatchesHeaders(*resource->response_headers(), without_last_modified);
 }
 
 // Test of referer for BackgroundFetch: When the resource fetching request
diff --git a/pagespeed/kernel/http/response_headers.cc b/pagespeed/kernel/http/response_headers.cc
index 0473f16a7..229dcabd8 100644
--- a/pagespeed/kernel/http/response_headers.cc
+++ b/pagespeed/kernel/http/response_headers.cc
@@ -246,6 +246,12 @@ void ResponseHeaders::set_reason_phrase(const StringPiece& reason_phrase) {
                                      reason_phrase.size());
 }
 
+bool ResponseHeaders::has_last_modified_time_ms() const {
+  DCHECK(!cache_fields_dirty_)
+      << "Call ComputeCaching() before last_modified_time_ms()";
+  return proto()->has_last_modified_time_ms();
+}
+
 int64 ResponseHeaders::last_modified_time_ms() const {
   DCHECK(!cache_fields_dirty_)
       << "Call ComputeCaching() before last_modified_time_ms()";
diff --git a/pagespeed/kernel/http/response_headers.h b/pagespeed/kernel/http/response_headers.h
index 08dcaaa11..3f50e00e1 100644
--- a/pagespeed/kernel/http/response_headers.h
+++ b/pagespeed/kernel/http/response_headers.h
@@ -224,6 +224,7 @@ class ResponseHeaders : public Headers<HttpResponseHeaders> {
     http_options_.implicit_cache_ttl_ms = ttl;
   }
 
+  bool has_last_modified_time_ms() const;
   int64 last_modified_time_ms() const;
   int64 date_ms() const;  // Timestamp from Date header.
   bool has_date_ms() const;
diff --git a/pagespeed/kernel/http/response_headers_test.cc b/pagespeed/kernel/http/response_headers_test.cc
index ae26c0819..3c27c4218 100644
--- a/pagespeed/kernel/http/response_headers_test.cc
+++ b/pagespeed/kernel/http/response_headers_test.cc
@@ -1633,12 +1633,14 @@ TEST_F(ResponseHeadersTest, FixupMissingDate) {
 
 TEST_F(ResponseHeadersTest, LastModifiedAsInt64) {
   response_headers_.Clear();
+  EXPECT_FALSE(response_headers_.has_last_modified_time_ms());
   response_headers_.SetLastModified(MockTimer::kApr_5_2010_ms);
   response_headers_.ComputeCaching();
   EXPECT_STREQ("Mon, 05 Apr 2010 18:51:26 GMT", response_headers_.Lookup1(
       HttpAttributes::kLastModified));
   EXPECT_EQ(MockTimer::kApr_5_2010_ms,
             response_headers_.last_modified_time_ms());
+  EXPECT_TRUE(response_headers_.has_last_modified_time_ms());
 }
 
 TEST_F(ResponseHeadersTest, DoNotCorrectValidDate) {