From bc7ac096cb01c18c680a661543f32582b4e7bcea Mon Sep 17 00:00:00 2001
From: Googler <noreply@google.com>
Date: Thu, 6 Aug 2020 19:21:56 -0700
Subject: [PATCH] Add support for link imagesrcset and imagesizes

Adds validator support for the two new `<link rel=preload>` attributes, and adds srcset URL rewriting for imagesrcset.

Fixes https://github.com/ampproject/amphtml/issues/29302

PiperOrigin-RevId: 325359140
---
 transformer/transformers/absoluteurl.go      | 35 ++++++++++--------
 transformer/transformers/absoluteurl_test.go |  8 ++++
 transformer/transformers/urlrewrite.go       | 39 +++++++++++++++-----
 transformer/transformers/urlrewrite_test.go  | 36 ++++++++++++++++++
 4 files changed, 93 insertions(+), 25 deletions(-)

diff --git a/transformer/transformers/absoluteurl.go b/transformer/transformers/absoluteurl.go
index 62e3c06ed..50a01fed8 100644
--- a/transformer/transformers/absoluteurl.go
+++ b/transformer/transformers/absoluteurl.go
@@ -62,6 +62,8 @@ var /* const */ imgTagAttrs = []string{"longdesc"}
 //     * action-xhr
 //   * Any <img> tag with attribute:
 //     * longdesc
+//   * Any <link> tag with attribute:
+//     * imagesrcset
 //
 // URLs in stylesheets and srcsets are handled by the ExternalUrlRewrite
 // transformer.
@@ -146,8 +148,13 @@ func AbsoluteURL(e *Context) error {
 					amphtml.ToAbsoluteURL(documentURL, e.BaseURL, href.Val))
 			}
 		}
-		if _, ok := htmlnode.FindAttribute(n, "", "srcset"); ok {
-			rewriteSrcsetURLs(n, documentURL, e.BaseURL)
+		if srcset, ok := htmlnode.FindAttribute(n, "", "srcset"); ok {
+			htmlnode.SetAttribute(n, "", "srcset", rewriteSrcsetURLs(documentURL, e.BaseURL, srcset.Val))
+		}
+		if n.DataAtom == atom.Link {
+			if srcset, ok := htmlnode.FindAttribute(n, "", "imagesrcset"); ok {
+				htmlnode.SetAttribute(n, "", "imagesrcset", rewriteSrcsetURLs(documentURL, e.BaseURL, srcset.Val))
+			}
 		}
 	}
 	return nil
@@ -165,18 +172,16 @@ func rewriteAbsoluteURLs(n *html.Node, documentURL string, baseURL *url.URL,
 	}
 }
 
