fix repeated params

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

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)

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
 	}
 }

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)
+			}
+		})
+	}
+}