0
Fork 0
mirror of https://github.com/willnorris/imageproxy.git synced 2024-12-16 21:56:43 -05:00

change how content-type enforcement is handled

If no content types are specified, then accept all responses, regardless
of content type (this is the behavior imageproxy has historically had).
Change default value for the contentTypes flag to be "image/*", so that
the new default when running cmd/imageproxy is that only images will be
proxied.  The old default behavior can be achieved by passing an empty
string for the contentTypes flag:

    imageproxy -contentTypes ""

Do not send the "XCTO: nosniff" header, since all documentation that I
can find still says that it can cause problems when served with images.
If it's effectively a noop when an explicit content-type is specified in
the response, then this shouldn't actually matter for us either way.
But in the absence of certainty, I'd rather err on the side of following
the spec.

Also add documentation for the new functionality.

Fixes #141
This commit is contained in:
Will Norris 2018-09-15 04:29:05 +00:00
parent 39a4e1813d
commit 0370572130
4 changed files with 60 additions and 45 deletions

View file

@ -175,6 +175,14 @@ Reload the [codercat URL][], and you should now get an error message. You can
specify multiple hosts as a comma separated list, or prefix a host value with specify multiple hosts as a comma separated list, or prefix a host value with
`*.` to allow all sub-domains as well. `*.` to allow all sub-domains as well.
### Content-Type whitelist ###
You can limit what content types can be proxied by using the `contentTypes`
flag. By default, this is set to `image/*`, meaning that imageproxy will
process any image types. You can specify multiple content types as a comma
separated list, and suffix values with `*` to perform a wildcard match. Set the
flag to an empty string to proxy all requests, regardless of content type.
### Signed Requests ### ### Signed Requests ###
Instead of a host whitelist, you can require that requests be signed. This is Instead of a host whitelist, you can require that requests be signed. This is

View file

