Skip to content

Commit 4c4956e

Browse files
committed
admin: address Gemini + Codex review (write endpoints)
- adapter/dynamodb_admin.go: drop the redundant empty-table-name check in AdminCreateTable. buildLegacyCreateTableInput already rejects an empty TableName, so the second guard was dead code that just shifted the error path. - internal/admin/dynamo_handler.go: reject trailing JSON tokens after the create-table body (Codex P2). `decodeCreateTableRequest` now calls dec.More() after the first Decode() and returns 400 if the client sent additional values; previously a `{...}{garbage}` body silently used the first object and ignored the rest. - Split out validateCreateTableRequest so the decoder + validator each stay under cyclop=10 after adding the dec.More() check. - Tests: extend TestDynamoHandler_CreateTable_RejectsBadJSON to cover both trailing JSON object and trailing scalar.
1 parent 1f65c91 commit 4c4956e

3 files changed

Lines changed: 34 additions & 14 deletions

File tree

adapter/dynamodb_admin.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,8 @@ func (d *DynamoDBServer) AdminCreateTable(ctx context.Context, principal AdminPr
201201
if err != nil {
202202
return nil, err
203203
}
204-
if strings.TrimSpace(legacy.TableName) == "" {
205-
return nil, newDynamoAPIError(http.StatusBadRequest, dynamoErrValidation, "missing table name")
206-
}
204+
// buildLegacyCreateTableInput already rejects an empty table
205+
// name; the previous duplicated check here was dead code.
207206
unlock := d.lockTableOperations([]string{legacy.TableName})
208207
defer unlock()
209208
schema, err := buildCreateTableSchema(legacy)

internal/admin/dynamo_handler.go

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -361,23 +361,42 @@ func decodeCreateTableRequest(body io.Reader) (CreateTableRequest, error) {
361361
}
362362
return CreateTableRequest{}, errors.New("request body is not valid JSON")
363363
}
364-
if strings.TrimSpace(out.TableName) == "" {
365-
return CreateTableRequest{}, errors.New("table_name is required")
366-
}
367-
if err := validateAttribute(out.PartitionKey, "partition_key"); err != nil {
364+
// Reject trailing JSON tokens — `{"table_name":"a", ...}{...}`
365+
// must surface as 400, not silently accept the first object and
366+
// drop the rest. dec.More() returns true when there is at least
367+
// one more JSON value in the stream beyond the one we just
368+
// decoded.
369+
if dec.More() {
370+
return CreateTableRequest{}, errors.New("request body has trailing data after the JSON object")
371+
}
372+
if err := validateCreateTableRequest(&out); err != nil {
368373
return CreateTableRequest{}, err
369374
}
370-
if out.SortKey != nil {
371-
if err := validateAttribute(*out.SortKey, "sort_key"); err != nil {
372-
return CreateTableRequest{}, err
375+
return out, nil
376+
}
377+
378+
// validateCreateTableRequest is the field-level validation pass
379+
// kept separate from the JSON decoding so each function stays under
380+
// the project's cyclomatic-complexity ceiling and the decoder is
381+
// trivially auditable on its own.
382+
func validateCreateTableRequest(in *CreateTableRequest) error {
383+
if strings.TrimSpace(in.TableName) == "" {
384+
return errors.New("table_name is required")
385+
}
386+
if err := validateAttribute(in.PartitionKey, "partition_key"); err != nil {
387+
return err
388+
}
389+
if in.SortKey != nil {
390+
if err := validateAttribute(*in.SortKey, "sort_key"); err != nil {
391+
return err
373392
}
374393
}
375-
for i := range out.GSI {
376-
if err := validateGSI(&out.GSI[i], i); err != nil {
377-
return CreateTableRequest{}, err
394+
for i := range in.GSI {
395+
if err := validateGSI(&in.GSI[i], i); err != nil {
396+
return err
378397
}
379398
}
380-
return out, nil
399+
return nil
381400
}
382401

383402
// validateAttribute enforces the "S | N | B" rule for primary-key

internal/admin/dynamo_handler_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,8 @@ func TestDynamoHandler_CreateTable_RejectsBadJSON(t *testing.T) {
438438
`{"table_name":"u","partition_key":{"name":"id","type":"X"}}`, // bad type
439439
`{"table_name":"u","partition_key":{"name":"id","type":"S"},"sort_key":{"name":"","type":"S"}}`, // bad sort key
440440
`{"table_name":"u","partition_key":{"name":"id","type":"S"},"unknown_field":1}`, // strict decode
441+
`{"table_name":"u","partition_key":{"name":"id","type":"S"}}{"second":"object"}`, // trailing JSON
442+
`{"table_name":"u","partition_key":{"name":"id","type":"S"}} 42`, // trailing scalar
441443
}
442444
for _, body := range cases {
443445
t.Run(body, func(t *testing.T) {

0 commit comments

Comments
 (0)