apache / apache/airavata-custos

Reject malformed API payloads and invalid domain inputs

Open
#478 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
21
Forks
39
Avg merge
2d 18h
Merged PRs (30d)
6

Description

## Summary

The Custos HTTP/JSON API should reject malformed request bodies and invalid domain values before they reach the database. Today, several client-side input errors are either accepted and persisted or reported as `500 Internal Server Error`, which makes API behavior hard to debug and can leave invalid state in the database.

## Affected Area

- `cmd/server` / `internal/server`: JSON request decoding and HTTP error mapping
- `pkg/service`: domain validation for statuses, time ranges, and numeric amounts
- API documentation for request body expectations and error responses

## Current Behavior

The following cases can be reproduced against a local server:

1. A request body with more than one JSON value is accepted after decoding the first value.

```bash
curl -i -H 'Content-Type: application/json' \
--data-binary '{"name":"Trailing JSON"} {"x":1}' \
http://127.0.0.1:8080/organizations
```

This currently creates an organization instead of returning a client error.

2. A request with a JSON body but no `Content-Type: application/json` header is accepted, even though `docs/API-Docs.md` says the content type is required.

```bash
curl -i --data-binary '{"name":"No Content Type"}' \
http://127.0.0.1:8080/organizations
```

3. Duplicate client-supplied IDs can surface as database duplicate-key errors and return `500 Internal Server Error` instead of `409 Conflict`.

```bash
curl -i -H 'Content-Type: application/json' \
-d '{"id":"bug-org","name":"Bug Org"}' \
http://127.0.0.1:8080/organizations

curl -i -H 'Content-Type: application/json' \
-d '{"id":"bug-org","name":"Bug Org Again"}' \
http://127.0.0.1:8080/organizations
```

4. Unsupported lifecycle status values can be persisted.

```bash
curl -i -X PUT -H 'Content-Type: application/json' \
-d '{"status":"BANANA"}' \
http://127.0.0.1:8080/users//status
```

5. A compute allocation without `start_time` / `end_time` can fail at the MySQL layer with an invalid zero datetime and return `500`.

6. Negative resource or SU amounts can be accepted in service-layer paths where they should be invalid.

## Expected Behavior

- Request bodies must use `Content-Type: application/json` when a body is present.
- JSON decoding should reject unknown fields and trailing JSON values.
- Invalid client input should return `400 Bad Request` with the existing JSON error shape.
- Duplicate keys / unique conflicts should return `409 Conflict`.
- Unsupported status values should be rejected instead of persisted.
- Required time ranges should be validated before database writes.
- Resource and SU amount fields should reject negative values.
- API documentation should match the enforced request-body behavior.

## Why This Matters

These cases are client input problems, not server failures. Returning `500` hides the actionable cause from API clients, and accepting invalid statuses or negative amounts can corrupt allocation/accounting data used by downstream connectors.

## Proposed Fix

- Make request decoding require `application/json` and reject trailing JSON values.
- Translate duplicate-key and invalid constraint database errors to the existing service-level HTTP semantics where appropriate.
- Add service-layer validation helpers for supported statuses, required time ranges, and non-negative amounts.
- Add focused tests for request decoding and validation behavior.
- Update API documentation to describe the stricter behavior.

## Verification

The fix should pass the repository-required checks from `CONTRIBUTING.md`:

```bash
go build ./...
go vet ./...
go test ./...
```

A proposed fix is available in #479.

Contributor guide

Open the contributing guide

Research direction

Start with the JSON decoding and HTTP error mapping in cmd/server and internal/server, then inspect the validation paths in pkg/service. Review docs/API-Docs.md and the focused tests proposed for request decoding, status, time-range, and amount validation. Done means the listed invalid inputs return the expected 400 or 409 responses and go build ./..., go vet ./..., and go test ./... pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, mysql
Domain
api, backend, databases, documentation, testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.