From 0a69c3b3eb74ef5b21e15e983474abb7b54b38a1 Mon Sep 17 00:00:00 2001 From: Gabor Hosszu Date: Tue, 23 Aug 2016 13:43:42 +0200 Subject: [PATCH] Revert REST API to no base64 The REST/JSON API is more natural to use when taking strings, rather than when taking base64 encoded data. This change also restores the previous API behavior, which suffered compatibility breakage previously. JSON by default unmarshals []byte by using base64 encoding. However, we want to override this by using strings instead. We fix this by providing a custom UnmarshalJSON method for ChaincodeInput, which converts the string-based REST/JSON input to the []byte-based current ChaincodeInput structure. Change-Id: I1cfef1447ee5680d3c8bb2b36d3853badbd7ef13 Signed-off-by: Bradley Gorman Signed-off-by: Gabor Hosszu --- bddtests/steps/peer_basic_impl.py | 21 +++++++++++++-------- core/rest/rest_api.go | 7 ++++--- core/rest/rest_api_test.go | 25 ++++++++++++------------- peer/chaincode/common.go | 19 +++++++------------ protos/chaincode.pb.go | 2 ++ protos/chaincode.proto | 2 ++ protos/transaction.go | 22 ++++++++++++++++++++++ 7 files changed, 62 insertions(+), 36 deletions(-) diff --git a/bddtests/steps/peer_basic_impl.py b/bddtests/steps/peer_basic_impl.py index a9dffc286cc..21b906b81b2 100644 --- a/bddtests/steps/peer_basic_impl.py +++ b/bddtests/steps/peer_basic_impl.py @@ -240,7 +240,7 @@ def deployChainCodeToContainer(context, chaincode, containerName): def createChaincodeSpec(context, chaincode): chaincode = validateChaincodeDictionary(chaincode) - args = to_bytes(prepend(chaincode["constructor"], chaincode["args"])) + args = prepend(chaincode["constructor"], chaincode["args"]) # Create a ChaincodeSpec structure chaincodeSpec = { "type": getChaincodeTypeValue(chaincode["language"]), @@ -366,17 +366,18 @@ def invokeChaincode(context, devopsFunc, functionName, containerName, idGenAlg=N if 'table' in context: # There is ctor arguments args = context.table[0].cells - args = to_bytes(prepend(functionName, args)) + args = prepend(functionName, args) for idx, attr in enumerate(attributes): attributes[idx] = attr.strip() - context.chaincodeSpec['ctorMsg']['args'] = args context.chaincodeSpec['attributes'] = attributes #If idGenAlg is passed then, we still using the deprecated devops API because this parameter can't be passed in the new API. if idGenAlg != None: + context.chaincodeSpec['ctorMsg']['args'] = to_bytes(args) invokeUsingDevopsService(context, devopsFunc, functionName, containerName, idGenAlg) else: + context.chaincodeSpec['ctorMsg']['args'] = args invokeUsingChaincodeService(context, devopsFunc, functionName, containerName) def invokeUsingChaincodeService(context, devopsFunc, functionName, containerName): @@ -424,7 +425,7 @@ def invokeMasterChaincode(context, devopsFunc, chaincodeName, functionName, cont args = [] if 'table' in context: args = context.table[0].cells - args = to_bytes(prepend(functionName, args)) + args = prepend(functionName, args) typeGolang = 1 chaincodeSpec = { "type": typeGolang, @@ -646,7 +647,7 @@ def step_impl(context, chaincodeName, functionName): if 'table' in context: # There is ctor arguments args = context.table[0].cells - args = to_bytes(prepend(functionName, args)) + args = prepend(functionName, args) context.chaincodeSpec['ctorMsg']['args'] = args #context.table[0].cells if ('table' in context) else [] # Invoke the POST chaincodeOpPayload = createChaincodeOpPayload("query", context.chaincodeSpec) @@ -678,7 +679,7 @@ def query_common(context, chaincodeName, functionName, value, failOnError): containerDataList = bdd_test_util.getContainerDataValuesFromContext(context, aliases, lambda containerData: containerData) # Update the chaincodeSpec ctorMsg for invoke - context.chaincodeSpec['ctorMsg']['args'] = to_bytes([functionName, value]) + context.chaincodeSpec['ctorMsg']['args'] = [functionName, value] # Invoke the POST # Make deep copy of chaincodeSpec as we will be changing the SecurityContext per call. chaincodeOpPayload = createChaincodeOpPayload("query", copy.deepcopy(context.chaincodeSpec)) @@ -824,8 +825,12 @@ def to_bytes(strlist): def prepend(elem, l): if l is None or l == "": - return [elem] - return [elem] + l + tail = [] + else: + tail = l + if elem is None: + return tail + return [elem] + tail @given(u'I do nothing') def step_impl(context): diff --git a/core/rest/rest_api.go b/core/rest/rest_api.go index e6836764b6c..67331c9b199 100644 --- a/core/rest/rest_api.go +++ b/core/rest/rest_api.go @@ -1297,10 +1297,10 @@ func (s *ServerOpenchainREST) ProcessChaincode(rw web.ResponseWriter, req *web.R } // Extract the ChaincodeSpec from the params field - deploySpec := requestPayload.Params + ccSpec := requestPayload.Params // Process the chaincode deployment request and record the result - result = s.processChaincodeDeploy(deploySpec) + result = s.processChaincodeDeploy(ccSpec) } else { // @@ -1310,7 +1310,8 @@ func (s *ServerOpenchainREST) ProcessChaincode(rw web.ResponseWriter, req *web.R // Because chaincode invocation/query requests require a ChaincodeInvocationSpec // message instead of a ChaincodeSpec message, we must initialize it here // before proceeding. - invokequeryPayload := &pb.ChaincodeInvocationSpec{ChaincodeSpec: requestPayload.Params} + ccSpec := requestPayload.Params + invokequeryPayload := &pb.ChaincodeInvocationSpec{ChaincodeSpec: ccSpec} // Payload params field must contain a ChaincodeSpec message if invokequeryPayload.ChaincodeSpec == nil { diff --git a/core/rest/rest_api_test.go b/core/rest/rest_api_test.go index c6816f7b68b..be86b7ce16c 100644 --- a/core/rest/rest_api_test.go +++ b/core/rest/rest_api_test.go @@ -18,7 +18,6 @@ package rest import ( "bytes" - "encoding/base64" "encoding/json" "fmt" "io/ioutil" @@ -34,10 +33,10 @@ import ( "github.com/hyperledger/fabric/protos" ) -var failb64 string = base64.StdEncoding.EncodeToString([]byte("fail")) -var initb64 string = base64.StdEncoding.EncodeToString([]byte("Init")) -var change_ownerb64 string = base64.StdEncoding.EncodeToString([]byte("change_owner")) -var get_ownerb64 string = base64.StdEncoding.EncodeToString([]byte("get_owner")) +var fail_func string = "fail" +var init_func string = "Init" +var change_owner_func string = "change_owner" +var get_owner_func string = "get_owner" func performHTTPGet(t *testing.T, url string) []byte { response, err := http.Get(url) @@ -594,7 +593,7 @@ func TestServerOpenchainREST_API_Chaincode_Deploy(t *testing.T) { }, "ctorMsg": { "args": ["` + - initb64 + + init_func + `"] }, "secureContext": "myuser" @@ -621,7 +620,7 @@ func TestServerOpenchainREST_API_Chaincode_Deploy(t *testing.T) { }, "ctorMsg": { "args": ["` + - initb64 + + init_func + `"] } } @@ -644,7 +643,7 @@ func TestServerOpenchainREST_API_Chaincode_Deploy(t *testing.T) { }, "ctorMsg": { "args": ["` + - initb64 + + init_func + `"] }, "secureContext": "myuser" @@ -691,7 +690,7 @@ func TestServerOpenchainREST_API_Chaincode_Invoke(t *testing.T) { performHTTPPost(t, httpServer.URL+"/registrar", []byte(`{"enrollId":"myuser","enrollSecret":"password"}`)) // Test invoke with "fail" function - httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"invoke","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"args":["`+failb64+`"]},"secureContext":"myuser"}}`)) + httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"invoke","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"Function":"`+fail_func+`","args":[]},"secureContext":"myuser"}}`)) if httpResponse.StatusCode != http.StatusOK { t.Errorf("Expected an HTTP status code %#v but got %#v", http.StatusOK, httpResponse.StatusCode) } @@ -701,7 +700,7 @@ func TestServerOpenchainREST_API_Chaincode_Invoke(t *testing.T) { } // Test invoke with "change_owner" function - httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"invoke","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"args":["`+change_ownerb64+`"]},"secureContext":"myuser"}}`)) + httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"invoke","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"Function":"`+change_owner_func+`","args":[]},"secureContext":"myuser"}}`)) if httpResponse.StatusCode != http.StatusOK { t.Errorf("Expected an HTTP status code %#v but got %#v", http.StatusOK, httpResponse.StatusCode) } @@ -742,7 +741,7 @@ func TestServerOpenchainREST_API_Chaincode_Query(t *testing.T) { performHTTPPost(t, httpServer.URL+"/registrar", []byte(`{"enrollId":"myuser","enrollSecret":"password"}`)) // Test query with non-existing chaincode name - httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"query","params":{"type":1,"chaincodeID":{"name":"non-existing"},"ctorMsg":{"args":["`+initb64+`"]},"secureContext":"myuser"}}`)) + httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"query","params":{"type":1,"chaincodeID":{"name":"non-existing"},"ctorMsg":{"Function":"`+init_func+`","args":[]},"secureContext":"myuser"}}`)) if httpResponse.StatusCode != http.StatusOK { t.Errorf("Expected an HTTP status code %#v but got %#v", http.StatusOK, httpResponse.StatusCode) } @@ -752,7 +751,7 @@ func TestServerOpenchainREST_API_Chaincode_Query(t *testing.T) { } // Test query with fail function - httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"query","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"args":["`+failb64+`"]},"secureContext":"myuser"}}`)) + httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"query","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"Function":"`+fail_func+`","args":[]},"secureContext":"myuser"}}`)) if httpResponse.StatusCode != http.StatusOK { t.Errorf("Expected an HTTP status code %#v but got %#v", http.StatusOK, httpResponse.StatusCode) } @@ -765,7 +764,7 @@ func TestServerOpenchainREST_API_Chaincode_Query(t *testing.T) { } // Test query with get_owner function - httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"query","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"args":["`+get_ownerb64+`"]},"secureContext":"myuser"}}`)) + httpResponse, body = performHTTPPost(t, httpServer.URL+"/chaincode", []byte(`{"jsonrpc":"2.0","ID":123,"method":"query","params":{"type":1,"chaincodeID":{"name":"dummy"},"ctorMsg":{"Function":"`+get_owner_func+`","args":[]},"secureContext":"myuser"}}`)) if httpResponse.StatusCode != http.StatusOK { t.Errorf("Expected an HTTP status code %#v but got %#v", http.StatusOK, httpResponse.StatusCode) } diff --git a/peer/chaincode/common.go b/peer/chaincode/common.go index 4ea50465afe..cf0f6f8d254 100755 --- a/peer/chaincode/common.go +++ b/peer/chaincode/common.go @@ -25,7 +25,6 @@ import ( "strings" "github.com/hyperledger/fabric/core" - u "github.com/hyperledger/fabric/core/util" "github.com/hyperledger/fabric/peer/common" "github.com/hyperledger/fabric/peer/util" pb "github.com/hyperledger/fabric/protos" @@ -34,10 +33,6 @@ import ( "golang.org/x/net/context" ) -type container struct { - Args []string -} - func getChaincodeSpecification(cmd *cobra.Command) (*pb.ChaincodeSpec, error) { spec := &pb.ChaincodeSpec{} if err := checkChaincodeCmdParams(cmd); err != nil { @@ -45,11 +40,10 @@ func getChaincodeSpecification(cmd *cobra.Command) (*pb.ChaincodeSpec, error) { } // Build the spec - inputc := container{} - if err := json.Unmarshal([]byte(chaincodeCtorJSON), &inputc); err != nil { + input := &pb.ChaincodeInput{} + if err := json.Unmarshal([]byte(chaincodeCtorJSON), &input); err != nil { return spec, fmt.Errorf("Chaincode argument error: %s", err) } - input := &pb.ChaincodeInput{Args: u.ToChaincodeArgs(inputc.Args...)} var attributes []string if err := json.Unmarshal([]byte(chaincodeAttributesJSON), &attributes); err != nil { @@ -193,18 +187,19 @@ func checkChaincodeCmdParams(cmd *cobra.Command) error { return fmt.Errorf("Chaincode argument error: %s", err) } m := f.(map[string]interface{}) - if len(m) != 1 { - return fmt.Errorf("Non-empty JSON chaincode parameters must contain exactly 1 key: 'Args'") + if len(m) != 2 { + return fmt.Errorf("Non-empty JSON chaincode parameters must contain exactly 2 keys: 'Function' and 'Args'") } for k := range m { switch strings.ToLower(k) { case "args": + case "function": default: - return fmt.Errorf("Illegal chaincode key '%s' - must be only 'Args'", k) + return fmt.Errorf("Illegal chaincode key '%s' - must be 'Function' or 'Args'", k) } } } else { - return errors.New("Empty JSON chaincode parameters must contain exactly 1 key: 'Args'") + return errors.New("Empty JSON chaincode parameters must contain exactly 2 keys: 'Function' and 'Args'") } if chaincodeAttributesJSON != "[]" { diff --git a/protos/chaincode.pb.go b/protos/chaincode.pb.go index e8a1cf1c1cc..7986a792bad 100644 --- a/protos/chaincode.pb.go +++ b/protos/chaincode.pb.go @@ -186,6 +186,8 @@ func (m *ChaincodeID) String() string { return proto.CompactTextString(m) } func (*ChaincodeID) ProtoMessage() {} // Carries the chaincode function and its arguments. +// UnmarshalJSON in transaction.go converts the string-based REST/JSON input to +// the []byte-based current ChaincodeInput structure. type ChaincodeInput struct { Args [][]byte `protobuf:"bytes,1,rep,name=args,proto3" json:"args,omitempty"` } diff --git a/protos/chaincode.proto b/protos/chaincode.proto index 495817d56bd..20c1e949bdb 100644 --- a/protos/chaincode.proto +++ b/protos/chaincode.proto @@ -46,6 +46,8 @@ message ChaincodeID { } // Carries the chaincode function and its arguments. +// UnmarshalJSON in transaction.go converts the string-based REST/JSON input to +// the []byte-based current ChaincodeInput structure. message ChaincodeInput { repeated bytes args = 1; } diff --git a/protos/transaction.go b/protos/transaction.go index 49360800b79..2d627734992 100644 --- a/protos/transaction.go +++ b/protos/transaction.go @@ -17,6 +17,7 @@ limitations under the License. package protos import ( + "encoding/json" "fmt" "github.com/golang/protobuf/proto" @@ -111,3 +112,24 @@ func NewChaincodeExecute(chaincodeInvocationSpec *ChaincodeInvocationSpec, uuid transaction.Payload = data return transaction, nil } + +type strArgs struct { + Function string + Args []string +} + +// UnmarshalJSON converts the string-based REST/JSON input to +// the []byte-based current ChaincodeInput structure. +func (c *ChaincodeInput) UnmarshalJSON(b []byte) error { + sa := &strArgs{} + err := json.Unmarshal(b, sa) + if err != nil { + return err + } + allArgs := sa.Args + if sa.Function != "" { + allArgs = append([]string{sa.Function}, sa.Args...) + } + c.Args = util.ToChaincodeArgs(allArgs...) + return nil +}