From 6d7462ac99e3cb0f957f97309c3a9d596875a711 Mon Sep 17 00:00:00 2001 From: Mateusz Gajewski Date: Tue, 29 Aug 2017 03:38:29 +0200 Subject: [PATCH] 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 | 42 +++++++++++------ caddyhttp/push/handler_test.go | 46 +++++++++++++++++++ caddyhttp/push/link_parser.go | 63 ++++++++++++++++++++++++++ caddyhttp/push/link_parser_test.go | 73 ++++++++++++++++++++++++++++++ 4 files changed, 210 insertions(+), 14 deletions(-) create mode 100644 caddyhttp/push/link_parser.go create mode 100644 caddyhttp/push/link_parser_test.go diff --git a/caddyhttp/push/handler.go b/caddyhttp/push/handler.go index fcc6ab0b..5fe41c10 100644 --- a/caddyhttp/push/handler.go +++ b/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,27 +58,40 @@ outer: return code, err } -func (h Middleware) servePreloadLinks(pusher http.Pusher, headers http.Header, links []string) { - for _, link := range links { - parts := strings.Split(link, ";") +// 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 + } - if link == "" || strings.HasSuffix(link, "nopush") { - continue - } + if h.isRemoteResource(resource.uri) { + continue + } - target := strings.TrimSuffix(strings.TrimPrefix(parts[0], "<"), ">") + err := pusher.Push(resource.uri, &http.PushOptions{ + Method: http.MethodGet, + Header: headers, + }) - err := pusher.Push(target, &http.PushOptions{ - Method: http.MethodGet, - Header: headers, - }) - - if err != nil { - break + if err != nil { + 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{} diff --git a/caddyhttp/push/handler_test.go b/caddyhttp/push/handler_test.go index 40903b6f..fb32b3f6 100644 --- a/caddyhttp/push/handler_test.go +++ b/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", "; rel=preload; as=style,; rel=preload; as=image,; rel=preload; as=image") + w.Header().Add("Link", "; rel=preload; as=script,; 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"}} diff --git a/caddyhttp/push/link_parser.go b/caddyhttp/push/link_parser.go new file mode 100644 index 00000000..98ae9305 --- /dev/null +++ b/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: ; as=script +// Link: ; as=script,; as=style +// Link: ; +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 +} diff --git a/caddyhttp/push/link_parser_test.go b/caddyhttp/push/link_parser_test.go new file mode 100644 index 00000000..a7d964a7 --- /dev/null +++ b/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: "; as=script", + expectedResources: []linkResource{{uri: "/resource", params: map[string]string{"as": "script"}}}, + }, + { + header: "", + expectedResources: []linkResource{{uri: "/resource", params: map[string]string{}}}, + }, + { + header: "; nopush", + expectedResources: []linkResource{{uri: "/resource", params: map[string]string{"nopush": "nopush"}}}, + }, + { + header: ";nopush;rel=next", + expectedResources: []linkResource{{uri: "/resource", params: map[string]string{"nopush": "nopush", "rel": "next"}}}, + }, + { + header: ";nopush;rel=next,;nopush", + expectedResources: []linkResource{ + {uri: "/resource", params: map[string]string{"nopush": "nopush", "rel": "next"}}, + {uri: "/resource2", params: map[string]string{"nopush": "nopush"}}, + }, + }, + { + header: ",", + expectedResources: []linkResource{ + {uri: "/resource", params: map[string]string{}}, + {uri: "/resource2", params: map[string]string{}}, + }, + }, + { + header: "malformed", + expectedResources: []linkResource{}, + }, + { + header: " ; ", + 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) + } + } +}