Add support in imagesrcset for URLs with commas and leading commas, per

twifkak · twifkak · commit a47f034c569a · 2020-10-22T17:59:10.000-07:00
https://html.spec.whatwg.org/multipage/images.html#parsing-a-srcset-attribute. Addresses #461. PiperOrigin-RevId: 338555233
diff --git a/transformer/transformer.go b/transformer/transformer.go
@@ -112,8 +112,9 @@ var ampAttrRE = func() *regexp.Regexp {
 	return r
 }()
 
-// firstSrcsetSourceRE captures the first source URL from a srcset.
-var firstSrcsetSourceRE = regexp.MustCompile(`[^\s,]+`)
+// firstSrcsetSourceRE captures the first source URL from a srcset. Caller must
+// remove trailing comma, if present.
+var firstSrcsetSourceRE = regexp.MustCompile(`^[\s,]*([^\s]+)`)
 
 // The allowed AMP formats, and their serialization as an html "amp4" attribute.
 var ampFormatSuffixes = map[rpb.Request_HtmlFormat]string{
@@ -229,11 +230,11 @@ func extractPreloads(dom *amphtml.DOM) []*rpb.Metadata_Preload {
 							}
 							// The href doesn't really matter here. Browsers will ignore it and instead prioritize whichever source is selected from imagesrcset. However, the Link header *must* have it.
 							// Stub this by just finding the first source URL possible.
-							firstSource := firstSrcsetSourceRE.FindString(imagesrcset)
+							firstSource := firstSrcsetSourceRE.FindStringSubmatch(imagesrcset)[1]
 							if firstSource == "" {
 								continue
 							}
-							href = firstSource
+							href = strings.TrimSuffix(firstSource, ",")
 						}
 						preload := &rpb.Metadata_Preload{Url: href, As: "image"}
 						for _, attr := range current.Attr {
diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go
@@ -125,6 +125,26 @@ func TestPreloads(t *testing.T) {
 			"<html ⚡><head></head><body></body></html>",
 			[]*rpb.Metadata_Preload{{Url: "foo", As: "image", Attributes: []*rpb.Metadata_Preload_Attribute{{Key: "imagesrcset", Val: "bar"}}}},
 		},
+		{
+			`<html ⚡><link rel=preload as=image imagesrcset="bar,">`,
+			"<html ⚡><head></head><body></body></html>",
+			[]*rpb.Metadata_Preload{{Url: "bar", As: "image", Attributes: []*rpb.Metadata_Preload_Attribute{{Key: "imagesrcset", Val: "bar,"}}}},
+		},
+		{
+			`<html ⚡><link rel=preload as=image imagesrcset="bar, baz">`,
+			"<html ⚡><head></head><body></body></html>",
+			[]*rpb.Metadata_Preload{{Url: "bar", As: "image", Attributes: []*rpb.Metadata_Preload_Attribute{{Key: "imagesrcset", Val: "bar, baz"}}}},
+		},
+		{
+			`<html ⚡><link rel=preload as=image imagesrcset=",, ,,bar">`,
+			"<html ⚡><head></head><body></body></html>",
+			[]*rpb.Metadata_Preload{{Url: "bar", As: "image", Attributes: []*rpb.Metadata_Preload_Attribute{{Key: "imagesrcset", Val: ",, ,,bar"}}}},
+		},
+		{
+			`<html ⚡><link rel=preload as=image imagesrcset="bar,baz quux">`,
+			"<html ⚡><head></head><body></body></html>",
+			[]*rpb.Metadata_Preload{{Url: "bar,baz", As: "image", Attributes: []*rpb.Metadata_Preload_Attribute{{Key: "imagesrcset", Val: "bar,baz quux"}}}},
+		},
 		{
 			"<html ⚡><link rel=preload as=image href=foo imagesrcset=bar imagesizes=100vw>",
 			"<html ⚡><head></head><body></body></html>",
