fixed issue with proxing response - default response was updated and …

…affected responses of all baskets, rework of forwarding logic, more tests, documented 'proxy_response' flag

baskets.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"io/ioutil"
 	"log"
 	"net/http"
@@ -126,55 +127,58 @@ func ToRequestData(req *http.Request) *RequestData {
 }
 
 // Forward forwards request data to specified URL
-func (req *RequestData) Forward(client *http.Client, config BasketConfig, basket string) *http.Response {
-	body := strings.NewReader(req.Body)
+func (req *RequestData) Forward(client *http.Client, config BasketConfig, basket string) (*http.Response, error) {
 	forwardURL, err := url.ParseRequestURI(config.ForwardURL)
-
 	if err != nil {
-		log.Printf("[warn] invalid forward URL: %s; basket: %s", config.ForwardURL, basket)
-	} else {
-		// expand path
-		if config.ExpandPath && len(req.Path) > len(basket)+1 {
-			forwardURL.Path = expand(forwardURL.Path, req.Path, basket)
-		}
+		return nil, fmt.Errorf("Invalid forward URL: %s - %s", config.ForwardURL, err)
+	}
 
-		// append query
-		if len(req.Query) > 0 {
-			if len(forwardURL.RawQuery) > 0 {
-				forwardURL.RawQuery += "&" + req.Query
-			} else {
-				forwardURL.RawQuery = req.Query
-			}
-		}
+	// expand path
+	if config.ExpandPath && len(req.Path) > len(basket)+1 {
+		forwardURL.Path = expandURL(forwardURL.Path, req.Path, basket)
+	}
 
-		forwardReq, err := http.NewRequest(req.Method, forwardURL.String(), body)
-		if err != nil {
-			log.Printf("[error] failed to create forward request: %s", err)
+	// append query
+	if len(req.Query) > 0 {
+		if len(forwardURL.RawQuery) > 0 {
+			forwardURL.RawQuery += "&" + req.Query
 		} else {
-			// copy headers
-			for header, vals := range req.Header {
-				for _, val := range vals {
-					forwardReq.Header.Add(header, val)
-				}
-			}
-			// set do not forward header
-			forwardReq.Header.Set(DoNotForwardHeader, "1")
-
-			var response *http.Response
-			response, err = client.Do(forwardReq)
+			forwardURL.RawQuery = req.Query
+		}
+	}
 
-			if err != nil {
-				log.Printf("[error] failed to forward request: %s", err)
-				return &http.Response{}
-			}
+	forwardReq, err := http.NewRequest(req.Method, forwardURL.String(), strings.NewReader(req.Body))
+	if err != nil {
+		return nil, fmt.Errorf("Failed to create forward request: %s", err)
+	}
 
-			return response
+	// copy headers
+	for header, vals := range req.Header {
+		for _, val := range vals {
+			forwardReq.Header.Add(header, val)
 		}
 	}
-	return &http.Response{}
+	// set do not forward header
+	forwardReq.Header.Set(DoNotForwardHeader, "1")
+
+	// forward request
+	response, err := client.Do(forwardReq)
+	if err != nil {
+		// HTTP issue during forwarding - HTTP 502 Bad Gateway
+		log.Printf("[warn] failed to forward request for basket: %s - %s", basket, err)
+		badGatewayResp := &http.Response{
+			StatusCode: http.StatusBadGateway,
+			Header:     http.Header{},
+			Body:       ioutil.NopCloser(strings.NewReader(fmt.Sprintf("Failed to forward request: %s", err)))}
+		badGatewayResp.Header.Set("Content-Type", "text/plain")
+
+		return badGatewayResp, nil
+	}
+
+	return response, nil
 }
 
-func expand(url string, original string, basket string) string {
+func expandURL(url string, original string, basket string) string {
 	return strings.TrimSuffix(url, "/") + strings.TrimPrefix(original, "/"+basket)
 }
 

baskets_test.go

@@ -101,10 +101,13 @@ func TestRequestData_Forward_BrokenURL(t *testing.T) {
 	data.Path = "/" + basket
 
 	// Config to forward requests to broken URL
-	config := BasketConfig{ForwardURL: "-.'", ExpandPath: false, Capacity: 20}
+	config := BasketConfig{ForwardURL: "abc", ExpandPath: false, Capacity: 20}
 
 	// Should not fail, warning in log is expected
-	data.Forward(new(http.Client), config, basket)
+	r, e := data.Forward(new(http.Client), config, basket)
+	assert.Nil(t, r, "response is not expected")
+	assert.NotNil(t, e, "error is expected")
+	assert.EqualError(t, e, "Invalid forward URL: abc - parse abc: invalid URI for request", "wrong error")
 }
 
 func TestRequestData_Forward_UnreachableURL(t *testing.T) {
@@ -124,12 +127,15 @@ func TestRequestData_Forward_UnreachableURL(t *testing.T) {
 	config := BasketConfig{ForwardURL: "http://localhost:81/should/fail/to/forward", ExpandPath: false, Capacity: 20}
 
 	// Should not fail, warning in log is expected
-	data.Forward(new(http.Client), config, basket)
+	r, e := data.Forward(new(http.Client), config, basket)
+	assert.Nil(t, e, "error is not expected")
+	assert.NotNil(t, r, "response is expected")
+	assert.Equal(t, 502, r.StatusCode, "wrong status code")
 }
 
-func TestExpand(t *testing.T) {
-	assert.Equal(t, "/notify/abc/123-123", expand("/notify", "/sniffer/abc/123-123", "sniffer"))
-	assert.Equal(t, "/hello/world", expand("/", "/mybasket/hello/world", "mybasket"))
-	assert.Equal(t, "/notify/hello/world", expand("/notify", "/notify/hello/world", "notify"))
-	assert.Equal(t, "/receive/notification/test/", expand("/receive/notification/", "/basket/test/", "basket"))
+func TestExpandURL(t *testing.T) {
+	assert.Equal(t, "/notify/abc/123-123", expandURL("/notify", "/sniffer/abc/123-123", "sniffer"))
+	assert.Equal(t, "/hello/world", expandURL("/", "/mybasket/hello/world", "mybasket"))
+	assert.Equal(t, "/notify/hello/world", expandURL("/notify", "/notify/hello/world", "notify"))
+	assert.Equal(t, "/receive/notification/test/", expandURL("/receive/notification/", "/basket/test/", "basket"))
 }

doc/api-swagger.json

@@ -417,6 +417,10 @@
           "type" : "string",
           "description" : "URL to forward all incoming requests of the basket, `empty` value disables forwarding"
         },
+        "proxy_response" : {
+          "type" : "boolean",
+          "description" : "If set to `true` this basket behaves as a full proxy: responses from underlying service configured in `forward_url`\nare passed back to clients of original requests. The configuration of basket responses is ignored in this case.\n"
+        },
         "insecure_tls" : {
           "type" : "boolean",
           "description" : "If set to `true` the certificate verification will be disabled if forward URL indicates HTTPS scheme.\n**Warning:** enabling this feature has known security implications.\n"
@@ -531,4 +535,4 @@
       }
     }
   }
-}
+}

