Repository: istio/fortio (go)

Issue:
% escaping makes loading URLs through fetch not work
http://X/fortio/fetch/Y/fortio1/fortio/?labels=Fortio&url=localhost%3A8080/echo&qps=100&t=3s&n=&c=8&p=50%2C+75%2C+90%2C+99%2C+99.9&r=0.0001&H=User-Agent%3A+istio%2Ffortio-0.8.0-pre&H=&H=&runner=http&save=on&load=Start

->

Fortio localhost%3A8080/echo

Aborting because unable to create client 0 for http://localhost%253A8080/echo


going to debug shows the issue:

GET /debug?labels=Fortio&url=localhost%253A8080/echo&qps=100&t=3s&n=&c=8&p=50%252C+75%252C+90%252C+99%252C+99.9&r=0.0001&H=User-Agent%253A+istio%252Ffortio-0.8.0-pre&H=&H=&runner=http&save=on&load=Start HTTP/1.1

%253A instead of %3A


Pull request:
remove logic to escape % in given urls
fixes #181 

@ldemailly this is a minor fix that I had on my local version as the escape logic failed a few of my perf test URLs. sending this in based on discussion on 181.

did i miss any documentation updates?

Patch:
diff --git a/fhttp/http_client.go b/fhttp/http_client.go
index 763f701..c39e55c 100644
--- a/fhttp/http_client.go
+++ b/fhttp/http_client.go
@@ -63,15 +63,12 @@ func NewHTTPOptions(url string) *HTTPOptions {
 }
 
 // Init initializes the headers in an HTTPOptions (User-Agent).
-// It replaces plain % to %25 in the url. If you already have properly
-// escaped URLs use o.URL = to set it.
 func (h *HTTPOptions) Init(url string) *HTTPOptions {
 	if h.initDone {
 		return h
 	}
 	h.initDone = true
-	// unescape then rescape % to %25 (so if it was already %25 it stays)
-	h.URL = strings.Replace(strings.Replace(url, "%25", "%", -1), "%", "%25", -1)
+	h.URL = url
 	h.NumConnections = 1
 	if h.HTTPReqTimeOut <= 0 {
 		h.HTTPReqTimeOut = HTTPReqTimeOutDefaultValue
diff --git a/ui/templates/main.html b/ui/templates/main.html
index 233263c..0a2f375 100644
--- a/ui/templates/main.html
+++ b/ui/templates/main.html
@@ -38,7 +38,7 @@ Count axis logarithmic: <input name="ylog" type="checkbox" onclick="updateChart(
 <form>
 <div>
 Title/Labels: <input type="text" name="labels" size="40" value="Fortio" /> (empty to skip title)<br />
-URL: <input type="text" name="url" size="80" value="http://{{.URLHostPort}}/echo?delay=250us:30%,5ms:5%&status=503:0.5%,429:1.5%" /> <br />
+URL: <input type="text" name="url" size="80" value="http://{{.URLHostPort}}/echo?delay=250us:30,5ms:5&status=503:0.5,429:1.5" /> <br />
 QPS: <input type="text" name="qps" size="6" value="1000" />
 Duration: <input id="duration" type="text" name="t" size="6" value="3s" />
 or run until interrupted: <input type="checkbox" name="t" onchange="toggleDuration(this)" />


Test patch:
diff --git a/fhttp/http_test.go b/fhttp/http_test.go
index 91ba26a..e9d4f91 100644
--- a/fhttp/http_test.go
+++ b/fhttp/http_test.go
@@ -88,9 +88,10 @@ func TestNewHTTPRequest(t *testing.T) {
 }
 
 func TestMultiInitAndEscape(t *testing.T) {
-	// one escaped already 2 not
-	o := NewHTTPOptions("localhost:8080/?delay=10ms:10%,0.5s:15%25,0.25s:5%")
-	expected := "http://localhost:8080/?delay=10ms:10%25,0.5s:15%25,0.25s:5%25"
+	// 2 escaped already
+	o := NewHTTPOptions("localhost%3A8080/?delay=10ms:10,0.5s:15%25,0.25s:5")
+	// shouldn't perform any escapes
+	expected := "http://localhost%3A8080/?delay=10ms:10,0.5s:15%25,0.25s:5"
 	if o.URL != expected {
 		t.Errorf("Got initially '%s', expected '%s'", o.URL, expected)
 	}
@@ -100,10 +101,6 @@ func TestMultiInitAndEscape(t *testing.T) {
 	if o.GetHeaders().Get("Foo") != "BaR" {
 		t.Errorf("Lost header after Init %+v", o.GetHeaders())
 	}
-	// Escaping should be indempotent
-	if o.URL != expected {
-		t.Errorf("Got after reinit '%s', expected '%s'", o.URL, expected)
-	}
 }
 
 func TestSchemeCheck(t *testing.T) {