rcserver: fix 500 error when marshalling errors from core/command

Before this change attempting to return an error from core/command
failed with a 500 error and a message about unmarshable types.

This is because it was attempting to marshal the input parameters
which get _response added to them which contains an unmarshalable
field.

This was fixed by using the original parameters in the error response
rather than the one modified during the error handling.

This also adds end to end tests for the streaming facilities as used
in core/command.

fs/rc/rcserver/rcserver.go

@@ -186,7 +186,7 @@ func writeError(path string, in rc.Params, w http.ResponseWriter, err error, sta
 	})
 	if err != nil {
 		// can't return the error at this point
-		fs.Errorf(nil, "rc: failed to write JSON output: %v", err)
+		fs.Errorf(nil, "rc: writeError: failed to write JSON output from %#v: %v", in, err)
 	}
 }
 
@@ -270,6 +270,9 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 		writeError(path, in, w, errors.Errorf("authentication must be set up on the rc server to use %q or the --rc-no-auth flag must be in use", path), http.StatusForbidden)
 		return
 	}
+
+	inOrig := in.Copy()
+
 	if call.NeedsRequest {
 		// Add the request to RC
 		in["_request"] = r
@@ -282,7 +285,7 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 	// Check to see if it is async or not
 	isAsync, err := in.GetBool("_async")
 	if rc.NotErrParamNotFound(err) {
-		writeError(path, in, w, err, http.StatusBadRequest)
+		writeError(path, inOrig, w, err, http.StatusBadRequest)
 		return
 	}
 	delete(in, "_async") // remove the async parameter after parsing so vfs operations don't get confused
@@ -297,7 +300,7 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 		w.Header().Add("x-rclone-jobid", fmt.Sprintf("%d", jobID))
 	}
 	if err != nil {
-		writeError(path, in, w, err, http.StatusInternalServerError)
+		writeError(path, inOrig, w, err, http.StatusInternalServerError)
 		return
 	}
 	if out == nil {
@@ -308,8 +311,8 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 	err = rc.WriteJSON(w, out)
 	if err != nil {
 		// can't return the error at this point - but have a go anyway
-		writeError(path, in, w, err, http.StatusInternalServerError)
-		fs.Errorf(nil, "rc: failed to write JSON output: %v", err)
+		writeError(path, inOrig, w, err, http.StatusInternalServerError)
+		fs.Errorf(nil, "rc: handlePost: failed to write JSON output: %v", err)
 	}
 }
 

fs/rc/rcserver/rcserver_test.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"os"
 	"regexp"
 	"strings"
 	"testing"
@@ -17,6 +18,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	_ "github.com/rclone/rclone/backend/local"
+	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/accounting"
 	"github.com/rclone/rclone/fs/rc"
 )
@@ -28,6 +30,21 @@ const (
 	remoteURL       = "[" + testFs + "]/" // initial URL path to fetch from that remote
 )
 
+func TestMain(m *testing.M) {
+	// Pretend to be rclone version if we have a version string parameter
+	if os.Args[len(os.Args)-1] == "version" {
+		fmt.Printf("rclone %s\n", fs.Version)
+		os.Exit(0)
+	}
+	// Pretend to error if we have an unknown command
+	if os.Args[len(os.Args)-1] == "unknown_command" {
+		fmt.Printf("rclone %s\n", fs.Version)
+		fmt.Fprintf(os.Stderr, "Unknown command\n")
+		os.Exit(1)
+	}
+	os.Exit(m.Run())
+}
+
 // Test the RC server runs and we can do HTTP fetches from it.
 // We'll do the majority of the testing with the httptest framework
 func TestRcServer(t *testing.T) {
@@ -456,6 +473,73 @@ func TestRC(t *testing.T) {
 	testServer(t, tests, &opt)
 }
 
+func TestRCWithAuth(t *testing.T) {
+	tests := []testRun{{
+		Name:        "core-command",
+		URL:         "core/command",
+		Method:      "POST",
+		Body:        `command=version`,
+		ContentType: "application/x-www-form-urlencoded",
+		Status:      http.StatusOK,
+		Expected: fmt.Sprintf(`{
+	"error": false,
+	"result": "rclone %s\n"
+}
+`, fs.Version),
+	}, {
+		Name:        "core-command-bad-returnType",
+		URL:         "core/command",
+		Method:      "POST",
+		Body:        `command=version&returnType=POTATO`,
+		ContentType: "application/x-www-form-urlencoded",
+		Status:      http.StatusInternalServerError,
+		Expected: `{
+	"error": "Unknown returnType \"POTATO\"",
+	"input": {
+		"command": "version",
+		"returnType": "POTATO"
+	},
+	"path": "core/command",
+	"status": 500
+}
+`,
+	}, {
+		Name:        "core-command-stream",
+		URL:         "core/command",
+		Method:      "POST",
+		Body:        `command=version&returnType=STREAM`,
+		ContentType: "application/x-www-form-urlencoded",
+		Status:      http.StatusOK,
+		Expected: fmt.Sprintf(`rclone %s
+{}
+`, fs.Version),
+	}, {
+		Name:        "core-command-stream-error",
+		URL:         "core/command",
+		Method:      "POST",
+		Body:        `command=unknown_command&returnType=STREAM`,
+		ContentType: "application/x-www-form-urlencoded",
+		Status:      http.StatusOK,
+		Expected: fmt.Sprintf(`rclone %s
+Unknown command
+{
+	"error": "exit status 1",
+	"input": {
+		"command": "unknown_command",
+		"returnType": "STREAM"
+	},
+	"path": "core/command",
+	"status": 500
+}
+`, fs.Version),
+	}}
+	opt := newTestOpt()
+	opt.Serve = true
+	opt.Files = testFs
+	opt.NoAuth = true
+	testServer(t, tests, &opt)
+}
+
 func TestMethods(t *testing.T) {
 	tests := []testRun{{
 		Name:     "options",