mozilla-services / outgoing

A redirector for outgoing links.
Mozilla Public License 2.0
4 stars 5 forks source link

Urls are double-escaped in the outgoing HTML #6

Closed muffinresearch closed 7 years ago

muffinresearch commented 8 years ago

We are getting reports on amo that some links are not resolving correctly via outgoing. Looking at the HTML in outgoing that does the redirect it appears that the urls are double-escaped.

If I'm reading it correctly looks like go templates apply escaping by default [1] so & in the url gets escaped a second time to &

[1]https://golang.org/pkg/html/template/

Here's a limited failing test-case - though if the escaping of the url is removed you'll want some coverage to ensure the end result is doing the right thing.

diff --git a/outgoing_test.go b/outgoing_test.go
index 084c215..1f97853 100644
--- a/outgoing_test.go
+++ b/outgoing_test.go
@@ -70,6 +70,21 @@ func TestReq(t *testing.T) {
                t.Errorf("http://www.mozilla.org/ is not in %s.", rec.Body.String())
        }

+       hSig = getHmac("http://www.mozilla.org/?foo=1&bar=2", "secret")
+
+       rec = httptest.NewRecorder()
+       req, err = http.NewRequest("GET", "/v1/"+hSig+"/http%3A//www.mozilla.org/%3Ffoo%3D1%26bar%3D2", nil)
+       if err != nil {
+               t.Fatalf("hmac: %s err: %v", hSig, err)
+       }
+       readReq(rec, req)
+       if rec.Code != 200 {
+               t.Errorf("Expected %d, returned %d. hmac: %s", 200, rec.Code, hSig)
+       }
+       if !strings.Contains(rec.Body.String(), "http://www.mozilla.org/?foo=1&bar=2") {
+               t.Errorf("http://www.mozilla.org/?foo=1&bar=2 is not in %s.", rec.Body.String())
+       }
+
        hSig = getHmac("http://www.mozilla.org/", "badsecret")

        rec = httptest.NewRecorder()