From 0693deea1cf158fb4e28f23c1d842c677e90ae78 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Sat, 10 Aug 2019 16:22:17 +0100
Subject: [PATCH] rc: fix unmarshalable http.AuthFn in options and put in test
 for marshalability

---
 cmd/serve/httplib/httplib.go |  2 +-
 fs/rc/config_test.go         | 40 ++++++++++++++++++++++++++++++------
 fs/rc/rcserver/rcserver.go   |  3 ++-
 3 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/cmd/serve/httplib/httplib.go b/cmd/serve/httplib/httplib.go
index 42d5392f2..6a6e3740f 100644
--- a/cmd/serve/httplib/httplib.go
+++ b/cmd/serve/httplib/httplib.go
@@ -97,7 +97,7 @@ type Options struct {
 	Realm              string        // realm for authentication
 	BasicUser          string        // single username for basic auth if not using Htpasswd
 	BasicPass          string        // password for BasicUser
-	Auth               AuthFn        // custom Auth (not set by command line flags)
+	Auth               AuthFn        `json:"-"` // custom Auth (not set by command line flags)
 }
 
 // AuthFn if used will be used to authenticate user, pass. If an error
diff --git a/fs/rc/config_test.go b/fs/rc/config_test.go
index 3cc1b3f5a..3ee4c27bc 100644
--- a/fs/rc/config_test.go
+++ b/fs/rc/config_test.go
@@ -2,16 +2,23 @@ package rc
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"testing"
 
 	"github.com/pkg/errors"
+	"github.com/rclone/rclone/cmd/serve/httplib"
+	"github.com/rclone/rclone/fs"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func clearOptionBlock() {
+func clearOptionBlock() func() {
+	oldOptionBlock := optionBlock
 	optionBlock = map[string]interface{}{}
+	return func() {
+		optionBlock = oldOptionBlock
+	}
 }
 
 var testOptions = struct {
@@ -23,7 +30,7 @@ var testOptions = struct {
 }
 
 func TestAddOption(t *testing.T) {
-	defer clearOptionBlock()
+	defer clearOptionBlock()()
 	assert.Equal(t, len(optionBlock), 0)
 	AddOption("potato", &testOptions)
 	assert.Equal(t, len(optionBlock), 1)
@@ -32,7 +39,7 @@ func TestAddOption(t *testing.T) {
 }
 
 func TestAddOptionReload(t *testing.T) {
-	defer clearOptionBlock()
+	defer clearOptionBlock()()
 	assert.Equal(t, len(optionBlock), 0)
 	reload := func() error { return nil }
 	AddOptionReload("potato", &testOptions, reload)
@@ -43,7 +50,7 @@ func TestAddOptionReload(t *testing.T) {
 }
 
 func TestOptionsBlocks(t *testing.T) {
-	defer clearOptionBlock()
+	defer clearOptionBlock()()
 	AddOption("potato", &testOptions)
 	call := Calls.Get("options/blocks")
 	require.NotNil(t, call)
@@ -55,7 +62,7 @@ func TestOptionsBlocks(t *testing.T) {
 }
 
 func TestOptionsGet(t *testing.T) {
-	defer clearOptionBlock()
+	defer clearOptionBlock()()
 	AddOption("potato", &testOptions)
 	call := Calls.Get("options/get")
 	require.NotNil(t, call)
@@ -66,8 +73,29 @@ func TestOptionsGet(t *testing.T) {
 	assert.Equal(t, Params{"potato": &testOptions}, out)
 }
 
+func TestOptionsGetMarshal(t *testing.T) {
+	defer clearOptionBlock()()
+
+	// Add some real options
+	AddOption("http", &httplib.DefaultOpt)
+	AddOption("main", fs.Config)
+	AddOption("rc", &DefaultOpt)
+
+	// get them
+	call := Calls.Get("options/get")
+	require.NotNil(t, call)
+	in := Params{}
+	out, err := call.Fn(context.Background(), in)
+	require.NoError(t, err)
+	require.NotNil(t, out)
+
+	// Check that they marshal
+	_, err = json.Marshal(out)
+	require.NoError(t, err)
+}
+
 func TestOptionsSet(t *testing.T) {
-	defer clearOptionBlock()
+	defer clearOptionBlock()()
 	var reloaded int
 	AddOptionReload("potato", &testOptions, func() error {
 		if reloaded > 0 {
diff --git a/fs/rc/rcserver/rcserver.go b/fs/rc/rcserver/rcserver.go
index 180942d54..646eaa649 100644
--- a/fs/rc/rcserver/rcserver.go
+++ b/fs/rc/rcserver/rcserver.go
@@ -229,7 +229,8 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 	fs.Debugf(nil, "rc: %q: reply %+v: %v", path, out, err)
 	err = rc.WriteJSON(w, out)
 	if err != nil {
-		// can't return the error at this point
+		// 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)
 	}
 }