[#618] Jail.Call to return valid JSON string based results (#619)

e2e/jail/jail_rpc_test.go

@@ -229,7 +229,7 @@ func (s *JailRPCTestSuite) TestJailVMPersistence() {
 			`["ping"]`,
 			`{"pong": "Ping1", "amount": 0.42}`,
 			func(response string) error {
-				expectedResponse := `{"result": "Ping1"}`
+				expectedResponse := `{"result":"Ping1"}`
 				if response != expectedResponse {
 					return fmt.Errorf("unexpected response, expected: %v, got: %v", expectedResponse, response)
 				}
@@ -240,7 +240,7 @@ func (s *JailRPCTestSuite) TestJailVMPersistence() {
 			`["ping"]`,
 			`{"pong": "Ping2", "amount": 0.42}`,
 			func(response string) error {
-				expectedResponse := `{"result": "Ping2"}`
+				expectedResponse := `{"result":"Ping2"}`
 				if response != expectedResponse {
 					return fmt.Errorf("unexpected response, expected: %v, got: %v", expectedResponse, response)
 				}

e2e/jail/jail_test.go

@@ -81,7 +81,7 @@ func (s *JailTestSuite) TestInitWithBaseJS() {
 
 	// make sure that Call succeeds even w/o running node
 	response = s.Jail.Call(testChatID, `["commands", "testCommand"]`, `{"val": 12}`)
-	expectedResponse := `{"result": 144}`
+	expectedResponse := `{"result":144}`
 	s.Equal(expectedResponse, response)
 }
 
@@ -93,13 +93,13 @@ func (s *JailTestSuite) TestCreateAndInitCell() {
 
 	// If any custom JS provided -- the result of the last op is returned
 	response = s.Jail.CreateAndInitCell("newChat2", "var a = 2", "a")
-	expectedResponse = `{"result": 2}`
+	expectedResponse = `{"result":2}`
 	s.Equal(expectedResponse, response)
 
 	// Reinitialization preserves the JS environment, so the 'test' variable exists
 	// even though we didn't initialize it here (in the second room).
 	response = s.Jail.CreateAndInitCell("newChat2", "a")
-	expectedResponse = `{"result": 2}`
+	expectedResponse = `{"result":2}`
 	s.Equal(expectedResponse, response)
 
 	// But this variable doesn't leak into other rooms.
@@ -122,8 +122,22 @@ func (s *JailTestSuite) TestFunctionCall() {
 	s.Equal(expectedError, response)
 
 	// call extraFunc()
-	response = s.Jail.Call(testChatID, `["commands", "testCommand"]`, `{"val": 12}`)
-	expectedResponse := `{"result": 144}`
+	response = s.Jail.Call(testChatID, `["commands", "testCommand"]`, `{"val":12}`)
+	expectedResponse := `{"result":144}`
+	s.Equal(expectedResponse, response)
+}
+
+func (s *JailTestSuite) TestFunctionCallTrue() {
+	// load Status JS and add test command to it
+	statusJS := baseStatusJSCode + `;
+	_status_catalog.commands["testCommandTrue"] = function (params) {
+		return true;
+	};`
+	s.Jail.CreateAndInitCell(testChatID, statusJS)
+
+	// call extraFunc()
+	response := s.Jail.Call(testChatID, `["commands", "testCommandTrue"]`, `{"val":12}`)
+	expectedResponse := `{"result":true}`
 	s.Equal(expectedResponse, response)
 }
 
@@ -220,8 +234,8 @@ func (s *JailTestSuite) TestSendSyncResponseOrder() {
 		wg.Add(1)
 		go func(i int) {
 			defer wg.Done()
-			res := s.Jail.Call(testChatID, `["commands", "testCommand"]`, fmt.Sprintf(`{"val": %d}`, i))
-			if !strings.Contains(string(res), fmt.Sprintf("result\": %d", i*i)) {
+			res := s.Jail.Call(testChatID, `["commands", "testCommand"]`, fmt.Sprintf(`{"val":%d}`, i))
+			if !strings.Contains(string(res), fmt.Sprintf("result\":%d", i*i)) {
 				errCh <- fmt.Errorf("result should be '%d', got %s", i*i, res)
 			}
 		}(i)

geth/jail/jail.go

@@ -157,7 +157,8 @@ func (j *Jail) createAndInitCell(chatID string, extraCode ...string) (*Cell, str
 		if err != nil {
 			return nil, "", err
 		}
-		response = newJailResultResponse(result)
+
+		response = newJailResultResponse(formatOttoValue(result))
 	}
 
 	return cell, response, nil
@@ -295,9 +296,26 @@ func newJailErrorResponse(err error) string {
 	return string(rawResponse)
 }
 
+// formatOttoValue : formats an otto value string to be processed as valid
+// javascript code
+func formatOttoValue(result otto.Value) otto.Value {
+	val := result.String()
+	if result.IsString() {
+		if val != "undefined" {
+			val = fmt.Sprintf(`"%s"`, strings.Replace(val, `"`, `\"`, -1))
+			result, _ = otto.ToValue(val)
+		}
+	}
+	return result
+}
+
 // newJailResultResponse returns a string that is a valid JavaScript code.
 // Marshaling is not required as result.String() produces a string
 // that is a valid JavaScript code.
-func newJailResultResponse(result otto.Value) string {
-	return `{"result": ` + result.String() + `}`
+func newJailResultResponse(value otto.Value) string {
+	res := value.String()
+	if res == "undefined" {
+		res = "null"
+	}
+	return `{"result":` + res + `}`
 }

geth/jail/jail_test.go

@@ -1,6 +1,7 @@
 package jail
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/robertkrimen/otto"
@@ -98,7 +99,7 @@ func (s *JailTestSuite) TestJailCall() {
 	result := s.Jail.Call("cell1", `["prop1", "prop2"]`, `arg1`)
 	s.Equal(`["prop1", "prop2"]`, <-propsc)
 	s.Equal(`arg1`, <-argsc)
-	s.Equal(`{"result": undefined}`, result)
+	s.Equal(`{"result":null}`, result)
 }
 
 func (s *JailTestSuite) TestCreateAndInitCell() {
@@ -110,7 +111,7 @@ func (s *JailTestSuite) TestCreateAndInitCell() {
 	)
 	s.NoError(err)
 	s.NotNil(cell)
-	s.Equal(`{"result": true}`, result)
+	s.Equal(`{"result":true}`, result)
 
 	value, err := cell.Get("testCreateAndInitCell1")
 	s.NoError(err)
@@ -122,13 +123,41 @@ func (s *JailTestSuite) TestCreateAndInitCell() {
 }
 
 func (s *JailTestSuite) TestPublicCreateAndInitCell() {
-	response := s.Jail.CreateAndInitCell("cell1")
-	s.Equal(EmptyResponse, response)
+	var createAndInitTests = []struct {
+		chatID      string
+		input       []string
+		expectation string
+	}{
+		{"cell1", []string{}, EmptyResponse},
+		{"cell1", []string{"var a = 2", "a"}, `{"result":2}`},
+		{"cell1", []string{`var a = "hello"`, "a"}, `{"result":"hello"}`},
+		{"cell1", []string{`var b = "2"; var a = b * b`, "a"}, `{"result":4}`},
+	}
+	for _, v := range createAndInitTests {
+		response := s.Jail.CreateAndInitCell(v.chatID, v.input...)
+		s.Equal(v.expectation, response)
+	}
 }
 
-func (s *JailTestSuite) TestPublicCreateAndInitCellWithJS() {
-	response := s.Jail.CreateAndInitCell("cell1", "var a = 2", "a")
-	s.Equal(`{"result": 2}`, response)
+func (s *JailTestSuite) TestNewJailResultResponseReturnsValidJson() {
+	var newJailResultResponseTests = []interface{}{
+		`Double quoted "success" response`,
+		float64(1),
+		true,
+	}
+	for _, input := range newJailResultResponseTests {
+		v, err := otto.ToValue(input)
+		s.NoError(err)
+
+		output := newJailResultResponse(formatOttoValue(v))
+		var response struct {
+			Result interface{} `json:"result"`
+		}
+		err = json.Unmarshal([]byte(output), &response)
+
+		s.NoError(err)
+		s.Equal(input, response.Result)
+	}
 }
 
 func (s *JailTestSuite) TestPublicCreateAndInitCellConsecutive() {

lib/utils.go

@@ -1310,7 +1310,7 @@ func testJailInit(t *testing.T) bool {
 
 	// Cell initialization return the result of the last JS operation provided to it.
 	response := CreateAndInitCell(chatID, C.CString(`var extraFunc = function (x) { return x * x; }; extraFunc(2);`))
-	require.Equal(t, `{"result": 4}`, C.GoString(response), "Unexpected response from jail.CreateAndInitCell()")
+	require.Equal(t, `{"result":4}`, C.GoString(response), "Unexpected response from jail.CreateAndInitCell()")
 
 	// Commands from the jail initialization are available in any of the created cells.
 	response = ExecuteJS(chatID, C.CString(`JSON.stringify({ result: _status_catalog });`))
@@ -1332,11 +1332,11 @@ func testJailParseDeprecated(t *testing.T) bool {
 	chatID := C.CString("CHAT_ID_PARSE_TEST")
 
 	response := Parse(chatID, C.CString(extraCode))
-	require.Equal(t, `{"result": 4}`, C.GoString(response))
+	require.Equal(t, `{"result":4}`, C.GoString(response))
 
 	// cell already exists but Parse should not complain
 	response = Parse(chatID, C.CString(extraCode))
-	require.Equal(t, `{"result": 4}`, C.GoString(response))
+	require.Equal(t, `{"result":4}`, C.GoString(response))
 
 	// test extraCode
 	response = ExecuteJS(chatID, C.CString(`extraFunc(10)`))
@@ -1367,7 +1367,7 @@ func testJailFunctionCall(t *testing.T) bool {
 	// call extraFunc()
 	rawResponse = Call(C.CString("CHAT_ID_CALL_TEST"), C.CString(`["commands", "testCommand"]`), C.CString(`{"val": 12}`))
 	parsedResponse = C.GoString(rawResponse)
-	expectedResponse := `{"result": 144}`
+	expectedResponse := `{"result":144}`
 	if parsedResponse != expectedResponse {
 		t.Errorf("expected response is not returned: expected %s, got %s", expectedResponse, parsedResponse)
 		return false