Skip to content

Commit 2a4ac95

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 d0418b5 commit 2a4ac95

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
@@ -209,9 +209,8 @@ func (d *DynamoDBServer) AdminCreateTable(ctx context.Context, principal AdminPr
209209
if err != nil {
210210
return nil, err
211211
}
212-
if strings.TrimSpace(legacy.TableName) == "" {
213-
return nil, newDynamoAPIError(http.StatusBadRequest, dynamoErrValidation, "missing table name")
214-
}
212+
// buildLegacyCreateTableInput already rejects an empty table
213+
// name; the previous duplicated check here was dead code.
215214
unlock := d.lockTableOperations([]string{legacy.TableName})
216215
defer unlock()
217216
schema, err := buildCreateTableSchema(legacy)

internal/admin/dynamo_handler.go

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

389408
// 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)