From 41de1cb723d2eff921da8911e4cf893d8907c178 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Wed, 1 Jul 2015 08:23:17 +0200 Subject: [PATCH 1/9] added pipelining support --- rpc/codec/json.go | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/rpc/codec/json.go b/rpc/codec/json.go index 0b1a90562..5d60104f7 100644 --- a/rpc/codec/json.go +++ b/rpc/codec/json.go @@ -18,19 +18,19 @@ const ( // Json serialization support type JsonCodec struct { c net.Conn + d *json.Decoder } // Create new JSON coder instance func NewJsonCoder(conn net.Conn) ApiCoder { return &JsonCodec{ c: conn, + d: json.NewDecoder(conn), } } // Serialize obj to JSON and write it to conn func (self *JsonCodec) ReadRequest() (requests []*shared.Request, isBatch bool, err error) { - bytesInBuffer := 0 - buf := make([]byte, MAX_REQUEST_SIZE) deadline := time.Now().Add(READ_TIMEOUT * time.Second) if err := self.c.SetDeadline(deadline); err != nil { @@ -38,31 +38,36 @@ func (self *JsonCodec) ReadRequest() (requests []*shared.Request, isBatch bool, } for { - n, err := self.c.Read(buf[bytesInBuffer:]) - if err != nil { - self.c.Close() - return nil, false, err - } - - bytesInBuffer += n - + var err error singleRequest := shared.Request{} - err = json.Unmarshal(buf[:bytesInBuffer], &singleRequest) - if err == nil { + if err = self.d.Decode(&singleRequest); err == nil { requests := make([]*shared.Request, 1) requests[0] = &singleRequest return requests, false, nil } + fmt.Printf("err %T %v\n", err) + + if opErr, ok := err.(*net.OpError); ok { + if opErr.Timeout() { + break + } + } + requests = make([]*shared.Request, 0) - err = json.Unmarshal(buf[:bytesInBuffer], &requests) - if err == nil { + if err = self.d.Decode(&requests); err == nil { return requests, true, nil } + + if opErr, ok := err.(*net.OpError); ok { + if opErr.Timeout() { + break + } + } } self.c.Close() // timeout - return nil, false, fmt.Errorf("Unable to read response") + return nil, false, fmt.Errorf("Timeout reading request") } func (self *JsonCodec) ReadResponse() (interface{}, error) { From c2590af7fda0c224cab5c0ebcb6973d6da447055 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Thu, 2 Jul 2015 15:26:55 +0200 Subject: [PATCH 2/9] prevent discarding requests when parsing fails --- rpc/codec/json.go | 122 ++++++++++++++++++++++------ rpc/codec/json_test.go | 177 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 273 insertions(+), 26 deletions(-) create mode 100644 rpc/codec/json_test.go diff --git a/rpc/codec/json.go b/rpc/codec/json.go index 5d60104f7..b5ef94380 100644 --- a/rpc/codec/json.go +++ b/rpc/codec/json.go @@ -10,64 +10,134 @@ import ( ) const ( - READ_TIMEOUT = 15 // read timeout in seconds + READ_TIMEOUT = 60 // in seconds MAX_REQUEST_SIZE = 1024 * 1024 MAX_RESPONSE_SIZE = 1024 * 1024 ) +var ( + // No new requests in buffer + EmptyRequestQueueError = fmt.Errorf("No incoming requests") + // Next request in buffer isn't yet complete + IncompleteRequestError = fmt.Errorf("Request incomplete") +) + // Json serialization support type JsonCodec struct { - c net.Conn - d *json.Decoder + c net.Conn + reqBuffer []byte + bytesInReqBuffer int + reqLastPos int } // Create new JSON coder instance func NewJsonCoder(conn net.Conn) ApiCoder { return &JsonCodec{ - c: conn, - d: json.NewDecoder(conn), + c: conn, + reqBuffer: make([]byte, MAX_REQUEST_SIZE), + bytesInReqBuffer: 0, + reqLastPos: 0, + } +} + +// Indication if the next request in the buffer is a batch request +func (self *JsonCodec) isNextBatchReq() (bool, error) { + for i := 0; i < self.bytesInReqBuffer; i++ { + switch self.reqBuffer[i] { + case 0x20, 0x09, 0x0a, 0x0d: // allow leading whitespace (JSON whitespace RFC4627) + continue + case 0x7b: // single req + return false, nil + case 0x5b: // batch req + return true, nil + default: + return false, &json.InvalidUnmarshalError{} + } + } + + return false, EmptyRequestQueueError +} + +// remove parsed request from buffer +func (self *JsonCodec) resetReqbuffer(pos int) { + copy(self.reqBuffer, self.reqBuffer[pos:self.bytesInReqBuffer]) + self.reqLastPos = 0 + self.bytesInReqBuffer -= pos +} + +// parse request in buffer +func (self *JsonCodec) nextRequest() (requests []*shared.Request, isBatch bool, err error) { + if isBatch, err := self.isNextBatchReq(); err == nil { + if isBatch { + requests = make([]*shared.Request, 0) + for ; self.reqLastPos <= self.bytesInReqBuffer; self.reqLastPos++ { + if err = json.Unmarshal(self.reqBuffer[:self.reqLastPos], &requests); err == nil { + self.resetReqbuffer(self.reqLastPos) + return requests, true, nil + } + } + return nil, true, IncompleteRequestError + } else { + request := shared.Request{} + for ; self.reqLastPos <= self.bytesInReqBuffer; self.reqLastPos++ { + if err = json.Unmarshal(self.reqBuffer[:self.reqLastPos], &request); err == nil { + requests := make([]*shared.Request, 1) + requests[0] = &request + self.resetReqbuffer(self.reqLastPos) + return requests, false, nil + } + } + return nil, true, IncompleteRequestError + } + } else { + return nil, false, err } } // Serialize obj to JSON and write it to conn func (self *JsonCodec) ReadRequest() (requests []*shared.Request, isBatch bool, err error) { + if self.bytesInReqBuffer != 0 { + req, batch, err := self.nextRequest() + if err == nil { + return req, batch, err + } + if err != IncompleteRequestError { + return nil, false, err + } + } + + // no/incomplete request in buffer -> read more data first deadline := time.Now().Add(READ_TIMEOUT * time.Second) if err := self.c.SetDeadline(deadline); err != nil { return nil, false, err } + var retErr error for { - var err error - singleRequest := shared.Request{} - if err = self.d.Decode(&singleRequest); err == nil { - requests := make([]*shared.Request, 1) - requests[0] = &singleRequest - return requests, false, nil + n, err := self.c.Read(self.reqBuffer[self.bytesInReqBuffer:]) + if err != nil { + retErr = err + break } - fmt.Printf("err %T %v\n", err) + self.bytesInReqBuffer += n - if opErr, ok := err.(*net.OpError); ok { - if opErr.Timeout() { - break - } + requests, isBatch, err := self.nextRequest() + if err == nil { + return requests, isBatch, nil } - requests = make([]*shared.Request, 0) - if err = self.d.Decode(&requests); err == nil { - return requests, true, nil + if err == IncompleteRequestError || err == EmptyRequestQueueError { + continue // need more data } - if opErr, ok := err.(*net.OpError); ok { - if opErr.Timeout() { - break - } - } + retErr = err + break } - self.c.Close() // timeout - return nil, false, fmt.Errorf("Timeout reading request") + self.c.Close() + return nil, false, retErr } func (self *JsonCodec) ReadResponse() (interface{}, error) { diff --git a/rpc/codec/json_test.go b/rpc/codec/json_test.go new file mode 100644 index 000000000..60cac05f7 --- /dev/null +++ b/rpc/codec/json_test.go @@ -0,0 +1,177 @@ +package codec + +import ( + "bytes" + "io" + "net" + "testing" + "time" +) + +type jsonTestConn struct { + buffer *bytes.Buffer +} + +func newJsonTestConn(data []byte) *jsonTestConn { + return &jsonTestConn{ + buffer: bytes.NewBuffer(data), + } +} + +func (self *jsonTestConn) Read(p []byte) (n int, err error) { + return self.buffer.Read(p) +} + +func (self *jsonTestConn) Write(p []byte) (n int, err error) { + return self.buffer.Write(p) +} + +func (self *jsonTestConn) Close() error { + // not implemented + return nil +} + +func (self *jsonTestConn) LocalAddr() net.Addr { + // not implemented + return nil +} + +func (self *jsonTestConn) RemoteAddr() net.Addr { + // not implemented + return nil +} + +func (self *jsonTestConn) SetDeadline(t time.Time) error { + return nil +} + +func (self *jsonTestConn) SetReadDeadline(t time.Time) error { + return nil +} + +func (self *jsonTestConn) SetWriteDeadline(t time.Time) error { + return nil +} + +func TestJsonDecoderWithValidRequest(t *testing.T) { + reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","params":[],"id":64}`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != nil { + t.Errorf("Read valid request failed - %v", err) + } + + if len(requests) != 1 { + t.Errorf("Expected to get a single request but got %d", len(requests)) + } + + if batch { + t.Errorf("Got batch indication while expecting single request") + } + + if requests[0].Id != float64(64) { + t.Errorf("Expected req.Id == 64 but got %v", requests[0].Id) + } + + if requests[0].Method != "modules" { + t.Errorf("Expected req.Method == 'modules' got '%s'", requests[0].Method) + } +} + +func TestJsonDecoderWithValidBatchRequest(t *testing.T) { + reqdata := []byte(`[{"jsonrpc":"2.0","method":"modules","params":[],"id":64}, + {"jsonrpc":"2.0","method":"modules","params":[],"id":64}]`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != nil { + t.Errorf("Read valid batch request failed - %v", err) + } + + if len(requests) != 2 { + t.Errorf("Expected to get two requests but got %d", len(requests)) + } + + if !batch { + t.Errorf("Got no batch indication while expecting batch request") + } + + for i := 0; i < len(requests); i++ { + if requests[i].Id != float64(64) { + t.Errorf("Expected req.Id == 64 but got %v", requests[i].Id) + } + + if requests[i].Method != "modules" { + t.Errorf("Expected req.Method == 'modules' got '%s'", requests[i].Method) + } + } +} + +func TestJsonDecoderWithIncompleteMessage(t *testing.T) { + reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","pa`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != io.EOF { + t.Errorf("Expected to read an incomplete request err but got %v", err) + } + + // remaining message + decoder.Write([]byte(`rams":[],"id":64}`)) + requests, batch, err = jsonDecoder.ReadRequest() + + if err != nil { + t.Errorf("Read valid request failed - %v", err) + } + + if len(requests) != 1 { + t.Errorf("Expected to get a single request but got %d", len(requests)) + } + + if batch { + t.Errorf("Got batch indication while expecting single request") + } + + if requests[0].Id != float64(64) { + t.Errorf("Expected req.Id == 64 but got %v", requests[0].Id) + } + + if requests[0].Method != "modules" { + t.Errorf("Expected req.Method == 'modules' got '%s'", requests[0].Method) + } +} + +func TestJsonDecoderWithInvalidIncompleteMessage(t *testing.T) { + reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","pa`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != io.EOF { + t.Errorf("Expected to read an incomplete request err but got %v", err) + } + + // remaining message + decoder.Write([]byte(`rams":[],"id:64"}`)) + requests, batch, err = jsonDecoder.ReadRequest() + + if err == nil { + t.Errorf("Expected an error but got nil") + } + + if len(requests) != 0 { + t.Errorf("Expected to get no requests but got %d", len(requests)) + } + + if batch { + t.Errorf("Got batch indication while expecting non batch") + } +} From 89525fcb4e6dcd180ce231e2addb739bf0f9dbc5 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Thu, 2 Jul 2015 17:20:58 +0200 Subject: [PATCH 3/9] ipcpath issue fix --- cmd/utils/flags.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 0d59980ec..f9458f346 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -438,17 +438,17 @@ func MakeAccountManager(ctx *cli.Context) *accounts.Manager { func IpcSocketPath(ctx *cli.Context) (ipcpath string) { if common.IsWindows() { ipcpath = common.DefaultIpcPath() - if ipcpath != ctx.GlobalString(IPCPathFlag.Name) { + if ctx.GlobalIsSet(IPCPathFlag.Name) { ipcpath = ctx.GlobalString(IPCPathFlag.Name) } } else { ipcpath = common.DefaultIpcPath() - if ctx.GlobalString(IPCPathFlag.Name) != common.DefaultIpcPath() { - ipcpath = ctx.GlobalString(IPCPathFlag.Name) - } else if ctx.GlobalString(DataDirFlag.Name) != "" && - ctx.GlobalString(DataDirFlag.Name) != common.DefaultDataDir() { + if ctx.GlobalIsSet(DataDirFlag.Name) { ipcpath = filepath.Join(ctx.GlobalString(DataDirFlag.Name), "geth.ipc") } + if ctx.GlobalIsSet(IPCPathFlag.Name) { + ipcpath = ctx.GlobalString(IPCPathFlag.Name) + } } return From effe9cc2cf7203634b9b418e1e9309911d0f2b00 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Wed, 1 Jul 2015 08:23:17 +0200 Subject: [PATCH 4/9] added pipelining support --- rpc/codec/json.go | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/rpc/codec/json.go b/rpc/codec/json.go index 0b1a90562..5d60104f7 100644 --- a/rpc/codec/json.go +++ b/rpc/codec/json.go @@ -18,19 +18,19 @@ const ( // Json serialization support type JsonCodec struct { c net.Conn + d *json.Decoder } // Create new JSON coder instance func NewJsonCoder(conn net.Conn) ApiCoder { return &JsonCodec{ c: conn, + d: json.NewDecoder(conn), } } // Serialize obj to JSON and write it to conn func (self *JsonCodec) ReadRequest() (requests []*shared.Request, isBatch bool, err error) { - bytesInBuffer := 0 - buf := make([]byte, MAX_REQUEST_SIZE) deadline := time.Now().Add(READ_TIMEOUT * time.Second) if err := self.c.SetDeadline(deadline); err != nil { @@ -38,31 +38,36 @@ func (self *JsonCodec) ReadRequest() (requests []*shared.Request, isBatch bool, } for { - n, err := self.c.Read(buf[bytesInBuffer:]) - if err != nil { - self.c.Close() - return nil, false, err - } - - bytesInBuffer += n - + var err error singleRequest := shared.Request{} - err = json.Unmarshal(buf[:bytesInBuffer], &singleRequest) - if err == nil { + if err = self.d.Decode(&singleRequest); err == nil { requests := make([]*shared.Request, 1) requests[0] = &singleRequest return requests, false, nil } + fmt.Printf("err %T %v\n", err) + + if opErr, ok := err.(*net.OpError); ok { + if opErr.Timeout() { + break + } + } + requests = make([]*shared.Request, 0) - err = json.Unmarshal(buf[:bytesInBuffer], &requests) - if err == nil { + if err = self.d.Decode(&requests); err == nil { return requests, true, nil } + + if opErr, ok := err.(*net.OpError); ok { + if opErr.Timeout() { + break + } + } } self.c.Close() // timeout - return nil, false, fmt.Errorf("Unable to read response") + return nil, false, fmt.Errorf("Timeout reading request") } func (self *JsonCodec) ReadResponse() (interface{}, error) { From 6be527dd52f0b40da87a6e2dc372fba5a08dd33a Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Thu, 2 Jul 2015 15:26:55 +0200 Subject: [PATCH 5/9] prevent discarding requests when parsing fails --- rpc/codec/json.go | 122 ++++++++++++++++++++++------ rpc/codec/json_test.go | 177 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 273 insertions(+), 26 deletions(-) create mode 100644 rpc/codec/json_test.go diff --git a/rpc/codec/json.go b/rpc/codec/json.go index 5d60104f7..b5ef94380 100644 --- a/rpc/codec/json.go +++ b/rpc/codec/json.go @@ -10,64 +10,134 @@ import ( ) const ( - READ_TIMEOUT = 15 // read timeout in seconds + READ_TIMEOUT = 60 // in seconds MAX_REQUEST_SIZE = 1024 * 1024 MAX_RESPONSE_SIZE = 1024 * 1024 ) +var ( + // No new requests in buffer + EmptyRequestQueueError = fmt.Errorf("No incoming requests") + // Next request in buffer isn't yet complete + IncompleteRequestError = fmt.Errorf("Request incomplete") +) + // Json serialization support type JsonCodec struct { - c net.Conn - d *json.Decoder + c net.Conn + reqBuffer []byte + bytesInReqBuffer int + reqLastPos int } // Create new JSON coder instance func NewJsonCoder(conn net.Conn) ApiCoder { return &JsonCodec{ - c: conn, - d: json.NewDecoder(conn), + c: conn, + reqBuffer: make([]byte, MAX_REQUEST_SIZE), + bytesInReqBuffer: 0, + reqLastPos: 0, + } +} + +// Indication if the next request in the buffer is a batch request +func (self *JsonCodec) isNextBatchReq() (bool, error) { + for i := 0; i < self.bytesInReqBuffer; i++ { + switch self.reqBuffer[i] { + case 0x20, 0x09, 0x0a, 0x0d: // allow leading whitespace (JSON whitespace RFC4627) + continue + case 0x7b: // single req + return false, nil + case 0x5b: // batch req + return true, nil + default: + return false, &json.InvalidUnmarshalError{} + } + } + + return false, EmptyRequestQueueError +} + +// remove parsed request from buffer +func (self *JsonCodec) resetReqbuffer(pos int) { + copy(self.reqBuffer, self.reqBuffer[pos:self.bytesInReqBuffer]) + self.reqLastPos = 0 + self.bytesInReqBuffer -= pos +} + +// parse request in buffer +func (self *JsonCodec) nextRequest() (requests []*shared.Request, isBatch bool, err error) { + if isBatch, err := self.isNextBatchReq(); err == nil { + if isBatch { + requests = make([]*shared.Request, 0) + for ; self.reqLastPos <= self.bytesInReqBuffer; self.reqLastPos++ { + if err = json.Unmarshal(self.reqBuffer[:self.reqLastPos], &requests); err == nil { + self.resetReqbuffer(self.reqLastPos) + return requests, true, nil + } + } + return nil, true, IncompleteRequestError + } else { + request := shared.Request{} + for ; self.reqLastPos <= self.bytesInReqBuffer; self.reqLastPos++ { + if err = json.Unmarshal(self.reqBuffer[:self.reqLastPos], &request); err == nil { + requests := make([]*shared.Request, 1) + requests[0] = &request + self.resetReqbuffer(self.reqLastPos) + return requests, false, nil + } + } + return nil, true, IncompleteRequestError + } + } else { + return nil, false, err } } // Serialize obj to JSON and write it to conn func (self *JsonCodec) ReadRequest() (requests []*shared.Request, isBatch bool, err error) { + if self.bytesInReqBuffer != 0 { + req, batch, err := self.nextRequest() + if err == nil { + return req, batch, err + } + if err != IncompleteRequestError { + return nil, false, err + } + } + + // no/incomplete request in buffer -> read more data first deadline := time.Now().Add(READ_TIMEOUT * time.Second) if err := self.c.SetDeadline(deadline); err != nil { return nil, false, err } + var retErr error for { - var err error - singleRequest := shared.Request{} - if err = self.d.Decode(&singleRequest); err == nil { - requests := make([]*shared.Request, 1) - requests[0] = &singleRequest - return requests, false, nil + n, err := self.c.Read(self.reqBuffer[self.bytesInReqBuffer:]) + if err != nil { + retErr = err + break } - fmt.Printf("err %T %v\n", err) + self.bytesInReqBuffer += n - if opErr, ok := err.(*net.OpError); ok { - if opErr.Timeout() { - break - } + requests, isBatch, err := self.nextRequest() + if err == nil { + return requests, isBatch, nil } - requests = make([]*shared.Request, 0) - if err = self.d.Decode(&requests); err == nil { - return requests, true, nil + if err == IncompleteRequestError || err == EmptyRequestQueueError { + continue // need more data } - if opErr, ok := err.(*net.OpError); ok { - if opErr.Timeout() { - break - } - } + retErr = err + break } - self.c.Close() // timeout - return nil, false, fmt.Errorf("Timeout reading request") + self.c.Close() + return nil, false, retErr } func (self *JsonCodec) ReadResponse() (interface{}, error) { diff --git a/rpc/codec/json_test.go b/rpc/codec/json_test.go new file mode 100644 index 000000000..60cac05f7 --- /dev/null +++ b/rpc/codec/json_test.go @@ -0,0 +1,177 @@ +package codec + +import ( + "bytes" + "io" + "net" + "testing" + "time" +) + +type jsonTestConn struct { + buffer *bytes.Buffer +} + +func newJsonTestConn(data []byte) *jsonTestConn { + return &jsonTestConn{ + buffer: bytes.NewBuffer(data), + } +} + +func (self *jsonTestConn) Read(p []byte) (n int, err error) { + return self.buffer.Read(p) +} + +func (self *jsonTestConn) Write(p []byte) (n int, err error) { + return self.buffer.Write(p) +} + +func (self *jsonTestConn) Close() error { + // not implemented + return nil +} + +func (self *jsonTestConn) LocalAddr() net.Addr { + // not implemented + return nil +} + +func (self *jsonTestConn) RemoteAddr() net.Addr { + // not implemented + return nil +} + +func (self *jsonTestConn) SetDeadline(t time.Time) error { + return nil +} + +func (self *jsonTestConn) SetReadDeadline(t time.Time) error { + return nil +} + +func (self *jsonTestConn) SetWriteDeadline(t time.Time) error { + return nil +} + +func TestJsonDecoderWithValidRequest(t *testing.T) { + reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","params":[],"id":64}`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != nil { + t.Errorf("Read valid request failed - %v", err) + } + + if len(requests) != 1 { + t.Errorf("Expected to get a single request but got %d", len(requests)) + } + + if batch { + t.Errorf("Got batch indication while expecting single request") + } + + if requests[0].Id != float64(64) { + t.Errorf("Expected req.Id == 64 but got %v", requests[0].Id) + } + + if requests[0].Method != "modules" { + t.Errorf("Expected req.Method == 'modules' got '%s'", requests[0].Method) + } +} + +func TestJsonDecoderWithValidBatchRequest(t *testing.T) { + reqdata := []byte(`[{"jsonrpc":"2.0","method":"modules","params":[],"id":64}, + {"jsonrpc":"2.0","method":"modules","params":[],"id":64}]`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != nil { + t.Errorf("Read valid batch request failed - %v", err) + } + + if len(requests) != 2 { + t.Errorf("Expected to get two requests but got %d", len(requests)) + } + + if !batch { + t.Errorf("Got no batch indication while expecting batch request") + } + + for i := 0; i < len(requests); i++ { + if requests[i].Id != float64(64) { + t.Errorf("Expected req.Id == 64 but got %v", requests[i].Id) + } + + if requests[i].Method != "modules" { + t.Errorf("Expected req.Method == 'modules' got '%s'", requests[i].Method) + } + } +} + +func TestJsonDecoderWithIncompleteMessage(t *testing.T) { + reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","pa`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != io.EOF { + t.Errorf("Expected to read an incomplete request err but got %v", err) + } + + // remaining message + decoder.Write([]byte(`rams":[],"id":64}`)) + requests, batch, err = jsonDecoder.ReadRequest() + + if err != nil { + t.Errorf("Read valid request failed - %v", err) + } + + if len(requests) != 1 { + t.Errorf("Expected to get a single request but got %d", len(requests)) + } + + if batch { + t.Errorf("Got batch indication while expecting single request") + } + + if requests[0].Id != float64(64) { + t.Errorf("Expected req.Id == 64 but got %v", requests[0].Id) + } + + if requests[0].Method != "modules" { + t.Errorf("Expected req.Method == 'modules' got '%s'", requests[0].Method) + } +} + +func TestJsonDecoderWithInvalidIncompleteMessage(t *testing.T) { + reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","pa`) + decoder := newJsonTestConn(reqdata) + + jsonDecoder := NewJsonCoder(decoder) + requests, batch, err := jsonDecoder.ReadRequest() + + if err != io.EOF { + t.Errorf("Expected to read an incomplete request err but got %v", err) + } + + // remaining message + decoder.Write([]byte(`rams":[],"id:64"}`)) + requests, batch, err = jsonDecoder.ReadRequest() + + if err == nil { + t.Errorf("Expected an error but got nil") + } + + if len(requests) != 0 { + t.Errorf("Expected to get no requests but got %d", len(requests)) + } + + if batch { + t.Errorf("Got batch indication while expecting non batch") + } +} From 56ed4084368e55bbcf9df4d7ef2e24662b8329d9 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Thu, 2 Jul 2015 17:20:58 +0200 Subject: [PATCH 6/9] ipcpath issue fix --- cmd/utils/flags.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 6f319eb40..45164741d 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -432,17 +432,17 @@ func MakeAccountManager(ctx *cli.Context) *accounts.Manager { func IpcSocketPath(ctx *cli.Context) (ipcpath string) { if common.IsWindows() { ipcpath = common.DefaultIpcPath() - if ipcpath != ctx.GlobalString(IPCPathFlag.Name) { + if ctx.GlobalIsSet(IPCPathFlag.Name) { ipcpath = ctx.GlobalString(IPCPathFlag.Name) } } else { ipcpath = common.DefaultIpcPath() - if ctx.GlobalString(IPCPathFlag.Name) != common.DefaultIpcPath() { - ipcpath = ctx.GlobalString(IPCPathFlag.Name) - } else if ctx.GlobalString(DataDirFlag.Name) != "" && - ctx.GlobalString(DataDirFlag.Name) != common.DefaultDataDir() { + if ctx.GlobalIsSet(DataDirFlag.Name) { ipcpath = filepath.Join(ctx.GlobalString(DataDirFlag.Name), "geth.ipc") } + if ctx.GlobalIsSet(IPCPathFlag.Name) { + ipcpath = ctx.GlobalString(IPCPathFlag.Name) + } } return From f0e94b4d714c45f7b03c66e01c643f4bd07033e3 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Fri, 3 Jul 2015 12:22:20 +0200 Subject: [PATCH 7/9] display rpc error in console --- rpc/codec/json.go | 10 +++++----- rpc/jeth.go | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/rpc/codec/json.go b/rpc/codec/json.go index b5ef94380..a4953a59c 100644 --- a/rpc/codec/json.go +++ b/rpc/codec/json.go @@ -156,15 +156,15 @@ func (self *JsonCodec) ReadResponse() (interface{}, error) { } bytesInBuffer += n + var failure shared.ErrorResponse + if err = json.Unmarshal(buf[:bytesInBuffer], &failure); err == nil && failure.Error != nil { + return failure, fmt.Errorf(failure.Error.Message) + } + var success shared.SuccessResponse if err = json.Unmarshal(buf[:bytesInBuffer], &success); err == nil { return success, nil } - - var failure shared.ErrorResponse - if err = json.Unmarshal(buf[:bytesInBuffer], &failure); err == nil && failure.Error != nil { - return failure, nil - } } self.c.Close() diff --git a/rpc/jeth.go b/rpc/jeth.go index 33fcd6efd..78e44c4da 100644 --- a/rpc/jeth.go +++ b/rpc/jeth.go @@ -3,6 +3,8 @@ package rpc import ( "encoding/json" + "fmt" + "github.com/ethereum/go-ethereum/jsre" "github.com/ethereum/go-ethereum/rpc/comms" "github.com/ethereum/go-ethereum/rpc/shared" @@ -20,14 +22,13 @@ func NewJeth(ethApi shared.EthereumApi, re *jsre.JSRE, client comms.EthereumClie } func (self *Jeth) err(call otto.FunctionCall, code int, msg string, id interface{}) (response otto.Value) { - rpcerr := &shared.ErrorObject{code, msg} - call.Otto.Set("ret_jsonrpc", shared.JsonRpcVersion) - call.Otto.Set("ret_id", id) - call.Otto.Set("ret_error", rpcerr) - response, _ = call.Otto.Run(` - ret_response = { jsonrpc: ret_jsonrpc, id: ret_id, error: ret_error }; - `) - return + errObj := fmt.Sprintf("{\"message\": \"%s\", \"code\": %d}", msg, code) + retResponse := fmt.Sprintf("ret_response = JSON.parse('{\"jsonrpc\": \"%s\", \"id\": %v, \"error\": %s}');", shared.JsonRpcVersion, id, errObj) + + call.Otto.Run("ret_error = " + errObj) + res, _ := call.Otto.Run(retResponse) + + return res } func (self *Jeth) Send(call otto.FunctionCall) (response otto.Value) { @@ -56,6 +57,7 @@ func (self *Jeth) Send(call otto.FunctionCall) (response otto.Value) { return self.err(call, -32603, err.Error(), req.Id) } respif, err = self.client.Recv() + if err != nil { return self.err(call, -32603, err.Error(), req.Id) } From d9efaf754c54b5a66f03c68a0c04fbad050e9370 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Fri, 3 Jul 2015 15:44:35 +0200 Subject: [PATCH 8/9] simplified implementation and improved performance --- rpc/codec/json.go | 123 ++++++++-------------------------------------- 1 file changed, 20 insertions(+), 103 deletions(-) diff --git a/rpc/codec/json.go b/rpc/codec/json.go index a4953a59c..8aa0e6bbf 100644 --- a/rpc/codec/json.go +++ b/rpc/codec/json.go @@ -15,129 +15,46 @@ const ( MAX_RESPONSE_SIZE = 1024 * 1024 ) -var ( - // No new requests in buffer - EmptyRequestQueueError = fmt.Errorf("No incoming requests") - // Next request in buffer isn't yet complete - IncompleteRequestError = fmt.Errorf("Request incomplete") -) - // Json serialization support type JsonCodec struct { - c net.Conn - reqBuffer []byte - bytesInReqBuffer int - reqLastPos int + c net.Conn + d *json.Decoder } // Create new JSON coder instance func NewJsonCoder(conn net.Conn) ApiCoder { return &JsonCodec{ - c: conn, - reqBuffer: make([]byte, MAX_REQUEST_SIZE), - bytesInReqBuffer: 0, - reqLastPos: 0, + c: conn, + d: json.NewDecoder(conn), } } -// Indication if the next request in the buffer is a batch request -func (self *JsonCodec) isNextBatchReq() (bool, error) { - for i := 0; i < self.bytesInReqBuffer; i++ { - switch self.reqBuffer[i] { - case 0x20, 0x09, 0x0a, 0x0d: // allow leading whitespace (JSON whitespace RFC4627) - continue - case 0x7b: // single req - return false, nil - case 0x5b: // batch req - return true, nil - default: - return false, &json.InvalidUnmarshalError{} - } - } - - return false, EmptyRequestQueueError -} - -// remove parsed request from buffer -func (self *JsonCodec) resetReqbuffer(pos int) { - copy(self.reqBuffer, self.reqBuffer[pos:self.bytesInReqBuffer]) - self.reqLastPos = 0 - self.bytesInReqBuffer -= pos -} - -// parse request in buffer -func (self *JsonCodec) nextRequest() (requests []*shared.Request, isBatch bool, err error) { - if isBatch, err := self.isNextBatchReq(); err == nil { - if isBatch { - requests = make([]*shared.Request, 0) - for ; self.reqLastPos <= self.bytesInReqBuffer; self.reqLastPos++ { - if err = json.Unmarshal(self.reqBuffer[:self.reqLastPos], &requests); err == nil { - self.resetReqbuffer(self.reqLastPos) - return requests, true, nil - } - } - return nil, true, IncompleteRequestError - } else { - request := shared.Request{} - for ; self.reqLastPos <= self.bytesInReqBuffer; self.reqLastPos++ { - if err = json.Unmarshal(self.reqBuffer[:self.reqLastPos], &request); err == nil { - requests := make([]*shared.Request, 1) - requests[0] = &request - self.resetReqbuffer(self.reqLastPos) - return requests, false, nil - } - } - return nil, true, IncompleteRequestError - } - } else { - return nil, false, err - } -} - -// Serialize obj to JSON and write it to conn +// Read incoming request and parse it to RPC request func (self *JsonCodec) ReadRequest() (requests []*shared.Request, isBatch bool, err error) { - if self.bytesInReqBuffer != 0 { - req, batch, err := self.nextRequest() - if err == nil { - return req, batch, err - } - - if err != IncompleteRequestError { - return nil, false, err - } - } - - // no/incomplete request in buffer -> read more data first deadline := time.Now().Add(READ_TIMEOUT * time.Second) if err := self.c.SetDeadline(deadline); err != nil { return nil, false, err } - var retErr error - for { - n, err := self.c.Read(self.reqBuffer[self.bytesInReqBuffer:]) - if err != nil { - retErr = err - break + var incoming json.RawMessage + err = self.d.Decode(&incoming) + if err == nil { + isBatch = incoming[0] == '[' + if isBatch { + requests = make([]*shared.Request, 0) + err = json.Unmarshal(incoming, &requests) + } else { + requests = make([]*shared.Request, 1) + var singleRequest shared.Request + if err = json.Unmarshal(incoming, &singleRequest); err == nil { + requests[0] = &singleRequest + } } - - self.bytesInReqBuffer += n - - requests, isBatch, err := self.nextRequest() - if err == nil { - return requests, isBatch, nil - } - - if err == IncompleteRequestError || err == EmptyRequestQueueError { - continue // need more data - } - - retErr = err - break + return } self.c.Close() - return nil, false, retErr + return nil, false, err } func (self *JsonCodec) ReadResponse() (interface{}, error) { From e8c1399bbf08234389f0e8f5da08f146856dab12 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Fri, 3 Jul 2015 16:57:40 +0200 Subject: [PATCH 9/9] fixed unittest after new implementation --- rpc/codec/json_test.go | 38 +------------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/rpc/codec/json_test.go b/rpc/codec/json_test.go index 60cac05f7..d5c672cdf 100644 --- a/rpc/codec/json_test.go +++ b/rpc/codec/json_test.go @@ -112,42 +112,6 @@ func TestJsonDecoderWithValidBatchRequest(t *testing.T) { } } -func TestJsonDecoderWithIncompleteMessage(t *testing.T) { - reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","pa`) - decoder := newJsonTestConn(reqdata) - - jsonDecoder := NewJsonCoder(decoder) - requests, batch, err := jsonDecoder.ReadRequest() - - if err != io.EOF { - t.Errorf("Expected to read an incomplete request err but got %v", err) - } - - // remaining message - decoder.Write([]byte(`rams":[],"id":64}`)) - requests, batch, err = jsonDecoder.ReadRequest() - - if err != nil { - t.Errorf("Read valid request failed - %v", err) - } - - if len(requests) != 1 { - t.Errorf("Expected to get a single request but got %d", len(requests)) - } - - if batch { - t.Errorf("Got batch indication while expecting single request") - } - - if requests[0].Id != float64(64) { - t.Errorf("Expected req.Id == 64 but got %v", requests[0].Id) - } - - if requests[0].Method != "modules" { - t.Errorf("Expected req.Method == 'modules' got '%s'", requests[0].Method) - } -} - func TestJsonDecoderWithInvalidIncompleteMessage(t *testing.T) { reqdata := []byte(`{"jsonrpc":"2.0","method":"modules","pa`) decoder := newJsonTestConn(reqdata) @@ -155,7 +119,7 @@ func TestJsonDecoderWithInvalidIncompleteMessage(t *testing.T) { jsonDecoder := NewJsonCoder(decoder) requests, batch, err := jsonDecoder.ReadRequest() - if err != io.EOF { + if err != io.ErrUnexpectedEOF { t.Errorf("Expected to read an incomplete request err but got %v", err) }