doc/api-swagger.yaml

@@ -369,6 +369,11 @@ definitions:
       forward_url:
         type: string
         description: URL to forward all incoming requests of the basket, `empty` value disables forwarding
+      proxy_response:
+        type: boolean
+        description: |
+          If set to `true` this basket behaves as a full proxy: responses from underlying service configured in `forward_url`
+          are passed back to clients of original requests. The configuration of basket responses is ignored in this case.
       insecure_tls:
         type: boolean
         description: |

handlers.go

@@ -17,7 +17,7 @@ import (
 )
 
 var validBasketName = regexp.MustCompile(basketNamePattern)
-var defaultResponse = ResponseConfig{Status: 200, Headers: http.Header{}, IsTemplate: false}
+var defaultResponse = ResponseConfig{Status: http.StatusOK, Headers: http.Header{}, IsTemplate: false}
 var basketPageTemplate = template.Must(template.New("basket").Parse(basketPageContentTemplate))
 
 // writeJSON writes JSON content to HTTP response
@@ -351,60 +351,86 @@ func AcceptBasketRequests(w http.ResponseWriter, r *http.Request) {
 	if basket := basketsDb.Get(name); basket != nil {
 		request := basket.Add(r)
 
-		responseConfig := basket.GetResponse(r.Method)
-		if responseConfig == nil {
-			responseConfig = &defaultResponse
-		}
-
-		// forward request in separate thread
+		// forward request if configured and it's a first forwarding
 		config := basket.Config()
 		if len(config.ForwardURL) > 0 && r.Header.Get(DoNotForwardHeader) != "1" {
-			client := httpClient
-			if config.InsecureTLS {
-				client = httpInsecureClient
+			if config.ProxyResponse {
+				forwardAndProxyResponse(w, request, config, name)
+				return
 			}
 
-			if config.ProxyResponse {
-				response := request.Forward(client, config, name)
-				defer response.Body.Close()
+			go forwardAndForget(request, config, name)
+		}
 
-				body, err := ioutil.ReadAll(response.Body)
-				if err != nil {
-					http.Error(w, "Error in "+err.Error(), http.StatusInternalServerError)
-				}
+		writeBasketResponse(w, r, name, basket)
+	} else {
+		w.WriteHeader(http.StatusNotFound)
+	}
+}
 
-				responseConfig.Headers = response.Header
-				responseConfig.Status = response.StatusCode
-				responseConfig.Body = string(body)
-			} else {
-				go request.Forward(client, config, name)
-			}
-		}
+func forwardAndForget(request *RequestData, config BasketConfig, name string) {
+	// forward request and discard the response
+	response, err := request.Forward(getHTTPClient(config.InsecureTLS), config, name)
+	if err != nil {
+		log.Printf("[warn] failed to forward request for basket: %s - %s", name, err)
+	} else {
+		io.Copy(ioutil.Discard, response.Body)
+		response.Body.Close()
+	}
+}
 