@ -51,7 +51,7 @@ var scaleUp = flag.Bool("scaleUp", false, "allow images to scale beyond their or
var timeout = flag.Duration("timeout", 0, "time limit for requests served by this proxy") var timeout = flag.Duration("timeout", 0, "time limit for requests served by this proxy")
var verbose = flag.Bool("verbose", false, "print verbose logging messages") var verbose = flag.Bool("verbose", false, "print verbose logging messages")
var version = flag.Bool("version", false, "Deprecated: this flag does nothing") var version = flag.Bool("version", false, "Deprecated: this flag does nothing")
var contentTypes = flag.String("contentTypes", "", "comma separated list of allowed content types") var contentTypes = flag.String("contentTypes", "image/*", "comma separated list of allowed content types")
func init() { func init() {
flag.Var(&cache, "cache", "location to cache images (see https://github.com/willnorris/imageproxy#cache)") flag.Var(&cache, "cache", "location to cache images (see https://github.com/willnorris/imageproxy#cache)")

View file

@ -169,12 +169,13 @@ func (p *Proxy) serveImage(w http.ResponseWriter, r *http.Request) {
} }
contentType, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type")) contentType, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type"))
if !validContentType(p.ContentTypes, contentType) { if resp.ContentLength != 0 && !validContentType(p.ContentTypes, contentType) {
http.Error(w, "forbidden content-type", http.StatusForbidden) msg := fmt.Sprintf("forbidden content-type: %q", contentType)
log.Print(msg)
http.Error(w, msg, http.StatusForbidden)
return return
} }
w.Header().Set("Content-Type", contentType) w.Header().Set("Content-Type", contentType)
w.Header().Set("X-Content-Type-Options", "nosniff")
copyHeader(w.Header(), resp.Header, "Content-Length") copyHeader(w.Header(), resp.Header, "Content-Length")
@ -225,25 +226,10 @@ func (p *Proxy) allowed(r *Request) error {
return fmt.Errorf("request does not contain an allowed host or valid signature: %v", r) return fmt.Errorf("request does not contain an allowed host or valid signature: %v", r)
} }
// validContentType returns whether contentType matches one of the patterns. If no patterns are // validContentType returns whether contentType matches one of the allowed patterns.
// given, validContentType returns true if contentType matches one of the whitelisted image types.
func validContentType(patterns []string, contentType string) bool { func validContentType(patterns []string, contentType string) bool {
if len(patterns) == 0 { if len(patterns) == 0 {
switch contentType { return true
case "image/bmp", "image/cgm", "image/g3fax", "image/gif", "image/ief", "image/jp2",
"image/jpeg", "image/jpg", "image/pict", "image/png", "image/prs.btif", "image/svg+xml",
"image/tiff", "image/vnd.adobe.photoshop", "image/vnd.djvu", "image/vnd.dwg",
"image/vnd.dxf", "image/vnd.fastbidsheet", "image/vnd.fpx", "image/vnd.fst",
"image/vnd.fujixerox.edmics-mmr", "image/vnd.fujixerox.edmics-rlc",
"image/vnd.microsoft.icon", "image/vnd.ms-modi", "image/vnd.net-fpx", "image/vnd.wap.wbmp",
"image/vnd.xiff", "image/webp", "image/x-cmu-raster", "image/x-cmx", "image/x-icon",
"image/x-macpaint", "image/x-pcx", "image/x-pict", "image/x-portable-anymap",
"image/x-portable-bitmap", "image/x-portable-graymap", "image/x-portable-pixmap",
"image/x-quicktime", "image/x-rgb", "image/x-xbitmap", "image/x-xpixmap",
"image/x-xwindowdump":
return true
}
return false
} }
for _, pattern := range patterns { for _, pattern := range patterns {

View file

@ -304,7 +304,7 @@ func (t testTransport) RoundTrip(req *http.Request) (*http.Response, error) {
case "/error": case "/error":
return nil, errors.New("http protocol error") return nil, errors.New("http protocol error")
case "/nocontent": case "/nocontent":
raw = "HTTP/1.1 204 No Content\nContent-Type: image/png\n\n" raw = "HTTP/1.1 204 No Content\n\n"
case "/etag": case "/etag":
raw = "HTTP/1.1 200 OK\nEtag: \"tag\"\n\n" raw = "HTTP/1.1 200 OK\nEtag: \"tag\"\n\n"
case "/png": case "/png":
@ -326,7 +326,8 @@ func TestProxy_ServeHTTP(t *testing.T) {
Client: &http.Client{ Client: &http.Client{
Transport: testTransport{}, Transport: testTransport{},
}, },
Whitelist: []string{"good.test"}, Whitelist: []string{"good.test"},
ContentTypes: []string{"image/*"},
} }
tests := []struct { tests := []struct {
@ -413,29 +414,49 @@ func TestTransformingTransport(t *testing.T) {
} }
func TestValidContentType(t *testing.T) { func TestValidContentType(t *testing.T) {
for contentType, expected := range map[string]bool{ tests := []struct {
"": false, patterns []string
"image/png": true, contentType string
"text/html": false, valid bool
} { }{
actual := validContentType(nil, contentType) // no patterns
if actual != expected { {nil, "", true},
t.Errorf("got %v, expected %v for content type: %v", actual, expected, contentType) {nil, "text/plain", true},
} {[]string{}, "", true},
} {[]string{}, "text/plain", true},
}
func TestValidContentType_Patterns(t *testing.T) { // empty pattern
for contentType, expected := range map[string]bool{ {[]string{""}, "", true},
"": false, {[]string{""}, "text/plain", false},
"image/png": false,
"foo/asdf": true, // exact match
"bar/baz": true, {[]string{"text/plain"}, "", false},
"bar/bazz": false, {[]string{"text/plain"}, "text", false},
} { {[]string{"text/plain"}, "text/html", false},
actual := validContentType([]string{"foo/*", "bar/baz"}, contentType) {[]string{"text/plain"}, "text/plain", true},
if actual != expected { {[]string{"text/plain"}, "text/plaintext", false},
t.Errorf("got %v, expected %v for content type: %v", actual, expected, contentType) {[]string{"text/plain"}, "text/plain+foo", false},
// wildcard match
{[]string{"text/*"}, "", false},
{[]string{"text/*"}, "text", false},
{[]string{"text/*"}, "text/html", true},
{[]string{"text/*"}, "text/plain", true},
{[]string{"text/*"}, "image/jpeg", false},
{[]string{"image/svg*"}, "image/svg", true},
{[]string{"image/svg*"}, "image/svg+html", true},
// complete wildcard does not match
{[]string{"*"}, "text/foobar", false},
// multiple patterns
{[]string{"text/*", "image/*"}, "image/jpeg", true},
}
for _, tt := range tests {
got := validContentType(tt.patterns, tt.contentType)
if want := tt.valid; got != want {
t.Errorf("validContentType(%q, %q) returned %v, want %v", tt.patterns, tt.contentType, got, want)
} }
} }
} }