diff --git a/proxy/data.go b/proxy/data.go index cbe3d86..51070f3 100644 --- a/proxy/data.go +++ b/proxy/data.go @@ -18,6 +18,7 @@ package proxy import ( "bytes" "fmt" + "net/http" "net/url" "strconv" "strings" @@ -116,6 +117,41 @@ type Request struct { Options *Options // Image transformation to perform } +// NewRequest parses an http.Request into an image request. +func NewRequest(r *http.Request) (*Request, error) { + var err error + req := new(Request) + + path := r.URL.Path[1:] // strip leading slash + req.URL, err = url.Parse(path) + if err != nil || !req.URL.IsAbs() { + // first segment is likely options + parts := strings.SplitN(path, "/", 2) + if len(parts) != 2 { + return nil, URLError{"too few path segments", r.URL} + } + + req.URL, err = url.Parse(parts[1]) + if err != nil { + return nil, URLError{fmt.Sprintf("unable to parse remote URL: %v", err), r.URL} + } + + req.Options = ParseOptions(parts[0]) + } + + if !req.URL.IsAbs() { + return nil, URLError{"must provide absolute remote URL", r.URL} + } + + if req.URL.Scheme != "http" && req.URL.Scheme != "https" { + return nil, URLError{"remote URL must have http or https URL", r.URL} + } + + // query string is always part of the remote URL + req.URL.RawQuery = r.URL.RawQuery + return req, nil +} + // Image represents a remote image that is being proxied. It tracks where // the image was originally retrieved from and how long the image can be cached. type Image struct { diff --git a/proxy/data_test.go b/proxy/data_test.go index 054b147..0df68db 100644 --- a/proxy/data_test.go +++ b/proxy/data_test.go @@ -15,6 +15,7 @@ package proxy import ( + "net/http" "reflect" "testing" ) @@ -80,3 +81,88 @@ func TestParseOptions(t *testing.T) { } } } + +// Test that request URLs are properly parsed into Options and RemoteURL. This +// test verifies that invalid remote URLs throw errors, and that valid +// combinations of Options and URL are accept. This does not exhaustively test +// the various Options that can be specified; see TestParseOptions for that. +func TestNewRequest(t *testing.T) { + tests := []struct { + URL string + RemoteURL string + Options *Options + ExpectError bool + }{ + // invalid URLs + { + "http://localhost/", "", nil, true, + }, + { + "http://localhost/1/", "", nil, true, + }, + { + "http://localhost//example.com/foo", "", nil, true, + }, + { + "http://localhost//ftp://example.com/foo", "", nil, true, + }, + + // invalid options. These won't return errors, but will not fully parse the options + { + "http://localhost/s/http://example.com/", + "http://example.com/", emptyOptions, false, + }, + { + "http://localhost/1xs/http://example.com/", + "http://example.com/", &Options{Width: 1}, false, + }, + + // valid URLs + { + "http://localhost/http://example.com/foo", + "http://example.com/foo", nil, false, + }, + { + "http://localhost//http://example.com/foo", + "http://example.com/foo", emptyOptions, false, + }, + { + "http://localhost//https://example.com/foo", + "https://example.com/foo", emptyOptions, false, + }, + { + "http://localhost/1x2/http://example.com/foo", + "http://example.com/foo", &Options{Width: 1, Height: 2}, false, + }, + { + "http://localhost//http://example.com/foo?bar", + "http://example.com/foo?bar", emptyOptions, false, + }, + } + + for i, tt := range tests { + req, err := http.NewRequest("GET", tt.URL, nil) + if err != nil { + t.Errorf("%d. Error parsing request: %v", i, err) + continue + } + + r, err := NewRequest(req) + if tt.ExpectError { + if err == nil { + t.Errorf("%d. Expected parsing error", i) + } + continue + } else if err != nil { + t.Errorf("%d. Error parsing request: %v", i, err) + continue + } + + if got := r.URL.String(); tt.RemoteURL != got { + t.Errorf("%d. Request URL = %v, want %v", i, got, tt.RemoteURL) + } + if !reflect.DeepEqual(tt.Options, r.Options) { + t.Errorf("%d. Request Options = %v, want %v", i, r.Options, tt.Options) + } + } +} diff --git a/proxy/proxy.go b/proxy/proxy.go index df0faa6..55a06f1 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -22,7 +22,6 @@ import ( "net/http" "net/url" "strconv" - "strings" "time" "github.com/golang/glog" @@ -39,41 +38,6 @@ func (e URLError) Error() string { return fmt.Sprintf("malformed URL %q: %s", e.URL, e.Message) } -// NewRequest parses an http.Request into an image request. -func NewRequest(r *http.Request) (*Request, error) { - var err error - req := new(Request) - - path := r.URL.Path[1:] // strip leading slash - req.URL, err = url.Parse(path) - if err != nil || !req.URL.IsAbs() { - // first segment is likely options - parts := strings.SplitN(path, "/", 2) - if len(parts) != 2 { - return nil, URLError{"too few path segments", r.URL} - } - - req.URL, err = url.Parse(parts[1]) - if err != nil { - return nil, URLError{fmt.Sprintf("unable to parse remote URL: %v", err), r.URL} - } - - req.Options = ParseOptions(parts[0]) - } - - if !req.URL.IsAbs() { - return nil, URLError{"must provide absolute remote URL", r.URL} - } - - if req.URL.Scheme != "http" && req.URL.Scheme != "https" { - return nil, URLError{"remote URL must have http or https URL", r.URL} - } - - // query string is always part of the remote URL - req.URL.RawQuery = r.URL.RawQuery - return req, nil -} - // Proxy serves image requests. type Proxy struct { Client *http.Client // client used to fetch remote URLs diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go deleted file mode 100644 index 9290035..0000000 --- a/proxy/proxy_test.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2013 Google Inc. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package proxy - -import ( - "net/http" - "reflect" - "testing" -) - -// Test that request URLs are properly parsed into Options and RemoteURL. This -// test verifies that invalid remote URLs throw errors, and that valid -// combinations of Options and URL are accept. This does not exhaustively test -// the various Options that can be specified; see TestParseOptions for that. -func TestNewRequest(t *testing.T) { - tests := []struct { - URL string - RemoteURL string - Options *Options - ExpectError bool - }{ - // invalid URLs - { - "http://localhost/", "", nil, true, - }, - { - "http://localhost/1/", "", nil, true, - }, - { - "http://localhost//example.com/foo", "", nil, true, - }, - { - "http://localhost//ftp://example.com/foo", "", nil, true, - }, - - // invalid options. These won't return errors, but will not fully parse the options - { - "http://localhost/s/http://example.com/", - "http://example.com/", emptyOptions, false, - }, - { - "http://localhost/1xs/http://example.com/", - "http://example.com/", &Options{Width: 1}, false, - }, - - // valid URLs - { - "http://localhost/http://example.com/foo", - "http://example.com/foo", nil, false, - }, - { - "http://localhost//http://example.com/foo", - "http://example.com/foo", emptyOptions, false, - }, - { - "http://localhost//https://example.com/foo", - "https://example.com/foo", emptyOptions, false, - }, - { - "http://localhost/1x2/http://example.com/foo", - "http://example.com/foo", &Options{Width: 1, Height: 2}, false, - }, - { - "http://localhost//http://example.com/foo?bar", - "http://example.com/foo?bar", emptyOptions, false, - }, - } - - for i, tt := range tests { - req, err := http.NewRequest("GET", tt.URL, nil) - if err != nil { - t.Errorf("%d. Error parsing request: %v", i, err) - continue - } - - r, err := NewRequest(req) - if tt.ExpectError { - if err == nil { - t.Errorf("%d. Expected parsing error", i) - } - continue - } else if err != nil { - t.Errorf("%d. Error parsing request: %v", i, err) - continue - } - - if got := r.URL.String(); tt.RemoteURL != got { - t.Errorf("%d. Request URL = %v, want %v", i, got, tt.RemoteURL) - } - if !reflect.DeepEqual(tt.Options, r.Options) { - t.Errorf("%d. Request Options = %v, want %v", i, r.Options, tt.Options) - } - } -}