Don't include rel=canonical headers on IPRO resources.

Fixes #1222

Author: jeffkaufman

net/instaweb/rewriter/in_place_rewrite_context.cc

@@ -433,6 +433,12 @@ void InPlaceRewriteContext::FixFetchFallbackHeaders(
     headers->SetDateAndCaching(now_ms, expire_at_ms - now_ms);
     AddVaryIfRequired(cached_result, headers);
   }
+  RemoveRedundantRelCanonicalHeader(cached_result, headers);
+}
+
+void InPlaceRewriteContext::RemoveRedundantRelCanonicalHeader(
+    const CachedResult& cached_result, ResponseHeaders* headers) {
+  headers->Remove(HttpAttributes::kLink, RelCanonicalHeaderValue(url_));
 }
 
 void InPlaceRewriteContext::UpdateDateAndExpiry(

net/instaweb/rewriter/public/in_place_rewrite_context.h

@@ -140,6 +140,11 @@ class InPlaceRewriteContext : public SingleRewriteContext {
   // if the fetch result may be browser dependent.
   void AddVaryIfRequired(const CachedResult& cached_result,
                          ResponseHeaders* headers) const;
+  // Image rewriting adds a Link rel=canonical header.  Because a single cached
+  // result can be served from multiple urls we do need to keep generating it.
+  // But when serving via IPRO we should remove it if the url hasn't changed.
+  void RemoveRedundantRelCanonicalHeader(const CachedResult& cached_result,
+                                         ResponseHeaders* headers);
 
   GoogleString url_;
   // Boolean indicating whether or not the resource was rewritten successfully.

net/instaweb/rewriter/public/single_rewrite_context.h

@@ -57,6 +57,9 @@ class SingleRewriteContext : public RewriteContext {
   void AddLinkRelCanonical(const ResourcePtr& input,
                            const OutputResourcePtr& output);
 
+  // Constructs a <url>; rel="canonical" value for use with a Link header.
+  GoogleString RelCanonicalHeaderValue(StringPiece url);
+
  private:
   DISALLOW_COPY_AND_ASSIGN(SingleRewriteContext);
 };

net/instaweb/rewriter/single_rewrite_context.cc

@@ -102,9 +102,11 @@ void SingleRewriteContext::AddLinkRelCanonical(
   }
 
   output->response_headers()->Add(
-      HttpAttributes::kLink,
-      StrCat("<", GoogleUrl::Sanitize(input->url()), ">; ",
-            "rel=\"canonical\""));
+      HttpAttributes::kLink, RelCanonicalHeaderValue(input->url()));
+}
+
+GoogleString SingleRewriteContext::RelCanonicalHeaderValue(StringPiece url) {
+  return StrCat("<", GoogleUrl::Sanitize(url), ">; rel=\"canonical\"");
 }
 
 }  // namespace net_instaweb

pagespeed/automatic/system_test.sh

@@ -89,6 +89,7 @@ fi
 
 run_test content_length
 run_test keep_data_urls
+run_test rel_canonical
 
 wait_for_async_tests
 

pagespeed/automatic/system_tests/rel_canonical.sh

@@ -0,0 +1,26 @@
+start_test rel-canonical
+
+# .pagespeed. resources should have Link rel=canonical headers, IPRO resources
+# should not have them.
+
+start_test link rel=canonical header not present with IPRO resources
+
+REL_CANONICAL_REGEXP='Link:.*rel.*canonical'
+
+URL=$EXAMPLE_ROOT/images/Puzzle.jpg
+# Fetch it a few times until IPRO is done and has given it an ipro ("aj") etag.
+fetch_until -save "$URL" 'grep -c E[Tt]ag:.W/.PSA-aj.' 1 --save-headers
+# rel=canonical should not be present.
+check [ $(grep -c "$REL_CANONICAL_REGEXP" $FETCH_FILE) = 0 ]
+
+start_test link rel=canonical header present with pagespeed.ce resources
+
+URL=$REWRITTEN_ROOT/images/Puzzle.jpg.pagespeed.ce.HASH.jpg
+OUT=$($CURL -D- -o/dev/null -sS $URL)
+check_from "$OUT" grep "$REL_CANONICAL_REGEXP"
+
+start_test link rel=canonical header present with pagespeed.ic resources
+
+URL=$REWRITTEN_ROOT/images/xPuzzle.jpg.pagespeed.ic.HASH.jpg
+OUT=$($CURL -D- -o/dev/null -sS  $URL)
+check_from "$OUT" grep "$REL_CANONICAL_REGEXP"