diff --git a/api/agent/call.go b/api/agent/call.go index 1cd5a0ea7..ee37772e8 100644 --- a/api/agent/call.go +++ b/api/agent/call.go @@ -49,6 +49,20 @@ type Param struct { } type Params []Param +func fixupRequestURL(req *http.Request) string { + if req.URL.Scheme == "" { + if req.TLS == nil { + req.URL.Scheme = "http" + } else { + req.URL.Scheme = "https" + } + } + if req.URL.Host == "" { + req.URL.Host = req.Host + } + return req.URL.String() +} + func FromRequest(appName, path string, req *http.Request, params Params) CallOpt { return func(a *agent, c *call) error { app, err := a.da.GetApp(req.Context(), appName) @@ -62,9 +76,10 @@ func FromRequest(appName, path string, req *http.Request, params Params) CallOpt } if route.Format == "" { - route.Format = "default" + route.Format = models.FormatDefault } + url := fixupRequestURL(req) id := id.New().String() // baseVars are the vars on the route & app, not on this specific request [for hot functions] @@ -96,19 +111,7 @@ func FromRequest(appName, path string, req *http.Request, params Params) CallOpt envVars["FN_CALL_ID"] = id envVars["FN_METHOD"] = req.Method - envVars["FN_REQUEST_URL"] = func() string { - if req.URL.Scheme == "" { - if req.TLS == nil { - req.URL.Scheme = "http" - } else { - req.URL.Scheme = "https" - } - } - if req.URL.Host == "" { - req.URL.Host = req.Host - } - return req.URL.String() - }() + envVars["FN_REQUEST_URL"] = url // params for _, param := range params { @@ -172,7 +175,7 @@ func FromRequest(appName, path string, req *http.Request, params Params) CallOpt BaseEnv: baseVars, EnvVars: envVars, CreatedAt: strfmt.DateTime(time.Now()), - URL: req.URL.String(), // TODO we should probably strip host/port + URL: url, Method: req.Method, } diff --git a/api/agent/protocol/factory.go b/api/agent/protocol/factory.go index db87d4837..1e94459d3 100644 --- a/api/agent/protocol/factory.go +++ b/api/agent/protocol/factory.go @@ -72,7 +72,7 @@ func (ci callInfoImpl) Request() *http.Request { return ci.req } func (ci callInfoImpl) RequestURL() string { - return ci.req.URL.RequestURI() + return ci.call.URL } func (ci callInfoImpl) Headers() map[string][]string { diff --git a/api/agent/protocol/http.go b/api/agent/protocol/http.go index 82331ddf3..cc8f1dccf 100644 --- a/api/agent/protocol/http.go +++ b/api/agent/protocol/http.go @@ -27,7 +27,7 @@ func (p *HTTPProtocol) IsStreamable() bool { return true } // TODO maybe we should take io.Writer, io.Reader but then we have to // dump the request to a buffer again :( func (h *HTTPProtocol) Dispatch(ctx context.Context, ci CallInfo, w io.Writer) error { - err := DumpRequestTo(h.in, ci.Request()) // TODO timeout + err := DumpRequestTo(h.in, ci) // TODO timeout if err != nil { return err } @@ -70,32 +70,17 @@ func (h *HTTPProtocol) Dispatch(ctx context.Context, ci CallInfo, w io.Writer) e // the body in the process. // // TODO we should support h2! -func DumpRequestTo(w io.Writer, req *http.Request) error { - // By default, print out the unmodified req.RequestURI, which - // is always set for incoming server requests. But because we - // previously used req.URL.RequestURI and the docs weren't - // always so clear about when to use DumpRequest vs - // DumpRequestOut, fall back to the old way if the caller - // provides a non-server Request. +func DumpRequestTo(w io.Writer, ci CallInfo) error { - reqURI := req.RequestURI - if reqURI == "" { - reqURI = req.URL.RequestURI() - } + req := ci.Request() + reqURI := ci.RequestURL() fmt.Fprintf(w, "%s %s HTTP/%d.%d\r\n", valueOrDefault(req.Method, "GET"), reqURI, req.ProtoMajor, req.ProtoMinor) - absRequestURI := strings.HasPrefix(req.RequestURI, "http://") || strings.HasPrefix(req.RequestURI, "https://") - if !absRequestURI { - host := req.Host - if host == "" && req.URL != nil { - host = req.URL.Host - } - - if host != "" { - fmt.Fprintf(w, "Host: %s\r\n", host) - } + absRequestURI := strings.HasPrefix(reqURI, "http://") || strings.HasPrefix(reqURI, "https://") + if !absRequestURI && req.URL.Host != "" { + fmt.Fprintf(w, "Host: %s\r\n", req.URL.Host) } chunked := len(req.TransferEncoding) > 0 && req.TransferEncoding[0] == "chunked" diff --git a/api/agent/protocol/json_test.go b/api/agent/protocol/json_test.go index cad6a0429..7161ae3cc 100644 --- a/api/agent/protocol/json_test.go +++ b/api/agent/protocol/json_test.go @@ -16,7 +16,7 @@ type RequestData struct { A string `json:"a"` } -func setupRequest(data interface{}) *http.Request { +func setupRequest(data interface{}) *callInfoImpl { req := &http.Request{ Method: http.MethodPost, URL: &url.URL{ @@ -40,15 +40,20 @@ func setupRequest(data interface{}) *http.Request { _ = json.NewEncoder(&buf).Encode(data) } req.Body = ioutil.NopCloser(&buf) - return req + + call := &models.Call{Type: "json"} + + // fixup URL in models.Call + call.URL = req.URL.String() + + ci := &callInfoImpl{call, req} + return ci } func TestJSONProtocolwriteJSONInputRequestWithData(t *testing.T) { rDataBefore := RequestData{A: "a"} - req := setupRequest(rDataBefore) + ci := setupRequest(rDataBefore) r, w := io.Pipe() - call := &models.Call{Type: "json"} - ci := &callInfoImpl{call, req} proto := JSONProtocol{w, r} go func() { err := proto.writeJSONToContainer(ci) @@ -77,18 +82,15 @@ func TestJSONProtocolwriteJSONInputRequestWithData(t *testing.T) { t.Errorf("Request data assertion mismatch: expected: %s, got %s", rDataBefore.A, rDataAfter.A) } - if incomingReq.Protocol.Type != call.Type { + if incomingReq.Protocol.Type != ci.call.Type { t.Errorf("Call protocol type assertion mismatch: expected: %s, got %s", - call.Type, incomingReq.Protocol.Type) + ci.call.Type, incomingReq.Protocol.Type) } } func TestJSONProtocolwriteJSONInputRequestWithoutData(t *testing.T) { - req := setupRequest(nil) - - call := &models.Call{Type: "json"} + ci := setupRequest(nil) r, w := io.Pipe() - ci := &callInfoImpl{call, req} proto := JSONProtocol{w, r} go func() { err := proto.writeJSONToContainer(ci) @@ -112,18 +114,15 @@ func TestJSONProtocolwriteJSONInputRequestWithoutData(t *testing.T) { t.Errorf("Request body assertion mismatch: expected: %s, got %s", "", incomingReq.Body) } - if !models.Headers(req.Header).Equals(models.Headers(incomingReq.Protocol.Headers)) { + if !models.Headers(ci.req.Header).Equals(models.Headers(incomingReq.Protocol.Headers)) { t.Errorf("Request headers assertion mismatch: expected: %s, got %s", - req.Header, incomingReq.Protocol.Headers) + ci.req.Header, incomingReq.Protocol.Headers) } } func TestJSONProtocolwriteJSONInputRequestWithQuery(t *testing.T) { - req := setupRequest(nil) - + ci := setupRequest(nil) r, w := io.Pipe() - call := &models.Call{Type: "json"} - ci := &callInfoImpl{call, req} proto := JSONProtocol{w, r} go func() { err := proto.writeJSONToContainer(ci) @@ -143,8 +142,8 @@ func TestJSONProtocolwriteJSONInputRequestWithQuery(t *testing.T) { if err != nil { t.Error(err.Error()) } - if incomingReq.Protocol.RequestURL != req.URL.RequestURI() { + if incomingReq.Protocol.RequestURL != ci.call.URL { t.Errorf("Request URL does not match protocol URL: expected: %s, got %s", - req.URL.RequestURI(), incomingReq.Protocol.RequestURL) + ci.call.URL, incomingReq.Protocol.RequestURL) } } diff --git a/api/server/runner_async_test.go b/api/server/runner_async_test.go index 943cf96fb..adad4303b 100644 --- a/api/server/runner_async_test.go +++ b/api/server/runner_async_test.go @@ -40,6 +40,8 @@ func TestRouteRunnerAsyncExecution(t *testing.T) { {Name: "myapp", Config: map[string]string{"app": "true"}}, }, []*models.Route{ + {Type: "async", Path: "/hot-http", AppName: "myapp", Image: "fnproject/fn-test-utils", Format: "http", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 4, IdleTimeout: 30}, + {Type: "async", Path: "/hot-json", AppName: "myapp", Image: "fnproject/fn-test-utils", Format: "json", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 4, IdleTimeout: 30}, {Type: "async", Path: "/myroute", AppName: "myapp", Image: "fnproject/hello", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, {Type: "async", Path: "/myerror", AppName: "myapp", Image: "fnproject/error", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, {Type: "async", Path: "/myroute/:param", AppName: "myapp", Image: "fnproject/hello", Config: map[string]string{"test": "true"}, Memory: 128, Timeout: 30, IdleTimeout: 30}, @@ -55,6 +57,8 @@ func TestRouteRunnerAsyncExecution(t *testing.T) { expectedEnv map[string]string }{ {"/r/myapp/myroute", ``, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, + {"/r/myapp/hot-http", `{"sleepTime": 0, "isDebug": true}`, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, + {"/r/myapp/hot-json", `{"sleepTime": 0, "isDebug": true}`, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, // FIXME: this just hangs //{"/r/myapp/myroute/1", ``, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, {"/r/myapp/myerror", ``, map[string][]string{}, http.StatusAccepted, map[string]string{"TEST": "true", "APP": "true"}}, diff --git a/api/server/runner_test.go b/api/server/runner_test.go index 3cbbfa80c..4dbd4355c 100644 --- a/api/server/runner_test.go +++ b/api/server/runner_test.go @@ -245,6 +245,7 @@ func TestRouteRunnerTimeout(t *testing.T) { []*models.Route{ {Path: "/cold", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Memory: 128, Timeout: 4, IdleTimeout: 30}, {Path: "/hot", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: 128, Timeout: 4, IdleTimeout: 30}, + {Path: "/hot-json", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Format: "json", Memory: 128, Timeout: 4, IdleTimeout: 30}, {Path: "/bigmem-cold", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Memory: hugeMem, Timeout: 1, IdleTimeout: 30}, {Path: "/bigmem-hot", AppName: "myapp", Image: "fnproject/fn-test-utils", Type: "sync", Format: "http", Memory: hugeMem, Timeout: 1, IdleTimeout: 30}, }, nil, @@ -267,6 +268,8 @@ func TestRouteRunnerTimeout(t *testing.T) { {"/r/myapp/cold", `{"sleepTime": 5000, "isDebug": true}`, "POST", http.StatusGatewayTimeout, nil}, {"/r/myapp/hot", `{"sleepTime": 5000, "isDebug": true}`, "POST", http.StatusGatewayTimeout, nil}, {"/r/myapp/hot", `{"sleepTime": 0, "isDebug": true}`, "POST", http.StatusOK, nil}, + {"/r/myapp/hot-json", `{"sleepTime": 5000, "isDebug": true}`, "POST", http.StatusGatewayTimeout, nil}, + {"/r/myapp/hot-json", `{"sleepTime": 0, "isDebug": true}`, "POST", http.StatusOK, nil}, {"/r/myapp/bigmem-cold", `{"sleepTime": 0, "isDebug": true}`, "POST", http.StatusServiceUnavailable, map[string][]string{"Retry-After": {"15"}}}, {"/r/myapp/bigmem-hot", `{"sleepTime": 0, "isDebug": true}`, "POST", http.StatusServiceUnavailable, map[string][]string{"Retry-After": {"15"}}}, } { diff --git a/images/fn-test-utils/Gopkg.lock b/images/fn-test-utils/Gopkg.lock index dcebfda2b..51c5ea916 100644 --- a/images/fn-test-utils/Gopkg.lock +++ b/images/fn-test-utils/Gopkg.lock @@ -5,7 +5,7 @@ branch = "master" name = "github.com/fnproject/fdk-go" packages = ["."] - revision = "ce12b15e559bb56980c4134cbeadb99db9cd563a" + revision = "2d62538bfa636d3135c957ec7b78f2bedeeffa3d" [solve-meta] analyzer-name = "dep"