Enable returning partial HTTPResponses

cmd/tusd/cli/hooks.go

@@ -2,7 +2,6 @@ package cli
 
 import (
 	"errors"
-	"fmt"
 	"strconv"
 	"strings"
 
@@ -151,14 +150,14 @@ func invokeHookSync(typ hooks.HookType, event handler.HookEvent) (httpRes handle
 	})
 
 	if err != nil {
-		err = fmt.Errorf("%s hook failed: %s", typ, err)
+		//err = fmt.Errorf("%s hook failed: %s", typ, err)
 		logEv(stderr, "HookInvocationError", "type", string(typ), "id", id, "error", err.Error())
 		MetricsHookErrorsTotal.WithLabelValues(string(typ)).Add(1)
 	} else if Flags.VerboseOutput {
 		logEv(stdout, "HookInvocationFinish", "type", string(typ), "id", id)
 	}
 
-	// IDEA: PreHooks work like this: error return value does not carry HTTP response information
+	// IDEA: PreHooks work like this: error return value does carry HTTP response information for error
 	// Instead the additional HTTP response return value
 
 	httpRes = hookRes.HTTPResponse
@@ -171,12 +170,10 @@ func invokeHookSync(typ hooks.HookType, event handler.HookEvent) (httpRes handle
 	// If the hook response includes the instruction to reject the upload, reuse the error code
 	// and message from ErrUploadRejectedByServer, but also include custom HTTP response values
 	if typ == hooks.HookPreCreate && hookRes.RejectUpload {
-		// TODO: Merge httpRes with default of ErrUploadRejected, so we always have a response code.
-		return httpRes, handler.Error{
-			ErrorCode:    handler.ErrUploadRejectedByServer.ErrorCode,
-			Message:      handler.ErrUploadRejectedByServer.Message,
-			HTTPResponse: httpRes,
-		}
+		err := handler.ErrUploadRejectedByServer
+		err.HTTPResponse = err.HTTPResponse.MergeWith(httpRes)
+
+		return httpRes, err
 	}
 
 	if typ == hooks.HookPostReceive && hookRes.StopUpload {

cmd/tusd/cli/hooks/hooks.go

@@ -1,5 +1,7 @@
 package hooks
 
+// TODO: Move hooks into a package in /pkg
+
 import (
 	"github.com/tus/tusd/pkg/handler"
 )

cmd/tusd/cli/hooks/plugin.go

@@ -7,8 +7,12 @@ import (
 	"os/exec"
 
 	"github.com/hashicorp/go-plugin"
+	"github.com/tus/tusd/pkg/handler"
 )
 
+// TODO: When the tusd process stops, the plugin does not get properly killed
+// and lives on as a zombie process.
+
 type PluginHook struct {
 	Path string
 
@@ -54,8 +58,8 @@ func (h *PluginHook) InvokeHook(req HookRequest) (HookResponse, error) {
 // directory. It is a UX feature, not a security feature.
 var handshakeConfig = plugin.HandshakeConfig{
 	ProtocolVersion:  1,
-	MagicCookieKey:   "BASIC_PLUGIN",
-	MagicCookieValue: "hello",
+	MagicCookieKey:   "TUSD_PLUGIN",
+	MagicCookieValue: "yes",
 }
 
 // pluginMap is the map of plugins we can dispense.
@@ -63,19 +67,35 @@ var pluginMap = map[string]plugin.Plugin{
 	"hookHandler": &HookHandlerPlugin{},
 }
 
+// TODO: Explain, mention that it is internal only
+// TODO: Do we actually need this? Maybe not...
+type InvokeHookRPCAnswer struct {
+	HookResponse HookResponse
+	TusdError    *handler.Error // Why is TusdError a pointer
+}
+
 // Here is an implementation that talks over RPC
 type HookHandlerRPC struct{ client *rpc.Client }
 
 func (g *HookHandlerRPC) Setup() error {
 	var res interface{}
 	err := g.client.Call("Plugin.Setup", new(interface{}), &res)
-	fmt.Println("after Setup")
 	return err
 }
 
-func (g *HookHandlerRPC) InvokeHook(req HookRequest) (res HookResponse, err error) {
-	err = g.client.Call("Plugin.InvokeHook", req, &res)
-	return res, err
+func (g *HookHandlerRPC) InvokeHook(req HookRequest) (HookResponse, error) {
+	var answer InvokeHookRPCAnswer
+	err := g.client.Call("Plugin.InvokeHook", req, &answer)
+	fmt.Printf("Client: %#v\n", answer.TusdError)
+	if err != nil {
+		return answer.HookResponse, err
+	}
+
+	if answer.TusdError != nil {
+		return answer.HookResponse, *answer.TusdError
+	}
+
+	return answer.HookResponse, nil
 }
 
 // Here is the RPC server that HookHandlerRPC talks to, conforming to
@@ -89,9 +109,21 @@ func (s *HookHandlerRPCServer) Setup(args interface{}, resp *interface{}) error
 	return s.Impl.Setup()
 }
 
-func (s *HookHandlerRPCServer) InvokeHook(args HookRequest, resp *HookResponse) (err error) {
-	*resp, err = s.Impl.InvokeHook(args)
-	return err
+func (s *HookHandlerRPCServer) InvokeHook(args HookRequest, answer *InvokeHookRPCAnswer) error {
+	resp, err := s.Impl.InvokeHook(args)
+	if err != nil {
+
+		if tusdErr, ok := err.(handler.Error); ok {
+			answer.TusdError = &tusdErr
+			return nil
+		} else {
+			return err
+		}
+	}
+
+	answer.HookResponse = resp
+
+	return nil
 }
 
 // This is the implementation of plugin.Plugin so we can serve/consume this

examples/hooks/plugin/hook_handler.go

@@ -26,6 +26,14 @@ func (g *MyHookHandler) InvokeHook(req hooks.HookRequest) (res hooks.HookRespons
 
 	if req.Type == hooks.HookPreCreate {
 		res.HTTPResponse.Headers["X-From-Pre-Create"] = "hello"
+
+		if req.Event.Upload.Size > 10 {
+			res.HTTPResponse.StatusCode = 413
+			res.HTTPResponse.Body = `{"error":"upload size is too large"}`
+			res.RejectUpload = true
+
+			return res, nil
+		}
 	}
 
 	if req.Type == hooks.HookPreFinish {
@@ -42,8 +50,8 @@ func (g *MyHookHandler) InvokeHook(req hooks.HookRequest) (res hooks.HookRespons
 // directory. It is a UX feature, not a security feature.
 var handshakeConfig = plugin.HandshakeConfig{
 	ProtocolVersion:  1,
-	MagicCookieKey:   "BASIC_PLUGIN",
-	MagicCookieValue: "hello",
+	MagicCookieKey:   "TUSD_PLUGIN",
+	MagicCookieValue: "yes",
 }
 
 func main() {

pkg/handler/unrouted_handler.go

@@ -118,18 +118,33 @@ func (resp HTTPResponse) writeTo(w http.ResponseWriter) {
 	}
 }
 
-func (resp *HTTPResponse) MergeWith(resp2 HTTPResponse) {
-	if resp2.StatusCode != 0 {
-		resp.StatusCode = resp2.StatusCode
-	}
+func (resp1 HTTPResponse) MergeWith(resp2 HTTPResponse) HTTPResponse {
+	// Clone the response 1 and use it as a basis
+	newResp := resp1
 
-	for key, value := range resp2.Headers {
-		resp.Headers[key] = value
+	// Take the status code and body from response 2 to
+	// overwrite values from response 1.
+	if resp2.StatusCode != 0 {
+		newResp.StatusCode = resp2.StatusCode
 	}
 
 	if len(resp2.Body) > 0 {
-		resp.Body = resp2.Body
+		newResp.Body = resp2.Body
 	}
+
+	// For the headers, me must make a new map to avoid writing
+	// into the header map from response 1.
+	newResp.Headers = make(HTTPHeaders, len(resp1.Headers)+len(resp2.Headers))
+
+	for key, value := range resp1.Headers {
+		newResp.Headers[key] = value
+	}
+
+	for key, value := range resp2.Headers {
+		newResp.Headers[key] = value
+	}
+
+	return newResp
 }
 
 // HookEvent represents an event from tusd which can be handled by the application.
@@ -400,7 +415,7 @@ func (handler *UnroutedHandler) PostFile(w http.ResponseWriter, r *http.Request)
 			handler.sendError(w, r, err)
 			return
 		}
-		resp.MergeWith(resp2)
+		resp = resp.MergeWith(resp2)
 	}
 
 	upload, err := handler.composer.Core.NewUpload(ctx, info)
@@ -770,7 +785,7 @@ func (handler *UnroutedHandler) finishUploadIfComplete(ctx context.Context, uplo
 			if err != nil {
 				return resp, err
 			}
-			resp.MergeWith(resp2)
+			resp = resp.MergeWith(resp2)
 		}
 	}
 
@@ -1035,7 +1050,7 @@ func (handler *UnroutedHandler) sendError(w http.ResponseWriter, r *http.Request
 func (handler *UnroutedHandler) sendResp(w http.ResponseWriter, r *http.Request, resp HTTPResponse) {
 	resp.writeTo(w)
 
-	handler.log("ResponseOutgoing", "status", strconv.Itoa(resp.StatusCode), "method", r.Method, "path", r.URL.Path, "requestId", getRequestId(r))
+	handler.log("ResponseOutgoing", "status", strconv.Itoa(resp.StatusCode), "method", r.Method, "path", r.URL.Path, "requestId", getRequestId(r), "body", resp.Body)
 }
 
 // Make an absolute URLs to the given upload id. If the base path is absolute