Don't add an extra canonical header when there is one already, and test

our behavior when one exists. Was motivated by #1238, but I can't seem to reproduce the behavior Jeff saw. If anyone has any ideas as to what I am missing, I would appreciate it. (Still going ahead with this since it does fix one case and adds more tests.)
apache · Jan 21, 2016 · 0195e29 · 0195e29
1 parent 02e8f7e
commit 0195e29
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 1 deletion.
diff --git a/net/instaweb/rewriter/cache_extender_test.cc b/net/instaweb/rewriter/cache_extender_test.cc
@@ -657,6 +657,19 @@ void CacheExtenderTest::TestCanonicalOnFallback(
   EXPECT_STREQ(ResponseHeaders::RelCanonicalHeaderValue(
                    StrCat(kTestDomain, "b.jpg")),
                headers.Lookup1(HttpAttributes::kLink));
+
+  // Should also propagate existing ones (and not add incompatible ones).
+  lru_cache()->Clear();
+  AddToResponse(StrCat(kTestDomain, "b.jpg"),
+                HttpAttributes::kLink,
+                ResponseHeaders::RelCanonicalHeaderValue(
+                    StrCat(kTestDomain, "notb.jpg")));
+  EXPECT_TRUE(
+      RewriteTestBase::FetchResourceUrl(out_jpeg_url, &content, &headers));
+
+  EXPECT_STREQ(ResponseHeaders::RelCanonicalHeaderValue(
+                   StrCat(kTestDomain, "notb.jpg")),
+               headers.Lookup1(HttpAttributes::kLink));
 }
 
 TEST_F(CacheExtenderTest, CanonicalOnFallbackToOtherPageSpeed) {

diff --git a/net/instaweb/rewriter/image_rewrite_filter_test.cc b/net/instaweb/rewriter/image_rewrite_filter_test.cc
@@ -2358,6 +2358,17 @@ TEST_F(ImageRewriteTest, CanonicalOnTimeout) {
   EXPECT_STREQ(ResponseHeaders::RelCanonicalHeaderValue(
                    StrCat(kTestDomain, "a.jpg")),
                headers.Lookup1(HttpAttributes::kLink));
+
+  // Now try with an existing canonical header. That should be preserved
+  lru_cache()->Clear();
+  AddToResponse(StrCat(kTestDomain, "a.jpg"),
+                HttpAttributes::kLink,
+                ResponseHeaders::RelCanonicalHeaderValue(
+                    StrCat(kTestDomain, "nota.jpg")));
+  EXPECT_TRUE(RewriteTestBase::FetchResourceUrl(out_url, &content, &headers));
+  EXPECT_STREQ(ResponseHeaders::RelCanonicalHeaderValue(
+                   StrCat(kTestDomain, "nota.jpg")),
+               headers.Lookup1(HttpAttributes::kLink));
 }
 
 TEST_F(ImageRewriteTest, HonorNoTransform) {

diff --git a/net/instaweb/rewriter/single_rewrite_context.cc b/net/instaweb/rewriter/single_rewrite_context.cc
@@ -84,7 +84,8 @@ void SingleRewriteContext::Rewrite(int partition_index,
 
 void SingleRewriteContext::AddLinkRelCanonical(
     const ResourcePtr& input, ResponseHeaders* output) {
-  if (output->HasLinkRelCanonical()) {
+  if (output->HasLinkRelCanonical() ||
+      input->response_headers()->HasLinkRelCanonical()) {
     return;
   }