push: Allow pushing multiple resources via Link header (#1798)

* Allow pushing multiple resources via Link header

* Add nopush test case

* Extract Link header parsing to separate function

* Parser regexp-free

* Remove dead code, thx gometalinter

* Redundant condition - won't happen

* Reduce duplication

caddyhttp/push/handler.go

@@ -32,6 +32,7 @@ outer:
 		if !matches {
 			_, matches = httpserver.IndexFile(h.Root, urlPath, staticfiles.IndexPages)
 		}
+
 		if matches {
 			for _, resource := range rule.Resources {
 				pushErr := pusher.Push(resource.Path, &http.PushOptions{
@@ -57,26 +58,39 @@ outer:
 	return code, err
 }
 
-func (h Middleware) servePreloadLinks(pusher http.Pusher, headers http.Header, links []string) {
-	for _, link := range links {
-		parts := strings.Split(link, ";")
-
-		if link == "" || strings.HasSuffix(link, "nopush") {
+// servePreloadLinks parses Link headers from backend and pushes resources found in them.
+// For accepted header formats check parseLinkHeader function.
+//
+// If resource has 'nopush' attribute then it will be omitted.
+func (h Middleware) servePreloadLinks(pusher http.Pusher, headers http.Header, resources []string) {
+outer:
+	for _, resource := range resources {
+		for _, resource := range parseLinkHeader(resource) {
+			if _, exists := resource.params["nopush"]; exists {
 				continue
 			}
 
-		target := strings.TrimSuffix(strings.TrimPrefix(parts[0], "<"), ">")
+			if h.isRemoteResource(resource.uri) {
+				continue
+			}
 
-		err := pusher.Push(target, &http.PushOptions{
+			err := pusher.Push(resource.uri, &http.PushOptions{
 				Method: http.MethodGet,
 				Header: headers,
 			})
 
 			if err != nil {
-			break
+				break outer
 			}
 		}
 	}
+}
+
+func (h Middleware) isRemoteResource(resource string) bool {
+	return strings.HasPrefix(resource, "//") ||
+		strings.HasPrefix(resource, "http://") ||
+		strings.HasPrefix(resource, "https://")
+}
 
 func (h Middleware) mergeHeaders(l, r http.Header) http.Header {
 	out := http.Header{}

caddyhttp/push/handler_test.go

@@ -269,6 +269,52 @@ func TestMiddlewareShouldInterceptLinkHeader(t *testing.T) {
 	comparePushedResources(t, expectedPushedResources, pushingWriter.pushed)
 }
 
+func TestMiddlewareShouldInterceptLinkHeaderWithMultipleResources(t *testing.T) {
+	// given
+	request, err := http.NewRequest(http.MethodGet, "/index.html", nil)
+	writer := httptest.NewRecorder()
+
+	if err != nil {
+		t.Fatalf("Could not create HTTP request: %v", err)
+	}
+
+	middleware := Middleware{
+		Next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
+			w.Header().Add("Link", "</assets/css/screen.css?v=5fc240c512>; rel=preload; as=style,</content/images/2016/06/Timeouts-001.png>; rel=preload; as=image,</content/images/2016/06/Timeouts-002.png>; rel=preload; as=image")
+			w.Header().Add("Link", "<//cdn.bizible.com/scripts/bizible.js>; rel=preload; as=script,</resource.png>; rel=preload; as=script; nopush")
+			return 0, nil
+		}),
+		Rules: []Rule{},
+	}
+
+	pushingWriter := &MockedPusher{ResponseWriter: writer}
+
+	// when
+	_, err2 := middleware.ServeHTTP(pushingWriter, request)
+
+	// then
+	if err2 != nil {
+		t.Error("Should not return error")
+	}
+
+	expectedPushedResources := map[string]*http.PushOptions{
+		"/assets/css/screen.css?v=5fc240c512": {
+			Method: http.MethodGet,
+			Header: http.Header{},
+		},
+		"/content/images/2016/06/Timeouts-001.png": {
+			Method: http.MethodGet,
+			Header: http.Header{},
+		},
+		"/content/images/2016/06/Timeouts-002.png": {
+			Method: http.MethodGet,
+			Header: http.Header{},
+		},
+	}
+
+	comparePushedResources(t, expectedPushedResources, pushingWriter.pushed)
+}
+
 func TestMiddlewareShouldInterceptLinkHeaderPusherError(t *testing.T) {
 	// given
 	expectedHeaders := http.Header{"Accept-Encoding": []string{"br"}}

caddyhttp/push/link_parser.go

@@ -0,0 +1,63 @@
+package push
+
+import (
+	"strings"
+)
+
+const (
+	commaSeparator     = ","
+	semicolonSeparator = ";"
+	equalSeparator     = "="
+)
+
+type linkResource struct {
+	uri    string
+	params map[string]string
+}
+
+// parseLinkHeader is responsible for parsing Link header and returning list of found resources.
+//
+// Accepted formats are:
+// Link: </resource>; as=script
+// Link: </resource>; as=script,</resource2>; as=style
+// Link: </resource>;</resource2>
+func parseLinkHeader(header string) []linkResource {
+	resources := []linkResource{}
+
+	if header == "" {
+		return resources
+	}
+
+	for _, link := range strings.Split(header, commaSeparator) {
+		l := linkResource{params: make(map[string]string)}
+
+		li, ri := strings.Index(link, "<"), strings.Index(link, ">")
+
+		if li == -1 || ri == -1 {
+			continue
+		}
+
+		l.uri = strings.TrimSpace(link[li+1 : ri])
+
+		for _, param := range strings.Split(strings.TrimSpace(link[ri+1:]), semicolonSeparator) {
+			parts := strings.SplitN(strings.TrimSpace(param), equalSeparator, 2)
+			key := strings.TrimSpace(parts[0])
+
+			if key == "" {
+				continue
+			}
+
+			if len(parts) == 1 {
+				l.params[key] = key
+			}
+
+			if len(parts) == 2 {
+				l.params[key] = strings.TrimSpace(parts[1])
+			}
+		}
+
+		resources = append(resources, l)
+	}
+
+	return resources
+}

caddyhttp/push/link_parser_test.go

@@ -0,0 +1,73 @@
+package push
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestDifferentParserInputs(t *testing.T) {
+	testCases := []struct {
+		header            string
+		expectedResources []linkResource
+	}{
+		{
+			header:            "</resource>; as=script",
+			expectedResources: []linkResource{{uri: "/resource", params: map[string]string{"as": "script"}}},
+		},
+		{
+			header:            "</resource>",
+			expectedResources: []linkResource{{uri: "/resource", params: map[string]string{}}},
+		},
+		{
+			header:            "</resource>; nopush",
+			expectedResources: []linkResource{{uri: "/resource", params: map[string]string{"nopush": "nopush"}}},
+		},
+		{
+			header:            "</resource>;nopush;rel=next",
+			expectedResources: []linkResource{{uri: "/resource", params: map[string]string{"nopush": "nopush", "rel": "next"}}},
+		},
+		{
+			header: "</resource>;nopush;rel=next,</resource2>;nopush",
+			expectedResources: []linkResource{
+				{uri: "/resource", params: map[string]string{"nopush": "nopush", "rel": "next"}},
+				{uri: "/resource2", params: map[string]string{"nopush": "nopush"}},
+			},
+		},
+		{
+			header: "</resource>,</resource2>",
+			expectedResources: []linkResource{
+				{uri: "/resource", params: map[string]string{}},
+				{uri: "/resource2", params: map[string]string{}},
+			},
+		},
+		{
+			header:            "malformed",
+			expectedResources: []linkResource{},
+		},
+		{
+			header:            "<malformed",
+			expectedResources: []linkResource{},
+		},
+		{
+			header:            ",",
+			expectedResources: []linkResource{},
+		},
+		{
+			header:            ";",
+			expectedResources: []linkResource{},
+		},
+		{
+			header:            "</resource> ; ",
+			expectedResources: []linkResource{{uri: "/resource", params: map[string]string{}}},
+		},
+	}
+
+	for i, test := range testCases {
+
+		actualResources := parseLinkHeader(test.header)
+
+		if !reflect.DeepEqual(actualResources, test.expectedResources) {
+			t.Errorf("Test %d (header: %s) - expected resources %v, got %v", i, test.header, test.expectedResources, actualResources)
+		}
+	}
+}