From 889c886e7fccb47b022b25db311c07e915653675 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 30 Apr 2019 11:55:25 -0700 Subject: [PATCH 1/2] http: use the request context CloseNotifier has been deprecated for a while. Also, ditch the "environment" context. We don't actually _need_ this. --- cli/run_test.go | 4 ---- examples/adder/remote/server/main.go | 5 ----- executor.go | 8 ++----- executor_test.go | 4 ---- http/handler.go | 31 +--------------------------- http/handler_test.go | 7 ------- http/parse.go | 16 ++++++++++++-- http/parse_test.go | 6 +++--- 8 files changed, 20 insertions(+), 61 deletions(-) diff --git a/cli/run_test.go b/cli/run_test.go index fc502f95..a843e015 100644 --- a/cli/run_test.go +++ b/cli/run_test.go @@ -28,10 +28,6 @@ type env struct { ch chan struct{} } -func (e env) Context() context.Context { - return context.Background() -} - func TestRunWaits(t *testing.T) { flag := make(chan struct{}, 1) diff --git a/examples/adder/remote/server/main.go b/examples/adder/remote/server/main.go index a5ef069c..75c2ca98 100644 --- a/examples/adder/remote/server/main.go +++ b/examples/adder/remote/server/main.go @@ -1,7 +1,6 @@ package main import ( - "context" nethttp "net/http" "github.com/ipfs/go-ipfs-cmds/examples/adder" @@ -11,10 +10,6 @@ import ( type env struct{} -func (env) Context() context.Context { - return context.TODO() -} - func main() { h := http.NewHandler(env{}, adder.RootCmd, http.NewServerConfig()) diff --git a/executor.go b/executor.go index 05c43973..f25f5e68 100644 --- a/executor.go +++ b/executor.go @@ -8,12 +8,8 @@ type Executor interface { Execute(req *Request, re ResponseEmitter, env Environment) error } -// Environment is the environment passed to commands. The only required method -// is Context. -type Environment interface { - // Context returns the environment's context. - Context() context.Context -} +// Environment is the environment passed to commands. +type Environment interface{} // MakeEnvironment takes a context and the request to construct the environment // that is passed to the command's Run function. diff --git a/executor_test.go b/executor_test.go index 255773fe..86c1bd6a 100644 --- a/executor_test.go +++ b/executor_test.go @@ -38,10 +38,6 @@ type wc struct { type env int -func (e *env) Context() context.Context { - return context.Background() -} - func TestExecutor(t *testing.T) { env := env(42) req, err := NewRequest(context.Background(), []string{"test"}, nil, nil, nil, root) diff --git a/http/handler.go b/http/handler.go index 912ac61e..db01c448 100644 --- a/http/handler.go +++ b/http/handler.go @@ -2,8 +2,6 @@ package http import ( "context" - "crypto/rand" - "encoding/base32" "errors" "net/http" "runtime/debug" @@ -97,12 +95,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } }() - ctx := h.env.Context() - if ctx == nil { - log.Error("no root context found, using background") - ctx = context.Background() - } - if !allowOrigin(r, h.cfg) || !allowReferer(r, h.cfg) { w.WriteHeader(http.StatusForbidden) w.Write([]byte("403 - Forbidden")) @@ -128,7 +120,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { r.Body = bw } - req, err := parseRequest(ctx, r, h.root) + req, err := parseRequest(r, h.root) if err != nil { if err == ErrNotFound { w.WriteHeader(http.StatusNotFound) @@ -152,18 +144,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } defer cancel() - req.Context = logging.ContextWithLoggable(req.Context, uuidLoggable()) - if cn, ok := w.(http.CloseNotifier); ok { - clientGone := cn.CloseNotify() - go func() { - select { - case <-clientGone: - case <-req.Context.Done(): - } - cancel() - }() - } - re, err := NewResponseEmitter(w, r.Method, req, withRequestBodyEOFChan(bodyEOFChan)) if err != nil { w.WriteHeader(http.StatusBadRequest) @@ -186,15 +166,6 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.root.Call(req, re, h.env) } -func uuidLoggable() logging.Loggable { - ids := make([]byte, 16) - rand.Read(ids) - - return logging.Metadata{ - "requestId": base32.HexEncoding.EncodeToString(ids), - } -} - func sanitizedErrStr(err error) string { s := err.Error() s = strings.Split(s, "\n")[0] diff --git a/http/handler_test.go b/http/handler_test.go index b82f8b55..10cd9f35 100644 --- a/http/handler_test.go +++ b/http/handler_test.go @@ -2,7 +2,6 @@ package http import ( "bytes" - "context" "errors" "fmt" "io" @@ -26,15 +25,10 @@ type VersionOutput struct { type testEnv struct { version, commit, repoVersion string - rootCtx context.Context t *testing.T wait chan struct{} } -func (env testEnv) Context() context.Context { - return env.rootCtx -} - func getCommit(env cmds.Environment) (string, bool) { tEnv, ok := env.(testEnv) return tEnv.commit, ok @@ -307,7 +301,6 @@ func getTestServer(t *testing.T, origins []string) (cmds.Environment, *httptest. version: "0.1.2", commit: "c0mm17", // yes, I know there's no 'm' in hex. repoVersion: "4", - rootCtx: context.Background(), t: t, wait: make(chan struct{}), } diff --git a/http/parse.go b/http/parse.go index 63cabac1..250e51ea 100644 --- a/http/parse.go +++ b/http/parse.go @@ -1,7 +1,8 @@ package http import ( - "context" + "crypto/rand" + "encoding/base32" "fmt" "io/ioutil" "mime" @@ -12,10 +13,11 @@ import ( "github.com/ipfs/go-ipfs-cmds" "github.com/ipfs/go-ipfs-files" + logging "github.com/ipfs/go-log" ) // parseRequest parses the data in a http.Request and returns a command Request object -func parseRequest(ctx context.Context, r *http.Request, root *cmds.Command) (*cmds.Request, error) { +func parseRequest(r *http.Request, root *cmds.Command) (*cmds.Request, error) { if r.URL.Path[0] == '/' { r.URL.Path = r.URL.Path[1:] } @@ -134,6 +136,7 @@ func parseRequest(ctx context.Context, r *http.Request, root *cmds.Command) (*cm return nil, fmt.Errorf("File argument '%s' is required", requiredFile) } + ctx := logging.ContextWithLoggable(r.Context(), uuidLoggable()) req, err := cmds.NewRequest(ctx, pth, opts, args, f, root) if err != nil { return nil, err @@ -229,3 +232,12 @@ func parseResponse(httpRes *http.Response, req *cmds.Request) (cmds.Response, er return res, nil } + +func uuidLoggable() logging.Loggable { + ids := make([]byte, 16) + rand.Read(ids) + + return logging.Metadata{ + "requestId": base32.HexEncoding.EncodeToString(ids), + } +} diff --git a/http/parse_test.go b/http/parse_test.go index acfbc0a8..7e5ceae4 100644 --- a/http/parse_test.go +++ b/http/parse_test.go @@ -33,7 +33,7 @@ func TestParse(t *testing.T) { if err != nil { t.Fatal(err) } - req, err := parseRequest(nil, r, root) + req, err := parseRequest(r, root) if err != nil { t.Fatal(err) } @@ -47,7 +47,7 @@ func TestParse(t *testing.T) { if err != nil { t.Fatal(err) } - req, err = parseRequest(nil, r, root) + req, err = parseRequest(r, root) if err != ErrNotFound { t.Errorf("expected ErrNotFound, got: %v", err) } @@ -78,7 +78,7 @@ func (tc parseReqTestCase) test(t *testing.T) { } httpReq.URL.RawQuery = vs.Encode() - req, err := parseRequest(nil, httpReq, cmdRoot) + req, err := parseRequest(httpReq, cmdRoot) if !errEq(err, tc.err) { t.Fatalf("expected error to be %v, but got %v", tc.err, err) } From 8d7fadfee377f24dbbeccd1b3fae690175002844 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 13 May 2019 10:23:34 -0700 Subject: [PATCH 2/2] use math/rand for local IDs --- http/parse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http/parse.go b/http/parse.go index 250e51ea..f57d72d3 100644 --- a/http/parse.go +++ b/http/parse.go @@ -1,10 +1,10 @@ package http import ( - "crypto/rand" "encoding/base32" "fmt" "io/ioutil" + "math/rand" "mime" "net/http" "strconv"