+func forwardAndProxyResponse(w http.ResponseWriter, request *RequestData, config BasketConfig, name string) {
+	// forward request in a full proxy mode
+	response, err := request.Forward(getHTTPClient(config.InsecureTLS), config, name)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+	} else {
 		// headers
-		for k, v := range responseConfig.Headers {
+		for k, v := range response.Header {
 			w.Header()[k] = v
 		}
+
+		// status
+		w.WriteHeader(response.StatusCode)
+
 		// body
-		if responseConfig.IsTemplate && len(responseConfig.Body) > 0 {
-			// template
-			t, err := template.New(name + "-" + r.Method).Parse(responseConfig.Body)
-			if err != nil {
-				// invalid template
-				http.Error(w, "Error in "+err.Error(), http.StatusInternalServerError)
-			} else {
-				// status
-				w.WriteHeader(responseConfig.Status)
-				// templated body
-				t.Execute(w, r.URL.Query())
-			}
+		_, err := io.Copy(w, response.Body)
+		if err != nil {
+			log.Printf("[warn] failed to proxy response body for basket: %s - %s", name, err)
+			io.Copy(ioutil.Discard, response.Body)
+		}
+		response.Body.Close()
+	}
+}
+
+func writeBasketResponse(w http.ResponseWriter, r *http.Request, name string, basket Basket) {
+	response := basket.GetResponse(r.Method)
+	if response == nil {
+		response = &defaultResponse
+	}
+
+	// headers
+	for k, v := range response.Headers {
+		w.Header()[k] = v
+	}
+
+	// body
+	if response.IsTemplate && len(response.Body) > 0 {
+		// template
+		t, err := template.New(name + "-" + r.Method).Parse(response.Body)
+		if err != nil {
+			// invalid template
+			http.Error(w, "Error in "+err.Error(), http.StatusInternalServerError)
 		} else {
 			// status
-			w.WriteHeader(responseConfig.Status)
-			// plain body
-			w.Write([]byte(responseConfig.Body))
+			w.WriteHeader(response.Status)
+			// templated body
+			t.Execute(w, r.URL.Query())
 		}
 	} else {
-		w.WriteHeader(http.StatusNotFound)
+		// status
+		w.WriteHeader(response.Status)
+		// plain body
+		w.Write([]byte(response.Body))
 	}
 }