From b4216d8da8572c3d605c25f7fe6610f27aa720ec Mon Sep 17 00:00:00 2001
From: Will Norris <will@willnorris.com>
Date: Mon, 7 Dec 2015 23:06:02 -0800
Subject: [PATCH] remove Proxy pointer from TransformingTransport

This pointer was only needed to pass along the scaleUp option.  In order
to prevent someone from specifying the scaleUp option on an individual
request against the owner's wishes, we didn't encode or decode that
field on the Options struct.  Instead, we stored the value on the Proxy
object and then set it on the Options struct inside the
TransformingTransport.  This worked, but I never really liked binding
those two together.

Instead, we now treat scaleUp as a normal Option field, encoding and
decoding it with all the others.  The primary difference is that the
initial value from the request URL will always be overwritten with
whatever is set in Proxy.ScaleUp.  This decouples the
TransformingTransport from the Proxy, but prevents the option from being
set in the request URL.

Modifies #37
---
 data.go            |  9 ++++++++-
 imageproxy.go      | 13 ++++---------
 imageproxy_test.go | 11 -----------
 3 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/data.go b/data.go
index d912864..382be17 100644
--- a/data.go
+++ b/data.go
@@ -31,6 +31,7 @@ const (
 	optQualityPrefix   = "q"
 	optSignaturePrefix = "s"
 	optSizeDelimiter   = "x"
+	optScaleUp         = "scaleUp"
 )
 
 // URLError reports a malformed URL error.
@@ -66,7 +67,8 @@ type Options struct {
 	// HMAC Signature for signed requests.
 	Signature string
 
-	// Allow images to scale beyond their original dimensions.
+	// Allow image to scale beyond its original dimensions.  This value
+	// will always be overwritten by the value of Proxy.ScaleUp.
 	ScaleUp bool
 }
 
@@ -93,6 +95,9 @@ func (o Options) String() string {
 	if o.Signature != "" {
 		fmt.Fprintf(buf, ",%s%s", string(optSignaturePrefix), o.Signature)
 	}
+	if o.ScaleUp {
+		fmt.Fprintf(buf, ",%s", optScaleUp)
+	}
 	return buf.String()
 }
 
@@ -166,6 +171,8 @@ func ParseOptions(str string) Options {
 			options.FlipVertical = true
 		case opt == optFlipHorizontal:
 			options.FlipHorizontal = true
+		case opt == optScaleUp: // this option is intentionally not documented above
+			options.ScaleUp = true
 		case strings.HasPrefix(opt, optRotatePrefix):
 			value := strings.TrimPrefix(opt, optRotatePrefix)
 			options.Rotate, _ = strconv.Atoi(value)
diff --git a/imageproxy.go b/imageproxy.go
index 8be6ce8..c8b5342 100644
--- a/imageproxy.go
+++ b/imageproxy.go
@@ -81,7 +81,7 @@ func NewProxy(transport http.RoundTripper, cache Cache) *Proxy {
 
 	client := new(http.Client)
 	client.Transport = &httpcache.Transport{
-		Transport:           &TransformingTransport{transport, client, &proxy},
+		Transport:           &TransformingTransport{transport, client},
 		Cache:               cache,
 		MarkCachedResponses: true,
 	}
@@ -105,6 +105,9 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// assign static settings from proxy to req.Options
+	req.Options.ScaleUp = p.ScaleUp
+
 	if !p.allowed(req) {
 		msg := fmt.Sprintf("request does not contain an allowed host or valid signature")
 		glog.Error(msg)
@@ -259,9 +262,6 @@ type TransformingTransport struct {
 	// used rather than Transport directly in order to ensure that
 	// responses are properly cached.
 	CachingClient *http.Client
-
-	// Proxy is used to access command line flag settings during roundtripping.
-	Proxy *Proxy
 }
 
 // RoundTrip implements the http.RoundTripper interface.
@@ -287,11 +287,6 @@ func (t *TransformingTransport) RoundTrip(req *http.Request) (*http.Response, er
 
 	opt := ParseOptions(req.URL.Fragment)
 
-	// assign static settings from proxy to options
-	if t.Proxy != nil {
-		opt.ScaleUp = t.Proxy.ScaleUp
-	}
-
 	img, err := Transform(b, opt)
 	if err != nil {
 		glog.Errorf("error transforming image: %v", err)
diff --git a/imageproxy_test.go b/imageproxy_test.go
index 298e41f..9ddee1d 100644
--- a/imageproxy_test.go
+++ b/imageproxy_test.go
@@ -12,8 +12,6 @@ import (
 	"net/url"
 	"strings"
 	"testing"
-
-	"github.com/gregjones/httpcache"
 )
 
 func TestAllowed(t *testing.T) {
@@ -206,15 +204,6 @@ func TestCheck304(t *testing.T) {
 	}
 }
 
-// make sure that the proxy is passed to transport in order
-// to access the command line flags.
-func TestProxyPointer(t *testing.T) {
-	p := NewProxy(nil, nil)
-	if p.Client.Transport.(*httpcache.Transport).Transport.(*TransformingTransport).Proxy != p {
-		t.Errorf("Transport doesnt have proxy pointer")
-	}
-}
-
 // testTransport is an http.RoundTripper that returns certained canned
 // responses for particular requests.
 type testTransport struct{}