From 463014126f11a20c5e40f912a6f7415a0bbc910b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 15 Nov 2017 13:54:40 +0200 Subject: [PATCH 1/9] core/bloombits: handle non 8-bit boundary section matches --- core/bloombits/matcher.go | 6 ++-- core/bloombits/matcher_test.go | 57 +++++++++++++++++++++------------- eth/filters/filter.go | 1 + 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/core/bloombits/matcher.go b/core/bloombits/matcher.go index d38d4ba83e..ce3031702f 100644 --- a/core/bloombits/matcher.go +++ b/core/bloombits/matcher.go @@ -192,10 +192,12 @@ func (m *Matcher) Start(ctx context.Context, begin, end uint64, results chan uin } // Iterate over all the blocks in the section and return the matching ones for i := first; i <= last; i++ { - // Skip the entire byte if no matches are found inside + // Skip the entire byte if no matches are found inside (and we're processing an entire byte!) next := res.bitset[(i-sectionStart)/8] if next == 0 { - i += 7 + if i%8 == 0 { + i += 7 + } continue } // Some bit it set, do the actual submatching diff --git a/core/bloombits/matcher_test.go b/core/bloombits/matcher_test.go index 4a31854c58..7a5f78ef3a 100644 --- a/core/bloombits/matcher_test.go +++ b/core/bloombits/matcher_test.go @@ -56,33 +56,48 @@ func TestMatcherWildcards(t *testing.T) { // Tests the matcher pipeline on a single continuous workflow without interrupts. func TestMatcherContinuous(t *testing.T) { - testMatcherDiffBatches(t, [][]bloomIndexes{{{10, 20, 30}}}, 100000, false, 75) - testMatcherDiffBatches(t, [][]bloomIndexes{{{32, 3125, 100}}, {{40, 50, 10}}}, 100000, false, 81) - testMatcherDiffBatches(t, [][]bloomIndexes{{{4, 8, 11}, {7, 8, 17}}, {{9, 9, 12}, {15, 20, 13}}, {{18, 15, 15}, {12, 10, 4}}}, 10000, false, 36) + testMatcherDiffBatches(t, [][]bloomIndexes{{{10, 20, 30}}}, 0, 100000, false, 75) + testMatcherDiffBatches(t, [][]bloomIndexes{{{32, 3125, 100}}, {{40, 50, 10}}}, 0, 100000, false, 81) + testMatcherDiffBatches(t, [][]bloomIndexes{{{4, 8, 11}, {7, 8, 17}}, {{9, 9, 12}, {15, 20, 13}}, {{18, 15, 15}, {12, 10, 4}}}, 0, 10000, false, 36) } // Tests the matcher pipeline on a constantly interrupted and resumed work pattern // with the aim of ensuring data items are requested only once. func TestMatcherIntermittent(t *testing.T) { - testMatcherDiffBatches(t, [][]bloomIndexes{{{10, 20, 30}}}, 100000, true, 75) - testMatcherDiffBatches(t, [][]bloomIndexes{{{32, 3125, 100}}, {{40, 50, 10}}}, 100000, true, 81) - testMatcherDiffBatches(t, [][]bloomIndexes{{{4, 8, 11}, {7, 8, 17}}, {{9, 9, 12}, {15, 20, 13}}, {{18, 15, 15}, {12, 10, 4}}}, 10000, true, 36) + testMatcherDiffBatches(t, [][]bloomIndexes{{{10, 20, 30}}}, 0, 100000, true, 75) + testMatcherDiffBatches(t, [][]bloomIndexes{{{32, 3125, 100}}, {{40, 50, 10}}}, 0, 100000, true, 81) + testMatcherDiffBatches(t, [][]bloomIndexes{{{4, 8, 11}, {7, 8, 17}}, {{9, 9, 12}, {15, 20, 13}}, {{18, 15, 15}, {12, 10, 4}}}, 0, 10000, true, 36) } // Tests the matcher pipeline on random input to hopefully catch anomalies. func TestMatcherRandom(t *testing.T) { for i := 0; i < 10; i++ { - testMatcherBothModes(t, makeRandomIndexes([]int{1}, 50), 10000, 0) - testMatcherBothModes(t, makeRandomIndexes([]int{3}, 50), 10000, 0) - testMatcherBothModes(t, makeRandomIndexes([]int{2, 2, 2}, 20), 10000, 0) - testMatcherBothModes(t, makeRandomIndexes([]int{5, 5, 5}, 50), 10000, 0) - testMatcherBothModes(t, makeRandomIndexes([]int{4, 4, 4}, 20), 10000, 0) + testMatcherBothModes(t, makeRandomIndexes([]int{1}, 50), 0, 10000, 0) + testMatcherBothModes(t, makeRandomIndexes([]int{3}, 50), 0, 10000, 0) + testMatcherBothModes(t, makeRandomIndexes([]int{2, 2, 2}, 20), 0, 10000, 0) + testMatcherBothModes(t, makeRandomIndexes([]int{5, 5, 5}, 50), 0, 10000, 0) + testMatcherBothModes(t, makeRandomIndexes([]int{4, 4, 4}, 20), 0, 10000, 0) } } +// Tests that the matcher can properly find matches if the starting block is +// shifter from a multiple of 8. This is needed to cover an optimisation with +// bitset matching https://github.com/ethereum/go-ethereum/issues/15309. +func TestMatcherShifted(t *testing.T) { + // Block 0 always matches in the tests, skip ahead of first 8 blocks with the + // start to get a potential zero byte in the matcher bitset. + + // To keep the second bitset byte zero, the filter must only match for the first + // time in block 16, so doing an all-16 bit filter should suffice. + + // To keep the starting block non divisible by 8, block number 9 is the first + // that would introduce a shift and not match block 0. + testMatcherBothModes(t, [][]bloomIndexes{{{16, 16, 16}}}, 9, 64, 0) +} + // Tests that matching on everything doesn't crash (special case internally). func TestWildcardMatcher(t *testing.T) { - testMatcherBothModes(t, nil, 10000, 0) + testMatcherBothModes(t, nil, 0, 10000, 0) } // makeRandomIndexes generates a random filter system, composed on multiple filter @@ -104,9 +119,9 @@ func makeRandomIndexes(lengths []int, max int) [][]bloomIndexes { // testMatcherDiffBatches runs the given matches test in single-delivery and also // in batches delivery mode, verifying that all kinds of deliveries are handled // correctly withn. -func testMatcherDiffBatches(t *testing.T, filter [][]bloomIndexes, blocks uint64, intermittent bool, retrievals uint32) { - singleton := testMatcher(t, filter, blocks, intermittent, retrievals, 1) - batched := testMatcher(t, filter, blocks, intermittent, retrievals, 16) +func testMatcherDiffBatches(t *testing.T, filter [][]bloomIndexes, start, blocks uint64, intermittent bool, retrievals uint32) { + singleton := testMatcher(t, filter, start, blocks, intermittent, retrievals, 1) + batched := testMatcher(t, filter, start, blocks, intermittent, retrievals, 16) if singleton != batched { t.Errorf("filter = %v blocks = %v intermittent = %v: request count mismatch, %v in signleton vs. %v in batched mode", filter, blocks, intermittent, singleton, batched) @@ -115,9 +130,9 @@ func testMatcherDiffBatches(t *testing.T, filter [][]bloomIndexes, blocks uint64 // testMatcherBothModes runs the given matcher test in both continuous as well as // in intermittent mode, verifying that the request counts match each other. -func testMatcherBothModes(t *testing.T, filter [][]bloomIndexes, blocks uint64, retrievals uint32) { - continuous := testMatcher(t, filter, blocks, false, retrievals, 16) - intermittent := testMatcher(t, filter, blocks, true, retrievals, 16) +func testMatcherBothModes(t *testing.T, filter [][]bloomIndexes, start, blocks uint64, retrievals uint32) { + continuous := testMatcher(t, filter, start, blocks, false, retrievals, 16) + intermittent := testMatcher(t, filter, start, blocks, true, retrievals, 16) if continuous != intermittent { t.Errorf("filter = %v blocks = %v: request count mismatch, %v in continuous vs. %v in intermittent mode", filter, blocks, continuous, intermittent) @@ -126,7 +141,7 @@ func testMatcherBothModes(t *testing.T, filter [][]bloomIndexes, blocks uint64, // testMatcher is a generic tester to run the given matcher test and return the // number of requests made for cross validation between different modes. -func testMatcher(t *testing.T, filter [][]bloomIndexes, blocks uint64, intermittent bool, retrievals uint32, maxReqCount int) uint32 { +func testMatcher(t *testing.T, filter [][]bloomIndexes, start, blocks uint64, intermittent bool, retrievals uint32, maxReqCount int) uint32 { // Create a new matcher an simulate our explicit random bitsets matcher := NewMatcher(testSectionSize, nil) matcher.filters = filter @@ -145,14 +160,14 @@ func testMatcher(t *testing.T, filter [][]bloomIndexes, blocks uint64, intermitt quit := make(chan struct{}) matches := make(chan uint64, 16) - session, err := matcher.Start(context.Background(), 0, blocks-1, matches) + session, err := matcher.Start(context.Background(), start, blocks-1, matches) if err != nil { t.Fatalf("failed to stat matcher session: %v", err) } startRetrievers(session, quit, &requested, maxReqCount) // Iterate over all the blocks and verify that the pipeline produces the correct matches - for i := uint64(0); i < blocks; i++ { + for i := start; i < blocks; i++ { if expMatch3(filter, i) { match, ok := <-matches if !ok { diff --git a/eth/filters/filter.go b/eth/filters/filter.go index e208f8f38e..43d7e2a812 100644 --- a/eth/filters/filter.go +++ b/eth/filters/filter.go @@ -158,6 +158,7 @@ func (f *Filter) indexedLogs(ctx context.Context, end uint64) ([]*types.Log, err return logs, err } f.begin = int64(number) + 1 + // Retrieve the suggested block and pull any truly matching logs header, err := f.backend.HeaderByNumber(ctx, rpc.BlockNumber(number)) if header == nil || err != nil { From 5aa3eac22d80c754d70651a918bccc9b3d1d216f Mon Sep 17 00:00:00 2001 From: jtakalai Date: Thu, 16 Nov 2017 13:14:51 +0200 Subject: [PATCH 2/9] eth/downloader: minor comments cleanup (#15495) it's -> its pet peeve, and I like to imagine I'm not alone. --- eth/downloader/downloader.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index cca4fe7a93..b338129e00 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -333,7 +333,7 @@ func (d *Downloader) Synchronise(id string, head common.Hash, td *big.Int, mode } // synchronise will select the peer and use it for synchronising. If an empty string is given -// it will use the best peer possible and synchronize if it's TD is higher than our own. If any of the +// it will use the best peer possible and synchronize if its TD is higher than our own. If any of the // checks fail an error will be returned. This method is synchronous func (d *Downloader) synchronise(id string, hash common.Hash, td *big.Int, mode SyncMode) error { // Mock out the synchronisation if testing @@ -1003,8 +1003,8 @@ func (d *Downloader) fetchParts(errCancel error, deliveryCh chan dataPack, deliv return errCancel case packet := <-deliveryCh: - // If the peer was previously banned and failed to deliver it's pack - // in a reasonable time frame, ignore it's message. + // If the peer was previously banned and failed to deliver its pack + // in a reasonable time frame, ignore its message. if peer := d.peers.Peer(packet.PeerId()); peer != nil { // Deliver the received chunk of data and check chain validity accepted, err := deliver(packet) @@ -1205,8 +1205,8 @@ func (d *Downloader) processHeaders(origin uint64, td *big.Int) error { case <-d.cancelCh: } } - // If no headers were retrieved at all, the peer violated it's TD promise that it had a - // better chain compared to ours. The only exception is if it's promised blocks were + // If no headers were retrieved at all, the peer violated its TD promise that it had a + // better chain compared to ours. The only exception is if its promised blocks were // already imported by other means (e.g. fecher): // // R , L : Both at block 10 From 4013e23312257d79caf4fb5030881d30a62cb618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 16 Nov 2017 13:51:06 +0200 Subject: [PATCH 3/9] rpc: allow dumb empty requests for AWS health checks --- rpc/http.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/rpc/http.go b/rpc/http.go index 3f572b34c0..2ac9f6c371 100644 --- a/rpc/http.go +++ b/rpc/http.go @@ -146,13 +146,17 @@ func NewHTTPServer(cors []string, srv *Server) *http.Server { // ServeHTTP serves JSON-RPC requests over HTTP. func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // Permit dumb empty requests for remote health-checks (AWS) + if r.Method == "GET" && r.ContentLength == 0 && r.URL.RawQuery == "" { + return + } + // For meaningful requests, validate it's size and content type if r.ContentLength > maxHTTPRequestContentLength { http.Error(w, fmt.Sprintf("content length too large (%d>%d)", r.ContentLength, maxHTTPRequestContentLength), http.StatusRequestEntityTooLarge) return } - ct := r.Header.Get("content-type") mt, _, err := mime.ParseMediaType(ct) if err != nil || mt != "application/json" { @@ -161,14 +165,13 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { http.StatusUnsupportedMediaType) return } - - w.Header().Set("content-type", "application/json") - - // create a codec that reads direct from the request body until - // EOF and writes the response to w and order the server to process - // a single request. + // All checks passed, create a codec that reads direct from the request body + // untilEOF and writes the response to w and order the server to process a + // single request. codec := NewJSONCodec(&httpReadWriteNopCloser{r.Body, w}) defer codec.Close() + + w.Header().Set("content-type", "application/json") srv.ServeSingleRequest(codec, OptionMethodInvocation) } From b0190189a386d13eb2e8bbdb6d64d9ef8c0e572a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 16 Nov 2017 18:53:18 +0200 Subject: [PATCH 4/9] core/vm, internal/ethapi: tracer no full storage, nicer json output (#15499) * core/vm, internal/ethapi: tracer no full storage, nicer json output * core/vm, internal/ethapi: omit disabled trace fields --- core/vm/logger.go | 26 +++---------------- core/vm/logger_test.go | 24 ------------------ internal/ethapi/api.go | 57 ++++++++++++++++++++++++------------------ 3 files changed, 36 insertions(+), 71 deletions(-) diff --git a/core/vm/logger.go b/core/vm/logger.go index 623c0d5632..75309da921 100644 --- a/core/vm/logger.go +++ b/core/vm/logger.go @@ -45,7 +45,6 @@ type LogConfig struct { DisableMemory bool // disable memory capture DisableStack bool // disable stack capture DisableStorage bool // disable storage capture - FullStorage bool // show full storage (slow) Limit int // maximum length of output, but zero means unlimited } @@ -136,14 +135,13 @@ func (l *StructLogger) CaptureState(env *EVM, pc uint64, op OpCode, gas, cost ui ) l.changedValues[contract.Address()][address] = value } - // copy a snapstot of the current memory state to a new buffer + // Copy a snapstot of the current memory state to a new buffer var mem []byte if !l.cfg.DisableMemory { mem = make([]byte, len(memory.Data())) copy(mem, memory.Data()) } - - // copy a snapshot of the current stack state to a new buffer + // Copy a snapshot of the current stack state to a new buffer var stck []*big.Int if !l.cfg.DisableStack { stck = make([]*big.Int, len(stack.Data())) @@ -151,26 +149,10 @@ func (l *StructLogger) CaptureState(env *EVM, pc uint64, op OpCode, gas, cost ui stck[i] = new(big.Int).Set(item) } } - - // Copy the storage based on the settings specified in the log config. If full storage - // is disabled (default) we can use the simple Storage.Copy method, otherwise we use - // the state object to query for all values (slow process). + // Copy a snapshot of the current storage to a new container var storage Storage if !l.cfg.DisableStorage { - if l.cfg.FullStorage { - storage = make(Storage) - // Get the contract account and loop over each storage entry. This may involve looping over - // the trie and is a very expensive process. - - env.StateDB.ForEachStorage(contract.Address(), func(key, value common.Hash) bool { - storage[key] = value - // Return true, indicating we'd like to continue. - return true - }) - } else { - // copy a snapshot of the current storage to a new container. - storage = l.changedValues[contract.Address()].Copy() - } + storage = l.changedValues[contract.Address()].Copy() } // create a new snaptshot of the EVM. log := StructLog{pc, op, gas, cost, mem, memory.Len(), stck, storage, depth, err} diff --git a/core/vm/logger_test.go b/core/vm/logger_test.go index b6fa31132a..915f7177e7 100644 --- a/core/vm/logger_test.go +++ b/core/vm/logger_test.go @@ -63,32 +63,8 @@ func TestStoreCapture(t *testing.T) { if len(logger.changedValues[contract.Address()]) == 0 { t.Fatalf("expected exactly 1 changed value on address %x, got %d", contract.Address(), len(logger.changedValues[contract.Address()])) } - exp := common.BigToHash(big.NewInt(1)) if logger.changedValues[contract.Address()][index] != exp { t.Errorf("expected %x, got %x", exp, logger.changedValues[contract.Address()][index]) } } - -func TestStorageCapture(t *testing.T) { - t.Skip("implementing this function is difficult. it requires all sort of interfaces to be implemented which isn't trivial. The value (the actual test) isn't worth it") - var ( - ref = &dummyContractRef{} - contract = NewContract(ref, ref, new(big.Int), 0) - env = NewEVM(Context{}, dummyStateDB{ref: ref}, params.TestChainConfig, Config{EnableJit: false, ForceJit: false}) - logger = NewStructLogger(nil) - mem = NewMemory() - stack = newstack() - ) - - logger.CaptureState(env, 0, STOP, 0, 0, mem, stack, contract, 0, nil) - if ref.calledForEach { - t.Error("didn't expect for each to be called") - } - - logger = NewStructLogger(&LogConfig{FullStorage: true}) - logger.CaptureState(env, 0, STOP, 0, 0, mem, stack, contract, 0, nil) - if !ref.calledForEach { - t.Error("expected for each to be called") - } -} diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 59a29d7226..cf15d1c71e 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -710,45 +710,52 @@ type ExecutionResult struct { // StructLogRes stores a structured log emitted by the EVM while replaying a // transaction in debug mode type StructLogRes struct { - Pc uint64 `json:"pc"` - Op string `json:"op"` - Gas uint64 `json:"gas"` - GasCost uint64 `json:"gasCost"` - Depth int `json:"depth"` - Error error `json:"error"` - Stack []string `json:"stack"` - Memory []string `json:"memory"` - Storage map[string]string `json:"storage"` + Pc uint64 `json:"pc"` + Op string `json:"op"` + Gas uint64 `json:"gas"` + GasCost uint64 `json:"gasCost"` + Depth int `json:"depth"` + Error error `json:"error,omitempty"` + Stack *[]string `json:"stack,omitempty"` + Memory *[]string `json:"memory,omitempty"` + Storage *map[string]string `json:"storage,omitempty"` } // formatLogs formats EVM returned structured logs for json output -func FormatLogs(structLogs []vm.StructLog) []StructLogRes { - formattedStructLogs := make([]StructLogRes, len(structLogs)) - for index, trace := range structLogs { - formattedStructLogs[index] = StructLogRes{ +func FormatLogs(logs []vm.StructLog) []StructLogRes { + formatted := make([]StructLogRes, len(logs)) + for index, trace := range logs { + formatted[index] = StructLogRes{ Pc: trace.Pc, Op: trace.Op.String(), Gas: trace.Gas, GasCost: trace.GasCost, Depth: trace.Depth, Error: trace.Err, - Stack: make([]string, len(trace.Stack)), - Storage: make(map[string]string), } - - for i, stackValue := range trace.Stack { - formattedStructLogs[index].Stack[i] = fmt.Sprintf("%x", math.PaddedBigBytes(stackValue, 32)) + if trace.Stack != nil { + stack := make([]string, len(trace.Stack)) + for i, stackValue := range trace.Stack { + stack[i] = fmt.Sprintf("%x", math.PaddedBigBytes(stackValue, 32)) + } + formatted[index].Stack = &stack } - - for i := 0; i+32 <= len(trace.Memory); i += 32 { - formattedStructLogs[index].Memory = append(formattedStructLogs[index].Memory, fmt.Sprintf("%x", trace.Memory[i:i+32])) + if trace.Memory != nil { + memory := make([]string, 0, (len(trace.Memory)+31)/32) + for i := 0; i+32 <= len(trace.Memory); i += 32 { + memory = append(memory, fmt.Sprintf("%x", trace.Memory[i:i+32])) + } + formatted[index].Memory = &memory } - - for i, storageValue := range trace.Storage { - formattedStructLogs[index].Storage[fmt.Sprintf("%x", i)] = fmt.Sprintf("%x", storageValue) + if trace.Storage != nil { + storage := make(map[string]string) + for i, storageValue := range trace.Storage { + storage[fmt.Sprintf("%x", i)] = fmt.Sprintf("%x", storageValue) + } + formatted[index].Storage = &storage } } - return formattedStructLogs + return formatted } // rpcOutputBlock converts the given block to the RPC output which depends on fullTx. If inclTx is true transactions are From c5b8569707cabe19f861cb67062c07598aff2aa1 Mon Sep 17 00:00:00 2001 From: Armani Ferrante Date: Fri, 17 Nov 2017 04:07:11 -0800 Subject: [PATCH 5/9] rpc: disallow PUT and DELETE on HTTP (#15501) Fixes #15493 --- rpc/http.go | 43 ++++++++++++++++++++++++++++--------------- rpc/http_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 rpc/http_test.go diff --git a/rpc/http.go b/rpc/http.go index 2ac9f6c371..68634e3fdf 100644 --- a/rpc/http.go +++ b/rpc/http.go @@ -33,6 +33,7 @@ import ( ) const ( + contentType = "application/json" maxHTTPRequestContentLength = 1024 * 128 ) @@ -69,8 +70,8 @@ func DialHTTP(endpoint string) (*Client, error) { if err != nil { return nil, err } - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Accept", "application/json") + req.Header.Set("Content-Type", contentType) + req.Header.Set("Accept", contentType) initctx := context.Background() return newClient(initctx, func(context.Context) (net.Conn, error) { @@ -150,21 +151,11 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.Method == "GET" && r.ContentLength == 0 && r.URL.RawQuery == "" { return } - // For meaningful requests, validate it's size and content type - if r.ContentLength > maxHTTPRequestContentLength { - http.Error(w, - fmt.Sprintf("content length too large (%d>%d)", r.ContentLength, maxHTTPRequestContentLength), - http.StatusRequestEntityTooLarge) - return - } - ct := r.Header.Get("content-type") - mt, _, err := mime.ParseMediaType(ct) - if err != nil || mt != "application/json" { - http.Error(w, - "invalid content type, only application/json is supported", - http.StatusUnsupportedMediaType) + if responseCode, errorMessage := httpErrorResponse(r); responseCode != 0 { + http.Error(w, errorMessage, responseCode) return } + // All checks passed, create a codec that reads direct from the request body // untilEOF and writes the response to w and order the server to process a // single request. @@ -175,6 +166,28 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { srv.ServeSingleRequest(codec, OptionMethodInvocation) } +// Returns a non-zero response code and error message if the request is invalid. +func httpErrorResponse(r *http.Request) (int, string) { + if r.Method == "PUT" || r.Method == "DELETE" { + errorMessage := "method not allowed" + return http.StatusMethodNotAllowed, errorMessage + } + + if r.ContentLength > maxHTTPRequestContentLength { + errorMessage := fmt.Sprintf("content length too large (%d>%d)", r.ContentLength, maxHTTPRequestContentLength) + return http.StatusRequestEntityTooLarge, errorMessage + } + + ct := r.Header.Get("content-type") + mt, _, err := mime.ParseMediaType(ct) + if err != nil || mt != contentType { + errorMessage := fmt.Sprintf("invalid content type, only %s is supported", contentType) + return http.StatusUnsupportedMediaType, errorMessage + } + + return 0, "" +} + func newCorsHandler(srv *Server, allowedOrigins []string) http.Handler { // disable CORS support if user has not specified a custom CORS configuration if len(allowedOrigins) == 0 { diff --git a/rpc/http_test.go b/rpc/http_test.go new file mode 100644 index 0000000000..f4afd52167 --- /dev/null +++ b/rpc/http_test.go @@ -0,0 +1,40 @@ +package rpc + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestHTTPErrorResponseWithDelete(t *testing.T) { + httpErrorResponseTest(t, "DELETE", contentType, "", http.StatusMethodNotAllowed) +} + +func TestHTTPErrorResponseWithPut(t *testing.T) { + httpErrorResponseTest(t, "PUT", contentType, "", http.StatusMethodNotAllowed) +} + +func TestHTTPErrorResponseWithMaxContentLength(t *testing.T) { + body := make([]rune, maxHTTPRequestContentLength+1, maxHTTPRequestContentLength+1) + httpErrorResponseTest(t, + "POST", contentType, string(body), http.StatusRequestEntityTooLarge) +} + +func TestHTTPErrorResponseWithEmptyContentType(t *testing.T) { + httpErrorResponseTest(t, "POST", "", "", http.StatusUnsupportedMediaType) +} + +func TestHTTPErrorResponseWithValidRequest(t *testing.T) { + httpErrorResponseTest(t, "POST", contentType, "", 0) +} + +func httpErrorResponseTest(t *testing.T, + method, contentType, body string, expectedResponse int) { + + request := httptest.NewRequest(method, "http://url.com", strings.NewReader(body)) + request.Header.Set("content-type", contentType) + if response, _ := httpErrorResponse(request); response != expectedResponse { + t.Fatalf("response code should be %d not %d", expectedResponse, response) + } +} From 3c6b9c5d726e6dda72f469fefd1a37b37a0a1621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 17 Nov 2017 14:18:46 +0200 Subject: [PATCH 6/9] rpc: minor cleanups to RPC PR --- rpc/http.go | 32 ++++++++++++++------------------ rpc/http_test.go | 34 ++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/rpc/http.go b/rpc/http.go index 68634e3fdf..5941c06776 100644 --- a/rpc/http.go +++ b/rpc/http.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -151,41 +152,36 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.Method == "GET" && r.ContentLength == 0 && r.URL.RawQuery == "" { return } - if responseCode, errorMessage := httpErrorResponse(r); responseCode != 0 { - http.Error(w, errorMessage, responseCode) + if code, err := validateRequest(r); err != nil { + http.Error(w, err.Error(), code) return } - // All checks passed, create a codec that reads direct from the request body // untilEOF and writes the response to w and order the server to process a // single request. codec := NewJSONCodec(&httpReadWriteNopCloser{r.Body, w}) defer codec.Close() - w.Header().Set("content-type", "application/json") + w.Header().Set("content-type", contentType) srv.ServeSingleRequest(codec, OptionMethodInvocation) } -// Returns a non-zero response code and error message if the request is invalid. -func httpErrorResponse(r *http.Request) (int, string) { +// validateRequest returns a non-zero response code and error message if the +// request is invalid. +func validateRequest(r *http.Request) (int, error) { if r.Method == "PUT" || r.Method == "DELETE" { - errorMessage := "method not allowed" - return http.StatusMethodNotAllowed, errorMessage + return http.StatusMethodNotAllowed, errors.New("method not allowed") } - if r.ContentLength > maxHTTPRequestContentLength { - errorMessage := fmt.Sprintf("content length too large (%d>%d)", r.ContentLength, maxHTTPRequestContentLength) - return http.StatusRequestEntityTooLarge, errorMessage + err := fmt.Errorf("content length too large (%d>%d)", r.ContentLength, maxHTTPRequestContentLength) + return http.StatusRequestEntityTooLarge, err } - - ct := r.Header.Get("content-type") - mt, _, err := mime.ParseMediaType(ct) + mt, _, err := mime.ParseMediaType(r.Header.Get("content-type")) if err != nil || mt != contentType { - errorMessage := fmt.Sprintf("invalid content type, only %s is supported", contentType) - return http.StatusUnsupportedMediaType, errorMessage + err := fmt.Errorf("invalid content type, only %s is supported", contentType) + return http.StatusUnsupportedMediaType, err } - - return 0, "" + return 0, nil } func newCorsHandler(srv *Server, allowedOrigins []string) http.Handler { diff --git a/rpc/http_test.go b/rpc/http_test.go index f4afd52167..1cb7a7acbf 100644 --- a/rpc/http_test.go +++ b/rpc/http_test.go @@ -1,3 +1,19 @@ +// Copyright 2017 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + package rpc import ( @@ -8,33 +24,31 @@ import ( ) func TestHTTPErrorResponseWithDelete(t *testing.T) { - httpErrorResponseTest(t, "DELETE", contentType, "", http.StatusMethodNotAllowed) + testHTTPErrorResponse(t, "DELETE", contentType, "", http.StatusMethodNotAllowed) } func TestHTTPErrorResponseWithPut(t *testing.T) { - httpErrorResponseTest(t, "PUT", contentType, "", http.StatusMethodNotAllowed) + testHTTPErrorResponse(t, "PUT", contentType, "", http.StatusMethodNotAllowed) } func TestHTTPErrorResponseWithMaxContentLength(t *testing.T) { body := make([]rune, maxHTTPRequestContentLength+1, maxHTTPRequestContentLength+1) - httpErrorResponseTest(t, + testHTTPErrorResponse(t, "POST", contentType, string(body), http.StatusRequestEntityTooLarge) } func TestHTTPErrorResponseWithEmptyContentType(t *testing.T) { - httpErrorResponseTest(t, "POST", "", "", http.StatusUnsupportedMediaType) + testHTTPErrorResponse(t, "POST", "", "", http.StatusUnsupportedMediaType) } func TestHTTPErrorResponseWithValidRequest(t *testing.T) { - httpErrorResponseTest(t, "POST", contentType, "", 0) + testHTTPErrorResponse(t, "POST", contentType, "", 0) } -func httpErrorResponseTest(t *testing.T, - method, contentType, body string, expectedResponse int) { - +func testHTTPErrorResponse(t *testing.T, method, contentType, body string, expected int) { request := httptest.NewRequest(method, "http://url.com", strings.NewReader(body)) request.Header.Set("content-type", contentType) - if response, _ := httpErrorResponse(request); response != expectedResponse { - t.Fatalf("response code should be %d not %d", expectedResponse, response) + if code, _ := validateRequest(request); code != expected { + t.Fatalf("response code should be %d not %d", expected, code) } } From c7b0abf86bf3039a87b5b5ae88635326a762cf31 Mon Sep 17 00:00:00 2001 From: tsarpaul Date: Fri, 17 Nov 2017 15:07:57 +0200 Subject: [PATCH 7/9] Added output to clarify gas calculation in txpool.inspect --- internal/ethapi/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index cf15d1c71e..025f426177 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -151,9 +151,9 @@ func (s *PublicTxPoolAPI) Inspect() map[string]map[string]map[string]string { // Define a formatter to flatten a transaction into a string var format = func(tx *types.Transaction) string { if to := tx.To(); to != nil { - return fmt.Sprintf("%s: %v wei + %v × %v gas", tx.To().Hex(), tx.Value(), tx.Gas(), tx.GasPrice()) + return fmt.Sprintf("%s: %v wei + %v gas × %v wei", tx.To().Hex(), tx.Value(), tx.Gas(), tx.GasPrice()) } - return fmt.Sprintf("contract creation: %v wei + %v × %v gas", tx.Value(), tx.Gas(), tx.GasPrice()) + return fmt.Sprintf("contract creation: %v wei + %v gas × %v wei", tx.Value(), tx.Gas(), tx.GasPrice()) } // Flatten the pending transactions for account, txs := range pending { From 2ab5c11261f742ee44afb0b70a33241fe2008c9d Mon Sep 17 00:00:00 2001 From: Martin Michlmayr Date: Fri, 17 Nov 2017 14:26:55 +0000 Subject: [PATCH 8/9] README: fix typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 57b0ea92bc..61e36afec4 100644 --- a/README.md +++ b/README.md @@ -266,7 +266,7 @@ instance for mining, run it with all your usual flags, extended by: $ geth --mine --minerthreads=1 --etherbase=0x0000000000000000000000000000000000000000 ``` -Which will start mining bocks and transactions on a single CPU thread, crediting all proceedings to +Which will start mining blocks and transactions on a single CPU thread, crediting all proceedings to the account specified by `--etherbase`. You can further tune the mining by changing the default gas limit blocks converge to (`--targetgaslimit`) and the price transactions are accepted at (`--gasprice`). From f5091e5711fc18205ed3a7c2d9d6a7fe7f0262f2 Mon Sep 17 00:00:00 2001 From: Pulyak Viktor Date: Sat, 18 Nov 2017 04:56:03 +0300 Subject: [PATCH 9/9] internal/ethapi: fix js tracer to properly decode addresses (#15297) * Add method getBalanceFromJs for work with address as bytes * expect []byte instead of common.Address in ethapi tracer --- internal/ethapi/tracer.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/ethapi/tracer.go b/internal/ethapi/tracer.go index 0516265270..fc742e6c44 100644 --- a/internal/ethapi/tracer.go +++ b/internal/ethapi/tracer.go @@ -130,28 +130,28 @@ type dbWrapper struct { } // getBalance retrieves an account's balance -func (dw *dbWrapper) getBalance(addr common.Address) *big.Int { - return dw.db.GetBalance(addr) +func (dw *dbWrapper) getBalance(addr []byte) *big.Int { + return dw.db.GetBalance(common.BytesToAddress(addr)) } // getNonce retrieves an account's nonce -func (dw *dbWrapper) getNonce(addr common.Address) uint64 { - return dw.db.GetNonce(addr) +func (dw *dbWrapper) getNonce(addr []byte) uint64 { + return dw.db.GetNonce(common.BytesToAddress(addr)) } // getCode retrieves an account's code -func (dw *dbWrapper) getCode(addr common.Address) []byte { - return dw.db.GetCode(addr) +func (dw *dbWrapper) getCode(addr []byte) []byte { + return dw.db.GetCode(common.BytesToAddress(addr)) } // getState retrieves an account's state data for the given hash -func (dw *dbWrapper) getState(addr common.Address, hash common.Hash) common.Hash { - return dw.db.GetState(addr, hash) +func (dw *dbWrapper) getState(addr []byte, hash common.Hash) common.Hash { + return dw.db.GetState(common.BytesToAddress(addr), hash) } // exists returns true iff the account exists -func (dw *dbWrapper) exists(addr common.Address) bool { - return dw.db.Exist(addr) +func (dw *dbWrapper) exists(addr []byte) bool { + return dw.db.Exist(common.BytesToAddress(addr)) } // toValue returns an otto.Value for the dbWrapper