From edc22e66330fca6ef2bff42f00fea8860b011f0c Mon Sep 17 00:00:00 2001 From: syntrust Date: Fri, 7 Feb 2025 17:02:07 +0800 Subject: [PATCH] fix repeated params --- ethstorage/archiver/config.go | 27 ++++++---- ethstorage/archiver/service.go | 21 +++----- ethstorage/archiver/utils.go | 51 +++++++++++++++++-- ethstorage/archiver/utils_test.go | 82 +++++++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 26 deletions(-) create mode 100644 ethstorage/archiver/utils_test.go diff --git a/ethstorage/archiver/config.go b/ethstorage/archiver/config.go index 11f0b7d3..7da4eaf6 100644 --- a/ethstorage/archiver/config.go +++ b/ethstorage/archiver/config.go @@ -9,15 +9,17 @@ import ( ) const ( - EnabledFlagName = "archiver.enabled" - ListenAddrFlagName = "archiver.addr" - ListenPortFlagName = "archiver.port" + EnabledFlagName = "archiver.enabled" + ListenAddrFlagName = "archiver.addr" + ListenPortFlagName = "archiver.port" + MaxBlobsPerBlockName = "archiver.maxBlobsPerBlock" ) type Config struct { - Enabled bool - ListenAddr string - ListenPort int + Enabled bool + ListenAddr string + ListenPort int + MaxBlobsPerBlock int } func CLIFlags(envPrefix string) []cli.Flag { @@ -40,15 +42,22 @@ func CLIFlags(envPrefix string) []cli.Flag { EnvVar: rollup.PrefixEnvVar(envPrefix, "PORT"), Value: 9645, }, + cli.IntFlag{ + Name: MaxBlobsPerBlockName, + Usage: "Max blobs per block", + EnvVar: rollup.PrefixEnvVar(envPrefix, "MAX_BLOBS_PER_BLOCK"), + Value: 6, + }, } return flags } func NewConfig(ctx *cli.Context) *Config { cfg := Config{ - Enabled: ctx.GlobalBool(EnabledFlagName), - ListenAddr: ctx.GlobalString(ListenAddrFlagName), - ListenPort: ctx.GlobalInt(ListenPortFlagName), + Enabled: ctx.GlobalBool(EnabledFlagName), + ListenAddr: ctx.GlobalString(ListenAddrFlagName), + ListenPort: ctx.GlobalInt(ListenPortFlagName), + MaxBlobsPerBlock: ctx.GlobalInt(MaxBlobsPerBlockName), } if cfg.Enabled { return &cfg diff --git a/ethstorage/archiver/service.go b/ethstorage/archiver/service.go index 5e4fd4f6..18e7e37a 100644 --- a/ethstorage/archiver/service.go +++ b/ethstorage/archiver/service.go @@ -10,7 +10,6 @@ import ( "net" "net/http" "strconv" - "strings" "time" "github.com/ethereum-optimism/optimism/op-service/httputil" @@ -47,23 +46,17 @@ func (a *APIService) blobSidecarHandler(w http.ResponseWriter, r *http.Request) a.logger.Info("Blob archiver API request", "url", r.RequestURI) id := mux.Vars(r)["id"] - var indices []uint64 - indicesStr := r.URL.Query().Get("indices") - if indicesStr != "" { - splits := strings.Split(indicesStr, ",") - for _, index := range splits { - parsedInt, err := strconv.ParseUint(index, 10, 64) - if err != nil { - // beacon API will ignore invalid indices and return all blobs - break - } - indices = append(indices, parsedInt) - } - } if hErr := validateBlockID(id); hErr != nil { hErr.write(w) return } + + indices, hErr := parseIndices(r, a.cfg.MaxBlobsPerBlock) + if hErr != nil { + hErr.write(w) + return + } + result, hErr := a.api.queryBlobSidecars(id, indices) if hErr != nil { hErr.write(w) diff --git a/ethstorage/archiver/utils.go b/ethstorage/archiver/utils.go index e04d3574..d0ae3e6f 100644 --- a/ethstorage/archiver/utils.go +++ b/ethstorage/archiver/utils.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "strconv" "strings" @@ -16,7 +17,7 @@ func validateBlockID(id string) *httpError { if isHash(id) || isSlot(id) || isKnownIdentifier(id) { return nil } - return newBlockIdError(id) + return newBadRequestError(fmt.Sprintf("Invalid block ID: %s", id)) } func isHash(s string) bool { @@ -82,9 +83,53 @@ var ( } ) -func newBlockIdError(input string) *httpError { +func newBadRequestError(input string) *httpError { return &httpError{ Code: http.StatusBadRequest, - Message: fmt.Sprintf("Invalid block ID: %s", input), + Message: input, + } +} + +// parseIndices filters out invalid and duplicate blob indices +func parseIndices(r *http.Request, max int) ([]uint64, *httpError) { + query := r.URL.Query() + normalizeQueryValues(query) + r.URL.RawQuery = query.Encode() + rawIndices := r.URL.Query()["indices"] + indices := make([]uint64, 0, max) + invalidIndices := make([]string, 0) +loop: + for _, raw := range rawIndices { + ix, err := strconv.ParseUint(raw, 10, 64) + if err != nil { + invalidIndices = append(invalidIndices, raw) + continue + } + if ix >= uint64(max) { + invalidIndices = append(invalidIndices, raw) + continue + } + for i := range indices { + if ix == indices[i] { + continue loop + } + } + indices = append(indices, ix) + } + + if len(invalidIndices) > 0 { + return nil, newBadRequestError(fmt.Sprintf("requested blob indices %v are invalid", invalidIndices)) + } + return indices, nil +} + +// normalizeQueryValues replaces comma-separated values with individual values +func normalizeQueryValues(queryParams url.Values) { + for key, vals := range queryParams { + splitVals := make([]string, 0) + for _, v := range vals { + splitVals = append(splitVals, strings.Split(v, ",")...) + } + queryParams[key] = splitVals } } diff --git a/ethstorage/archiver/utils_test.go b/ethstorage/archiver/utils_test.go new file mode 100644 index 00000000..b3826694 --- /dev/null +++ b/ethstorage/archiver/utils_test.go @@ -0,0 +1,82 @@ +// Copyright 2022-2023, EthStorage. +// For license information, see https://github.com/ethstorage/es-node/blob/main/LICENSE +package archiver + +import ( + "net/http" + "net/http/httptest" + "reflect" + "testing" +) + +func Test_parseIndices(t *testing.T) { + tests := []struct { + name string + query string + expected []uint64 + expectError *httpError + }{ + { + name: "happy path with comma-separated indices", + query: "indices=0,1,2", + expected: []uint64{0, 1, 2}, + expectError: nil, + }, + { + name: "happy path with repeated indices parameter", + query: "indices=0&indices=1&indices=2", + expected: []uint64{0, 1, 2}, + expectError: nil, + }, + { + name: "happy path with duplicate indices within bound and other query parameters ignored", + query: "indices=1&indices=2&indices=1&indices=3&bar=bar", + expected: []uint64{1, 2, 3}, + expectError: nil, + }, + { + name: "happy path with comma-separated indices within bound and other query parameters ignored", + query: "indices=1,2,3,3&bar=bar", + expected: []uint64{1, 2, 3}, + expectError: nil, + }, + { + name: "invalid indices with comma-separated indices", + query: "indices=1,abc,2&bar=bar", + expected: nil, + expectError: newBadRequestError("requested blob indices [abc] are invalid"), + }, + { + name: "invalid indices with repeated indices", + query: "indices=0&indices=abc&indices=2", + expected: nil, + expectError: newBadRequestError("requested blob indices [abc] are invalid"), + }, + { + name: "out of bounds indices throws error", + query: "indices=2&indices=7", + expected: nil, + expectError: newBadRequestError("requested blob indices [7] are invalid"), + }, + { + name: "negative indices", + query: "indices=-1", + expected: nil, + expectError: newBadRequestError("requested blob indices [-1] are invalid"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/eth/v1/beacon/blob_sidecars/6930317?"+tt.query, nil) + + got, err := parseIndices(req, 6) + if !reflect.DeepEqual(got, tt.expected) { + t.Errorf("parseIndices() got = %v, want %v", got, tt.expected) + } + if !reflect.DeepEqual(err, tt.expectError) { + t.Errorf("parseIndices() got err = %v, want %v", err, tt.expectError) + } + }) + } +}