diff --git a/imageproxy.go b/imageproxy.go index 55df86a..3431aef 100644 --- a/imageproxy.go +++ b/imageproxy.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package proxy provides the image proxy. +// Package imageproxy provides an image proxy server. package imageproxy // import "willnorris.com/go/imageproxy" import ( @@ -97,7 +97,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Add("Expires", resp.Header.Get("Expires")) w.Header().Add("Etag", resp.Header.Get("Etag")) - if is304 := check304(w, r, resp); is304 { + if is304 := check304(r, resp); is304 { w.WriteHeader(http.StatusNotModified) return } @@ -125,7 +125,13 @@ func (p *Proxy) allowed(u *url.URL) bool { return false } -func check304(w http.ResponseWriter, req *http.Request, resp *http.Response) bool { +// check304 checks whether we should send a 304 Not Modified in response to +// req, based on the response resp. This is determined using the last modified +// time and the entity tag of resp. +func check304(req *http.Request, resp *http.Response) bool { + // TODO(willnorris): if-none-match header can be a comma separated list + // of multiple tags to be matched, or the special value "*" which + // matches all etags etag := resp.Header.Get("Etag") if etag != "" && etag == req.Header.Get("If-None-Match") { return true diff --git a/imageproxy_test.go b/imageproxy_test.go new file mode 100644 index 0000000..5965c4e --- /dev/null +++ b/imageproxy_test.go @@ -0,0 +1,114 @@ +package imageproxy + +import ( + "bufio" + "net/http" + "net/url" + "strings" + "testing" +) + +func TestAllowed(t *testing.T) { + p := &Proxy{ + Whitelist: []string{"a.test", "*.b.test", "*c.test"}, + } + + tests := []struct { + url string + allowed bool + }{ + {"http://a.test/image", true}, + {"http://x.a.test/image", false}, + + {"http://b.test/image", true}, + {"http://x.b.test/image", true}, + {"http://x.y.b.test/image", true}, + + {"http://c.test/image", false}, + {"http://xc.test/image", false}, + {"/image", false}, + } + + for _, tt := range tests { + u, err := url.Parse(tt.url) + if err != nil { + t.Errorf("error parsing url %q: %v", tt.url, err) + } + if got, want := p.allowed(u), tt.allowed; got != want { + t.Errorf("allowed(%q) returned %v, want %v", u, got, want) + } + } +} + +func TestCheck304(t *testing.T) { + tests := []struct { + req, resp string + is304 bool + }{ + { // etag match + "GET / HTTP/1.1\nIf-None-Match: \"v\"\n\n", + "HTTP/1.1 200 OK\nEtag: \"v\"\n\n", + true, + }, + { // last-modified match + "GET / HTTP/1.1\nIf-Modified-Since: Sun, 02 Jan 2000 00:00:00 GMT\n\n", + "HTTP/1.1 200 OK\nLast-Modified: Sat, 01 Jan 2000 00:00:00 GMT\n\n", + true, + }, + + // mismatches + { + "GET / HTTP/1.1\n\n", + "HTTP/1.1 200 OK\n\n", + false, + }, + { + "GET / HTTP/1.1\n\n", + "HTTP/1.1 200 OK\nEtag: \"v\"\n\n", + false, + }, + { + "GET / HTTP/1.1\nIf-None-Match: \"v\"\n\n", + "HTTP/1.1 200 OK\n\n", + false, + }, + { + "GET / HTTP/1.1\nIf-None-Match: \"a\"\n\n", + "HTTP/1.1 200 OK\nEtag: \"b\"\n\n", + false, + }, + { // last-modified match + "GET / HTTP/1.1\n\n", + "HTTP/1.1 200 OK\nLast-Modified: Sat, 01 Jan 2000 00:00:00 GMT\n\n", + false, + }, + { // last-modified match + "GET / HTTP/1.1\nIf-Modified-Since: Sun, 02 Jan 2000 00:00:00 GMT\n\n", + "HTTP/1.1 200 OK\n\n", + false, + }, + { // last-modified match + "GET / HTTP/1.1\nIf-Modified-Since: Fri, 31 Dec 1999 00:00:00 GMT\n\n", + "HTTP/1.1 200 OK\nLast-Modified: Sat, 01 Jan 2000 00:00:00 GMT\n\n", + false, + }, + } + + for _, tt := range tests { + buf := bufio.NewReader(strings.NewReader(tt.req)) + req, err := http.ReadRequest(buf) + if err != nil { + t.Errorf("http.ReadRequest(%q) returned error: %v", tt.req, err) + } + + buf = bufio.NewReader(strings.NewReader(tt.resp)) + resp, err := http.ReadResponse(buf, req) + if err != nil { + t.Errorf("http.ReadResponse(%q) returned error: %v", tt.resp, err) + } + + if got, want := check304(req, resp), tt.is304; got != want { + t.Errorf("check304(%q, %q) returned: %v, want %v", tt.req, tt.resp, got, want) + } + } +}