mirror of
https://github.com/luxfi/graph.git
synced 2026-09-24 00:26:07 +00:00
Let a page ask for as many tables as it needs
The Explore transactions view requests 23 tables in one query and got "too many top-level fields (23), maximum 20" — the whole tab died on it. 20 was a bare literal at the check while every neighbouring limit lives on Config. Asking for several tables at once is ordinary, not abusive: the cap exists to bound work per request, so it belongs with the other limits and starts high enough for real pages. A zero value means unset, not zero allowed, so configs written before this field keep working. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
+17
-3
@@ -46,10 +46,24 @@ import (
|
||||
// Config controls query execution limits.
|
||||
type Config struct {
|
||||
MaxQueryDepth int `json:"maxQueryDepth" yaml:"max_query_depth"`
|
||||
MaxTopFields int `json:"maxTopFields" yaml:"max_top_fields"`
|
||||
MaxResultSize int `json:"maxResultSize" yaml:"max_result_size"`
|
||||
QueryTimeoutMs int `json:"queryTimeoutMs" yaml:"query_timeout_ms"`
|
||||
}
|
||||
|
||||
// A page that asks for several tables at once is ordinary, not abusive: the
|
||||
// Explore transactions view alone requests 23. The cap exists to bound work per
|
||||
// request, so it sits with the other limits rather than as a literal at the
|
||||
// check.
|
||||
const defaultMaxTopFields = 64
|
||||
|
||||
func (e *Engine) maxTopFields() int {
|
||||
if e.config == nil || e.config.MaxTopFields <= 0 {
|
||||
return defaultMaxTopFields
|
||||
}
|
||||
return e.config.MaxTopFields
|
||||
}
|
||||
|
||||
// Engine executes GraphQL queries against indexed data.
|
||||
type Engine struct {
|
||||
store *storage.Store
|
||||
@@ -93,7 +107,7 @@ type Error struct {
|
||||
// New creates a query engine backed by the given store.
|
||||
func New(store *storage.Store, cfg *Config) *Engine {
|
||||
if cfg == nil {
|
||||
cfg = &Config{MaxQueryDepth: 10, MaxResultSize: 1 << 20, QueryTimeoutMs: 30000}
|
||||
cfg = &Config{MaxQueryDepth: 10, MaxTopFields: defaultMaxTopFields, MaxResultSize: 1 << 20, QueryTimeoutMs: 30000}
|
||||
}
|
||||
|
||||
e := &Engine{
|
||||
@@ -289,8 +303,8 @@ func (e *Engine) Execute(ctx context.Context, req *Request) *Response {
|
||||
return &Response{Errors: []Error{{Message: err.Error()}}}
|
||||
}
|
||||
|
||||
if len(fields) > 20 {
|
||||
return &Response{Errors: []Error{{Message: fmt.Sprintf("too many top-level fields (%d), maximum 20", len(fields))}}}
|
||||
if max := e.maxTopFields(); len(fields) > max {
|
||||
return &Response{Errors: []Error{{Message: fmt.Sprintf("too many top-level fields (%d), maximum %d", len(fields), max)}}}
|
||||
}
|
||||
|
||||
data := make(map[string]interface{})
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
package engine
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/luxfi/graph/storage"
|
||||
)
|
||||
|
||||
// The Explore transactions view asks for 23 tables in one request and got back
|
||||
// "too many top-level fields (23), maximum 20" — the page died on a limit that
|
||||
// was a bare literal while every neighbouring limit was configurable.
|
||||
func query(n int) string {
|
||||
// One field per line, the shape Apollo actually sends — parseTopFields
|
||||
// splits on newlines, so a single-line query reads as one field.
|
||||
var b strings.Builder
|
||||
b.WriteString("{\n")
|
||||
for i := 0; i < n; i++ {
|
||||
fmt.Fprintf(&b, " f%d { id }\n", i)
|
||||
}
|
||||
b.WriteString("}")
|
||||
return b.String()
|
||||
}
|
||||
|
||||
func newTestEngine(t *testing.T, cfg *Config, fields int) *Engine {
|
||||
t.Helper()
|
||||
e := New(nil, cfg)
|
||||
for i := 0; i < fields; i++ {
|
||||
e.RegisterResolver(fmt.Sprintf("f%d", i), func(_ context.Context, _ *storage.Store, _ map[string]interface{}) (interface{}, error) {
|
||||
return []interface{}{}, nil
|
||||
})
|
||||
}
|
||||
return e
|
||||
}
|
||||
|
||||
func errText(r *Response) string {
|
||||
if len(r.Errors) == 0 {
|
||||
return ""
|
||||
}
|
||||
return r.Errors[0].Message
|
||||
}
|
||||
|
||||
func TestTopFields_RealPageIsNotRejected(t *testing.T) {
|
||||
e := newTestEngine(t, nil, 23)
|
||||
if got := errText(e.Execute(context.Background(), &Request{Query: query(23)})); strings.Contains(got, "too many top-level fields") {
|
||||
t.Fatalf("23 fields rejected by default: %s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTopFields_CapStillApplies(t *testing.T) {
|
||||
e := newTestEngine(t, &Config{MaxQueryDepth: 10, MaxTopFields: 5, MaxResultSize: 1 << 20, QueryTimeoutMs: 30000}, 8)
|
||||
got := errText(e.Execute(context.Background(), &Request{Query: query(8)}))
|
||||
if !strings.Contains(got, "too many top-level fields (8), maximum 5") {
|
||||
t.Fatalf("configured cap not enforced, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTopFields_UnsetConfigFallsBackToDefault(t *testing.T) {
|
||||
// A zero value means "unset", not "reject everything" — a config written
|
||||
// before this field existed must not brick every query.
|
||||
e := newTestEngine(t, &Config{MaxQueryDepth: 10, MaxResultSize: 1 << 20, QueryTimeoutMs: 30000}, 23)
|
||||
if got := errText(e.Execute(context.Background(), &Request{Query: query(23)})); strings.Contains(got, "too many top-level fields") {
|
||||
t.Fatalf("zero MaxTopFields rejected a normal query: %s", got)
|
||||
}
|
||||
if e.maxTopFields() != defaultMaxTopFields {
|
||||
t.Fatalf("expected default %d, got %d", defaultMaxTopFields, e.maxTopFields())
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user