Properly make use of the Allow header#1020
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: This PR ensures HTTP 405 responses include an Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| response.write_header("Access-Control-Allow-Origin", "*"); | ||
| // RFC 9110 §15.5.6: 405 responses MUST carry Allow listing supported methods. | ||
| // https://datatracker.ietf.org/doc/html/rfc9110#section-15.5.6 | ||
| if (!allow.empty()) { |
There was a problem hiding this comment.
src/http/include/sourcemeta/one/http_helpers.h:69: json_error() writes an Allow header whenever allow is non-empty, even if code isn’t 405. Consider guarding this on code == STATUS_METHOD_NOT_ALLOWED to avoid accidentally emitting an Allow header on non-405 error responses.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| "This schema was not precompiled for schema evaluation", | ||
| error_schema); | ||
| "This schema was not precompiled for schema evaluation", error_schema, | ||
| "POST, OPTIONS"); |
There was a problem hiding this comment.
src/actions/action_jsonschema_evaluate_v1.h:173-177: in the no-schema-template branch, the response is 405 but the Allow header includes POST even though a POST to that resource will still hit this branch and return 405. This makes the Allow header inaccurate for this target resource (RFC 9110).
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // RFC 9110 §15.5.6: 405 responses MUST carry Allow listing supported | ||
| // methods. The MCP endpoint accepts POST and OPTIONS only. | ||
| // https://datatracker.ietf.org/doc/html/rfc9110#section-15.5.6 | ||
| if (status == sourcemeta::one::STATUS_METHOD_NOT_ALLOWED) { |
There was a problem hiding this comment.
src/actions/action_mcp_v1.h:267: status is a const char*, so status == STATUS_METHOD_NOT_ALLOWED is comparing pointer addresses, not the status value. If an equivalent status string ever comes from a different storage, this check will fail and the 405 response may miss the required Allow header.
Severity: low
Other Locations
enterprise/server/include/sourcemeta/one/enterprise_server_actions.h:187
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 38 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark Index (community)
Details
| Benchmark suite | Current: b88d4b5 | Previous: 33ae7a4 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
401 ms |
404 ms |
0.99 |
Add one schema (100 existing) |
30 ms |
29 ms |
1.03 |
Add one schema (1000 existing) |
92 ms |
89 ms |
1.03 |
Add one schema (10000 existing) |
734 ms |
703 ms |
1.04 |
Update one schema (1 existing) |
22 ms |
22 ms |
1 |
Update one schema (101 existing) |
31 ms |
30 ms |
1.03 |
Update one schema (1001 existing) |
94 ms |
86 ms |
1.09 |
Update one schema (10001 existing) |
760 ms |
722 ms |
1.05 |
Cached rebuild (1 existing) |
7 ms |
6 ms |
1.17 |
Cached rebuild (101 existing) |
9 ms |
8 ms |
1.13 |
Cached rebuild (1001 existing) |
32 ms |
29 ms |
1.10 |
Cached rebuild (10001 existing) |
272 ms |
251 ms |
1.08 |
Index 100 schemas |
670 ms |
505 ms |
1.33 |
Index 1000 schemas |
1508 ms |
1607 ms |
0.94 |
Index 10000 schemas |
13606 ms |
14311 ms |
0.95 |
Index 10000 schemas (custom meta-schema) |
134308 ms |
147757 ms |
0.91 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark Index (enterprise)
Details
| Benchmark suite | Current: b88d4b5 | Previous: 33ae7a4 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
406 ms |
295 ms |
1.38 |
Add one schema (100 existing) |
31 ms |
26 ms |
1.19 |
Add one schema (1000 existing) |
92 ms |
84 ms |
1.10 |
Add one schema (10000 existing) |
899 ms |
791 ms |
1.14 |
Update one schema (1 existing) |
24 ms |
19 ms |
1.26 |
Update one schema (101 existing) |
33 ms |
25 ms |
1.32 |
Update one schema (1001 existing) |
91 ms |
90 ms |
1.01 |
Update one schema (10001 existing) |
719 ms |
1024 ms |
0.70 |
Cached rebuild (1 existing) |
7 ms |
6 ms |
1.17 |
Cached rebuild (101 existing) |
9 ms |
8 ms |
1.13 |
Cached rebuild (1001 existing) |
29 ms |
26 ms |
1.12 |
Cached rebuild (10001 existing) |
246 ms |
237 ms |
1.04 |
Index 100 schemas |
649 ms |
489 ms |
1.33 |
Index 1000 schemas |
1654 ms |
1305 ms |
1.27 |
Index 10000 schemas |
13768 ms |
12711 ms |
1.08 |
Index 10000 schemas (custom meta-schema) |
142530 ms |
107540 ms |
1.33 |
This comment was automatically generated by workflow using github-action-benchmark.
1 participant
Signed-off-by: Juan Cruz Viotti jv@jviotti.com