Skip to content

Commit c1ef21d

Browse files
danielgerlagCopilot
andcommitted
Address PR review feedback on JSON/YAML request body support
- Preserve the inner Bytes extractor status code (e.g. 413) on body-read failures instead of forcing 400, and make ConfigBody's rejection type a consistent (StatusCode, Json<ErrorResponse>) tuple across all branches. - Move raw serde parse errors out of ErrorResponse.message into ErrorDetail::technical_details, per the project error convention. - Reject YAML anchor/alias expansion ('billion laughs') before deserializing. - Move ConfigBody and helpers out of error.rs into a dedicated src/api/shared/extractor.rs module; re-export from shared. - Keep push_source_data (a data-proxy endpoint) on axum::Json to avoid silent YAML->JSON coercion. - README: list all five accepted YAML content types. - CLAUDE.md: document ConfigBody usage and the data-plane exception. - Tests: fix TOCTOU port race (bind port 0 once), narrow the module docstring, and add coverage for application/x-yaml and text/vnd.yaml, the missing-Content-Type JSON default, malformed JSON, anchor/alias rejection, source/reaction YAML wiring, plus is_yaml_content_type and anchor-scanner unit tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d1223c4 commit c1ef21d

18 files changed

Lines changed: 512 additions & 114 deletions

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ This repository contains only the server wrapper functionality:
5959
- `v1/` - API version 1 handlers, routes, and OpenAPI spec
6060
- `v1/plugin_handlers.rs` - Plugin management API endpoints
6161
- `shared/` - Common handlers, error types, and response types shared across versions
62+
- `shared/extractor.rs` - `ConfigBody<T>` request-body extractor (JSON/YAML content negotiation). New handlers that accept a config request body should use `ConfigBody<T>` rather than `axum::Json<T>` so JSON and YAML are accepted interchangeably. Data-plane proxy endpoints (e.g. `push_source_data`) stay on `axum::Json` to avoid silent YAML→JSON coercion.
6263
- `version.rs` - API version constants and utilities
6364
- `models/` - Data Transfer Objects (DTOs)
6465
- `mappings/` - DTO to domain model conversions

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,8 +1662,8 @@ The server exposes a REST API at `http://localhost:8080` (default). For complete
16621662
Every endpoint that accepts a request body accepts **both JSON and YAML** payloads
16631663
interchangeably. The format is selected from the request's `Content-Type` header:
16641664
send `application/json` (the default) for JSON, or one of `application/yaml`,
1665-
`application/x-yaml`, or `text/yaml` for YAML. When no `Content-Type` is provided,
1666-
the body is parsed as JSON.
1665+
`application/x-yaml`, `text/yaml`, `text/x-yaml`, or `text/vnd.yaml` for YAML.
1666+
When no `Content-Type` is provided, the body is parsed as JSON.
16671667

16681668
```bash
16691669
# Create a query using a YAML body

src/api/joins_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ mod api_query_joins_tests {
2020
use crate::api::shared::handlers::*;
2121
use crate::persistence::ConfigPersistence;
2222
use async_trait::async_trait;
23-
use crate::api::shared::error::ConfigBody;
23+
use crate::api::shared::extractor::ConfigBody;
2424
use axum::Extension;
2525
use drasi_lib::{
2626
channels::{

src/api/shared/error.rs

Lines changed: 1 addition & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -29,95 +29,12 @@
2929
//! - **Layer 3 (drasi-lib):** Returns `DrasiError` which is converted to
3030
//! `ErrorResponse` via `From<DrasiError>` with proper status code mapping.
3131
32-
use axum::async_trait;
33-
use axum::body::Bytes;
34-
use axum::extract::FromRequest;
35-
use axum::http::header::CONTENT_TYPE;
3632
use axum::http::StatusCode;
3733
use axum::response::IntoResponse;
3834
use drasi_lib::DrasiError;
39-
use serde::{de::DeserializeOwned, Serialize};
35+
use serde::Serialize;
4036
use utoipa::ToSchema;
4137

42-
/// A request-body extractor that accepts both JSON and YAML payloads.
43-
///
44-
/// The body format is selected from the request's `Content-Type` header:
45-
/// YAML media types (`application/yaml`, `application/x-yaml`, `text/yaml`,
46-
/// `text/x-yaml`, `text/vnd.yaml`) are parsed with `serde_yaml`; everything
47-
/// else (including a missing `Content-Type`) defaults to JSON. This lets every
48-
/// HTTP route on the API accept JSON and YAML interchangeably.
49-
///
50-
/// On failure it returns a structured `ErrorResponse` with the serde error
51-
/// details included (HTTP 400 via the `INVALID_REQUEST` code).
52-
#[derive(Debug, Clone, Copy, Default)]
53-
pub struct ConfigBody<T>(pub T);
54-
55-
/// Returns `true` when the supplied `Content-Type` value denotes a YAML media type.
56-
fn is_yaml_content_type(content_type: &str) -> bool {
57-
// Ignore any parameters (e.g. "; charset=utf-8") and surrounding whitespace.
58-
let essence = content_type
59-
.split(';')
60-
.next()
61-
.unwrap_or("")
62-
.trim()
63-
.to_ascii_lowercase();
64-
matches!(
65-
essence.as_str(),
66-
"application/yaml"
67-
| "application/x-yaml"
68-
| "text/yaml"
69-
| "text/x-yaml"
70-
| "text/vnd.yaml"
71-
)
72-
}
73-
74-
#[async_trait]
75-
impl<T, S> FromRequest<S> for ConfigBody<T>
76-
where
77-
T: DeserializeOwned,
78-
S: Send + Sync,
79-
{
80-
type Rejection = ErrorResponse;
81-
82-
async fn from_request(
83-
req: axum::http::Request<axum::body::Body>,
84-
state: &S,
85-
) -> Result<Self, Self::Rejection> {
86-
let is_yaml = req
87-
.headers()
88-
.get(CONTENT_TYPE)
89-
.and_then(|value| value.to_str().ok())
90-
.map(is_yaml_content_type)
91-
.unwrap_or(false);
92-
93-
let bytes = Bytes::from_request(req, state).await.map_err(|rejection| {
94-
log::debug!("Failed to read request body: {}", rejection.body_text());
95-
ErrorResponse::new(
96-
error_codes::INVALID_REQUEST,
97-
"Failed to read request body".to_string(),
98-
)
99-
})?;
100-
101-
if is_yaml {
102-
serde_yaml::from_slice(&bytes).map(ConfigBody).map_err(|e| {
103-
log::debug!("YAML extraction failed: {e}");
104-
ErrorResponse::new(
105-
error_codes::INVALID_REQUEST,
106-
format!("Failed to parse YAML request body: {e}"),
107-
)
108-
})
109-
} else {
110-
serde_json::from_slice(&bytes).map(ConfigBody).map_err(|e| {
111-
log::debug!("JSON extraction failed: {e}");
112-
ErrorResponse::new(
113-
error_codes::INVALID_REQUEST,
114-
format!("Failed to parse JSON request body: {e}"),
115-
)
116-
})
117-
}
118-
}
119-
}
120-
12138
/// Error codes for API responses
12239
pub mod error_codes {
12340
pub const SOURCE_CREATE_FAILED: &str = "SOURCE_CREATE_FAILED";

0 commit comments

Comments
 (0)