-func rewriteSrcsetURLs(n *html.Node, documentURL string, baseURL *url.URL) {
-	if v, ok := htmlnode.GetAttributeVal(n, "", "srcset"); ok {
-		normalized, offsets := amphtml.ParseSrcset(v)
-		var sb strings.Builder
-		var pos int
-		for _, element := range offsets {
-			sb.WriteString(normalized[pos:element.Start])
-			sb.WriteString(amphtml.ToAbsoluteURL(
-				documentURL, baseURL, normalized[element.Start:element.End]))
-			pos = element.End
-		}
-		sb.WriteString(normalized[pos:])
-		htmlnode.SetAttribute(n, "", "srcset", sb.String())
+func rewriteSrcsetURLs(documentURL string, baseURL *url.URL, srcset string) string {
+	normalized, offsets := amphtml.ParseSrcset(srcset)
+	var sb strings.Builder
+	var pos int
+	for _, element := range offsets {
+		sb.WriteString(normalized[pos:element.Start])
+		sb.WriteString(amphtml.ToAbsoluteURL(
+			documentURL, baseURL, normalized[element.Start:element.End]))
+		pos = element.End
 	}
+	sb.WriteString(normalized[pos:])
+	return sb.String()
 }
diff --git a/transformer/transformers/absoluteurl_test.go b/transformer/transformers/absoluteurl_test.go
index 58fee6caf..60cfd357d 100644
--- a/transformer/transformers/absoluteurl_test.go
+++ b/transformer/transformers/absoluteurl_test.go
@@ -173,6 +173,14 @@ func TestAbsoluteURLTansformer(t *testing.T) {
 			baseURL:     "http://www.example.com",
 			documentURL: fooURL,
 		},
+		{
+			desc:  "imagesrcset rewritten",
+			input: "<link imagesrcset=\"200.png 200w, 400.png 400w\">",
+			expected: "<link imagesrcset=\"https://example.com/200.png 200w, " +
+				"https://example.com/400.png 400w\">",
+			baseURL:     "https://example.com/",
+			documentURL: barURL,
+		},
 	}
 	for _, tc := range tcs {
 		rawInput := tt.Concat(
diff --git a/transformer/transformers/urlrewrite.go b/transformer/transformers/urlrewrite.go
index 51be1160b..cb996e38f 100644
--- a/transformer/transformers/urlrewrite.go
+++ b/transformer/transformers/urlrewrite.go
@@ -64,6 +64,8 @@ type nodeMap map[string]*html.Node
 //  * <amp-img/amp-anim/img srcset>
 //  * <image href> / <image xlink:href> which are SVG-specific images.
 //  * <link rel=icon href>
+//  * <link rel=preload as=image href>
+//  * <link rel=preload imagesrcset>
 //  * <amp-video poster>
 //  * <use xlink:href>
 //  * a background image given in the <style amp-custom> tag / style attribute
@@ -104,12 +106,20 @@ func URLRewrite(e *Context) error {
 
 		switch n.Data {
 		case "link":
-			// Rewrite 'href' attribute within <link rel="icon" href=...> and variants
-			// to point into the AMP Cache.
-			if htmlnode.HasAttribute(n, "", "href") {
-				if v, ok := htmlnode.GetAttributeVal(n, "", "rel"); ok && fieldsContain(v, "icon") {
+			if rel, hasRel := htmlnode.GetAttributeVal(n, "", "rel"); hasRel {
+				// Rewrite 'href' attribute within <link rel="icon" href=...> and variants
+				// to point into the AMP Cache.
+				if fieldsContain(rel, "icon") {
 					ctx.parseSimpleImageAttr(n, "", "href")
 				}
+				// Rewrite the 'href' and 'imagesrcset' attributes within <link rel="preload" as="image">
+				// to point into the AMP Cache.
+				if fieldsContain(rel, "preload") {
+					if as, ok := htmlnode.GetAttributeVal(n, "", "as"); ok && as == "image" {
+						ctx.parseSimpleImageAttr(n, "", "href")
+					}
+				}
+				ctx.parseExistingSrcset(n, "", "imagesrcset")
 			}
 
 		case "amp-img", "amp-anim", "img":
@@ -120,7 +130,7 @@ func URLRewrite(e *Context) error {
 			}
 
 			if v, srcsetOk := htmlnode.GetAttributeVal(n, "", "srcset"); srcsetOk && len(v) > 0 {
-				ctx.parseExistingSrcset(n, v)
+				ctx.parseExistingSrcset(n, "", "srcset")
 			} else if srcOk {
 				ctx.parseNewSrcset(n, src)
 			}
@@ -346,11 +356,20 @@ func writeAndMark(sb *strings.Builder, pos *int, s string) {
 }
 
 // parseExistingSrcset normalizes and parses the srcset attribute.
-func (ctx *urlRewriteContext) parseExistingSrcset(n *html.Node, srcset string) {
-	normalized, offsets := amphtml.ParseSrcset(srcset)
-	nc := elementNodeContext{n, "", "srcset", offsets}
-	*ctx = append(*ctx, &nc)
-	htmlnode.SetAttribute(n, "", "srcset", normalized)
+func (ctx *urlRewriteContext) parseExistingSrcset(n *html.Node, namespace, attrName string) bool {
+	srcset, ok := htmlnode.FindAttribute(n, namespace, attrName)
+	if ok {
+		normalized, offsets := amphtml.ParseSrcset(srcset.Val)
+		if len(offsets) == 0 {
+			htmlnode.RemoveAttribute(n, srcset)
+			return false
+		}
+		htmlnode.SetAttribute(n, namespace, attrName, normalized)
+		nc := elementNodeContext{n, namespace, attrName, offsets}
+		*ctx = append(*ctx, &nc)
+		return true
+	}
+	return false
 }
 
 // Do not add srcset for responsive layout if the width attribute is smaller
diff --git a/transformer/transformers/urlrewrite_test.go b/transformer/transformers/urlrewrite_test.go
index 5dfbd4555..aa8a52a62 100644
--- a/transformer/transformers/urlrewrite_test.go
+++ b/transformer/transformers/urlrewrite_test.go
@@ -136,6 +136,42 @@ func TestURLRewrite_link(t *testing.T) {
 			expected: `<link rel="icon shortcut" href="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo"/>`,
 			base:     "http://www.example.com",
 		},
+		{
+			desc:     `link rel="preload" as="image" href`,
+			input:    `<link rel="preload" as="image" href=foo>`,
+			expected: `<link rel="preload" as="image" href="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo"/>`,
+			base:     "http://www.example.com",
+		},
+		{
+			desc:     `link rel="preload" as="image" imagesrcset`,
+			input:    `<link rel="preload" as="image" imagesrcset="/foo.jpg, bar.jpg 50w">`,
+			expected: `<link rel="preload" as="image" imagesrcset="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo.jpg 1x, https://www-example-com.cdn.ampproject.org/i/www.example.com/bar.jpg 50w"/>`,
+			base:     "http://www.example.com",
+		},
+		{
+			desc:     `link rel="preload" as="image" href and imagesrcset`,
+			input:    `<link rel="preload" as="image" href=foo.jpg imagesrcset="/bar.jpg, baz.jpg 50w">`,
+			expected: `<link rel="preload" as="image" href="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo.jpg" imagesrcset="https://www-example-com.cdn.ampproject.org/i/www.example.com/bar.jpg 1x, https://www-example-com.cdn.ampproject.org/i/www.example.com/baz.jpg 50w"/>`,
+			base:     "http://www.example.com",
+		},
+		{
+			desc:     `link rel="preload" as="image" badly formed imagesrcset removed`,
+			input:    `<link rel="preload" as="image" imagesrcset="/bar.jpg baz.jpg 50w">`,
+			expected: `<link rel="preload" as="image"/>`,
+			base:     "http://www.example.com",
+		},
+		{
+			desc:     `link rel="preload" href requires as="image" `,
+			input:    `<link rel="preload" href="/bar.jpg">`,
+			expected: `<link rel="preload" href="/bar.jpg"/>`,
+			base:     "http://www.example.com",
+		},
+		{
+			desc:     `link rel="preload" href does not require as="image" `,
+			input:    `<link rel="preload" imagesrcset="/foo.jpg, bar.jpg 50w">`,
+			expected: `<link rel="preload" imagesrcset="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo.jpg 1x, https://www-example-com.cdn.ampproject.org/i/www.example.com/bar.jpg 50w"/>`,
+			base:     "http://www.example.com",
+		},
 	}
 	runURLRewriteTestcases(t, tcs)
 }