Skip to content

Commit 19b260c

Browse files
committed
Check java buffer
1 parent 35c311b commit 19b260c

17 files changed

Lines changed: 415 additions & 86 deletions

File tree

crates/vespera/src/multipart.rs

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,13 @@ impl TypedMultipartError {
171171
impl IntoResponse for TypedMultipartError {
172172
fn into_response(self) -> Response {
173173
let status = match &self {
174-
Self::InvalidRequest { .. }
175-
| Self::InvalidRequestBody { .. }
176-
| Self::MissingField { .. }
174+
// Preserve the SOURCE rejection / stream status so an over-limit
175+
// multipart body surfaces as `413 Payload Too Large` (axum's body
176+
// limit), an unsupported media type as `415`, etc. — instead of
177+
// collapsing every transport-level failure to a generic `400`.
178+
Self::InvalidRequest { source } => source.status(),
179+
Self::InvalidRequestBody { source } => source.status(),
180+
Self::MissingField { .. }
177181
| Self::DuplicateField { .. }
178182
| Self::UnknownField { .. }
179183
| Self::InvalidEnumValue { .. }
@@ -185,16 +189,40 @@ impl IntoResponse for TypedMultipartError {
185189
Self::FieldTooLarge { .. } => StatusCode::PAYLOAD_TOO_LARGE,
186190
Self::Other { .. } => StatusCode::INTERNAL_SERVER_ERROR,
187191
};
188-
// Canonical JSON error envelope — the SAME shape `Validated<T>`
189-
// emits ({"errors":[{"path","message"}]}), so multipart failures are
190-
// consumed uniformly across the API instead of as ad-hoc plain text;
191-
// under JNI a 422 body is additionally hoisted into the wire header
192-
// exactly like a `Validated` rejection. `path` is the offending
193-
// field name when known, else empty.
194-
let path = self.field_name().unwrap_or("").to_owned();
192+
// Canonical JSON error envelope, byte-identical to `Validated<T>`'s
193+
// 422 envelope — `{"errors":[{"message":...,"path":...}]}` (message
194+
// before path) — so multipart failures are consumed uniformly and,
195+
// under JNI, the 422 body hoists into the wire header exactly like a
196+
// `Validated` rejection. Serialized through a borrowing `Serialize`
197+
// (no `serde_json::Value` map/array/object intermediate). `path` is
198+
// the offending field name when known, else empty.
199+
#[derive(serde::Serialize)]
200+
struct OneError<'a> {
201+
message: &'a str,
202+
path: &'a str,
203+
}
204+
#[derive(serde::Serialize)]
205+
struct Envelope<'a> {
206+
errors: [OneError<'a>; 1],
207+
}
208+
let path = self.field_name().unwrap_or("");
195209
let message = self.response_message();
196-
let body = serde_json::json!({ "errors": [{ "path": path, "message": message }] });
197-
(status, axum::Json(body)).into_response()
210+
let body = serde_json::to_vec(&Envelope {
211+
errors: [OneError {
212+
message: &message,
213+
path,
214+
}],
215+
})
216+
.expect("multipart error envelope is infallible to serialize");
217+
(
218+
status,
219+
[(
220+
axum::http::header::CONTENT_TYPE,
221+
axum::http::HeaderValue::from_static("application/json"),
222+
)],
223+
body,
224+
)
225+
.into_response()
198226
}
199227
}
200228

crates/vespera_core/src/openapi.rs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -286,20 +286,16 @@ impl OpenApi {
286286
self.external_docs = other.external_docs;
287287
}
288288

289-
// Merge tags, de-duplicating by name in a single pass. `seen` starts
290-
// with the existing tag names and grows as incoming tags are appended,
291-
// so an incoming tag is kept only when its name collides with neither
292-
// an existing tag nor an already-appended incoming one (first-wins,
293-
// incoming insertion order preserved). A name is cloned only when the
294-
// tag is actually kept — a duplicate is detected by borrow and skipped
295-
// without cloning.
289+
// Merge tags, de-duplicating by name in a single pass with first-wins
290+
// semantics (existing tags and already-appended incoming tags both
291+
// win; incoming insertion order preserved). Tag lists are tiny, so a
292+
// linear membership scan over `self_tags` beats a `HashSet` here: it
293+
// allocates nothing and clones nothing — the kept tag is *moved* in,
294+
// and a duplicate is detected by borrow and skipped.
296295
if let Some(other_tags) = other.tags {
297296
let self_tags = self.tags.get_or_insert_with(Vec::new);
298-
let mut seen: std::collections::HashSet<String> =
299-
self_tags.iter().map(|tag| tag.name.clone()).collect();
300297
for tag in other_tags {
301-
if !seen.contains(tag.name.as_str()) {
302-
seen.insert(tag.name.clone());
298+
if !self_tags.iter().any(|existing| existing.name == tag.name) {
303299
self_tags.push(tag);
304300
}
305301
}

crates/vespera_core/src/schema.rs

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,20 @@ pub struct Schema {
191191
serialize_with = "serialize_number_constraint"
192192
)]
193193
pub maximum: Option<f64>,
194-
/// Exclusive minimum
194+
/// Exclusive minimum.
195+
///
196+
/// NOTE: currently modeled as the OpenAPI 3.0 / draft-04 **boolean
197+
/// flag** (paired with `minimum`). Migrating this to the JSON Schema
198+
/// 2020-12 / OpenAPI 3.1 **numeric** form is tracked as a deliberate,
199+
/// breaking spec-conformance change (it alters generated output and the
200+
/// `#[schema(exclusive_minimum)]` attribute semantics) — see the 3.1
201+
/// conformance decision, not done here to avoid a half-migrated model.
195202
#[serde(skip_serializing_if = "Option::is_none")]
196203
pub exclusive_minimum: Option<bool>,
197-
/// Exclusive maximum
204+
/// Exclusive maximum.
205+
///
206+
/// See [`Schema::exclusive_minimum`]: still the OpenAPI 3.0 boolean
207+
/// flag, pending the bundled strict-3.1 conformance migration.
198208
#[serde(skip_serializing_if = "Option::is_none")]
199209
pub exclusive_maximum: Option<bool>,
200210
/// Multiple of
@@ -411,17 +421,23 @@ impl Schema {
411421
Ok(schema) => schema,
412422
Err(e) => {
413423
// Surface the (in-practice-unreachable) macro/serde drift in
414-
// debug / CI builds while degrading gracefully in release.
415-
// `debug_assert!` keeps `e` referenced in both profiles (its
416-
// release expansion is a dead `if false` branch), so there is
417-
// no unused-binding warning.
424+
// debug / CI builds via `debug_assert!`. In release, degrade
425+
// to a VISIBLE sentinel schema (a description-only object)
426+
// rather than a silent `Schema::default()`, so a drift never
427+
// disappears unnoticed from the generated spec yet never
428+
// panics in downstream user code.
418429
debug_assert!(
419430
false,
420431
"vespera: Schema::from_compiled_json failed to parse macro-emitted \
421-
JSON ({e}); falling back to Schema::default(). This indicates a \
432+
JSON ({e}); emitting a sentinel schema. This indicates a \
422433
vespera bug — the macro serialized a Schema that cannot round-trip."
423434
);
424-
Self::default()
435+
Self {
436+
description: Some(format!(
437+
"vespera: schema unavailable — macro/serde drift ({e})"
438+
)),
439+
..Self::default()
440+
}
425441
}
426442
}
427443
}
@@ -489,6 +505,10 @@ pub enum SecuritySchemeType {
489505
/// `mutualTls` the container rule would produce).
490506
#[serde(rename = "mutualTLS")]
491507
MutualTls,
508+
/// OpenAPI's canonical wire name is `oauth2`; the `camelCase` container
509+
/// rule would otherwise lowercase only the leading char and emit the
510+
/// invalid `oAuth2`.
511+
#[serde(rename = "oauth2")]
492512
OAuth2,
493513
OpenIdConnect,
494514
}
@@ -662,6 +682,38 @@ mod tests {
662682
);
663683
}
664684

685+
// ── CORE: OpenAPI 3.1 conformance of the schema model ────────────
686+
687+
#[test]
688+
fn oauth2_security_scheme_serializes_to_canonical_lowercase() {
689+
// OpenAPI's canonical wire name is `oauth2`. serde's `camelCase`
690+
// container rule lowercases only the leading char, which would emit
691+
// the invalid `oAuth2` without the explicit `#[serde(rename)]`.
692+
let json = serde_json::to_string(&SecuritySchemeType::OAuth2).unwrap();
693+
assert_eq!(json, "\"oauth2\"", "must be exactly \"oauth2\"");
694+
}
695+
696+
#[rstest]
697+
#[case(SecuritySchemeType::ApiKey, "\"apiKey\"")]
698+
#[case(SecuritySchemeType::Http, "\"http\"")]
699+
#[case(SecuritySchemeType::MutualTls, "\"mutualTLS\"")]
700+
#[case(SecuritySchemeType::OAuth2, "\"oauth2\"")]
701+
#[case(SecuritySchemeType::OpenIdConnect, "\"openIdConnect\"")]
702+
fn security_scheme_type_uses_openapi_canonical_wire_names(
703+
#[case] ty: SecuritySchemeType,
704+
#[case] expected: &str,
705+
) {
706+
assert_eq!(serde_json::to_string(&ty).unwrap(), expected);
707+
}
708+
709+
#[test]
710+
#[should_panic(expected = "from_compiled_json failed to parse")]
711+
fn from_compiled_json_invalid_input_trips_debug_assert() {
712+
// In debug / test builds the (in-practice-unreachable) macro/serde
713+
// drift guard fires loudly so a bug never goes unnoticed in CI.
714+
let _ = Schema::from_compiled_json("{not valid json");
715+
}
716+
665717
// ── CORE-04: typed `additionalProperties` (untagged) ─────────────
666718
//
667719
// The untagged enum MUST serialize to the bare JSON Schema wire form

crates/vespera_inprocess/src/internal.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,13 @@ where
292292
&& !data.is_empty()
293293
&& on_chunk(data.as_ref()).is_break()
294294
{
295-
break;
295+
// The chunk sink asked to stop EARLY (e.g. the host's
296+
// OutputStream failed mid-stream). The bytes already
297+
// delivered are truncated, so surface a 500 — exactly
298+
// like the body-error arm below — instead of falling
299+
// through to the original success header, which would
300+
// report a short, truncated response as a clean success.
301+
return Err((500, "response body sink stopped before completion".to_owned()));
296302
}
297303
}
298304
Some(Err(_)) => {

crates/vespera_inprocess/src/streaming.rs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,26 @@ where
379379
C: FnOnce(),
380380
{
381381
let mut header_bytes: Vec<u8> = Vec::with_capacity(4 + WIRE_HEADER_RESERVE);
382-
{
382+
let outcome = {
383383
let on_header = |h: &[u8]| header_bytes.extend_from_slice(h);
384384
bidirectional_streaming_inner(input_header, pull_chunk, on_chunk, on_header, request_close)
385-
.await;
385+
.await
386+
};
387+
match outcome {
388+
// `Complete` covers a clean drain AND the pre-dispatch error paths
389+
// (which deliver a full `error_wire(...)` via `on_header`), so the
390+
// captured bytes are authoritative.
391+
StreamOutcome::Complete => header_bytes,
392+
// The response body errored, or the chunk sink stopped, AFTER the
393+
// success header was captured into `header_bytes` — the delivered
394+
// body is truncated. Replace the captured success header with a 500
395+
// so a truncated bidirectional response is never returned as a clean
396+
// success (mirrors `dispatch_streaming_async`).
397+
StreamOutcome::BodyError => error_wire(500, "response body stream error"),
398+
StreamOutcome::SinkStopped => {
399+
error_wire(500, "response body sink stopped before completion")
400+
}
386401
}
387-
header_bytes
388402
}
389403

390404
/// **Bidirectional streaming with explicit header callback** — the
@@ -725,7 +739,20 @@ fn spawn_request_producer(
725739
// handler aborted mid-stream, so we stop pulling.
726740
let mut consecutive_empty: u32 = 0;
727741
loop {
728-
match pull() {
742+
// A panic inside the user / JNI-supplied `pull()` must NOT be
743+
// turned into a clean end-of-stream — that would accept a
744+
// TRUNCATED upload as a complete request body (silent data
745+
// loss). Catch it and forward a `StreamAbort`, exactly like the
746+
// explicit `RequestChunk::Error` path, so axum/the handler
747+
// rejects the body instead of seeing a short, "successful" read.
748+
let next = match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| pull())) {
749+
Ok(next) => next,
750+
Err(_) => {
751+
let _ = tx.blocking_send(Err(StreamAbort));
752+
break;
753+
}
754+
};
755+
match next {
729756
RequestChunk::Data(chunk) => {
730757
if chunk.is_empty() {
731758
// A conformant blocking `InputStream.read(byte[])`

crates/vespera_inprocess/tests/streaming_with_header.rs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,13 @@ async fn streaming_with_header_chunk_break_returns_sink_stopped_outcome() {
10441044
}
10451045

10461046
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
1047-
async fn response_streaming_stops_draining_when_chunk_callback_breaks() {
1047+
async fn response_streaming_chunk_break_returns_500_not_silent_success() {
1048+
// When the chunk sink returns `Break` (the host output sink failed
1049+
// mid-stream), the non-header `dispatch_streaming_async` must surface a
1050+
// 500 — NOT the original success header — so a TRUNCATED response is never
1051+
// reported as a clean success. (Mirrors the header-first
1052+
// `...sink_stopped_outcome` and direct-write
1053+
// `...body_error_yields_500_not_truncated_success` contracts.)
10481054
install_router();
10491055
let wire = encode_wire("GET", "/multi-chunk", HashMap::new(), &[]);
10501056
let body_buf: Arc<Mutex<Vec<u8>>> = Arc::new(Mutex::new(Vec::new()));
@@ -1056,12 +1062,39 @@ async fn response_streaming_stops_draining_when_chunk_callback_breaks() {
10561062
})
10571063
.await;
10581064

1059-
let (header_json, header_body) = decode_wire(&header);
1060-
assert_eq!(header_json["status"].as_u64(), Some(200));
1061-
assert!(header_body.is_empty());
1065+
let (header_json, _header_body) = decode_wire(&header);
1066+
assert_eq!(
1067+
header_json["status"].as_u64(),
1068+
Some(500),
1069+
"a chunk-sink break must yield 500, not a truncated 200 success"
1070+
);
1071+
// The first chunk was already delivered to the sink before the break fired.
10621072
assert_eq!(body_buf.lock().unwrap().as_slice(), b"first");
10631073
}
10641074

1075+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
1076+
async fn bidirectional_chunk_break_returns_500_not_silent_success() {
1077+
// The non-header BIDIRECTIONAL path must also surface a 500 when the chunk
1078+
// sink breaks mid-response, instead of returning the captured success
1079+
// header (which would report a truncated bidirectional response as a clean
1080+
// success). Mirrors `response_streaming_chunk_break_returns_500...`.
1081+
install_router();
1082+
let wire = encode_wire("GET", "/multi-chunk", HashMap::new(), &[]);
1083+
let header = dispatch_bidirectional_streaming_closing(
1084+
wire,
1085+
|| RequestChunk::End, // no request body
1086+
|_chunk| ControlFlow::Break(()), // sink fails on the first chunk
1087+
|| {}, // no-op request-source close
1088+
)
1089+
.await;
1090+
let (header_json, _) = decode_wire(&header);
1091+
assert_eq!(
1092+
header_json["status"].as_u64(),
1093+
Some(500),
1094+
"a bidirectional chunk-sink break must yield 500, not truncated success"
1095+
);
1096+
}
1097+
10651098
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
10661099
async fn direct_write_body_error_yields_500_not_truncated_success() {
10671100
// Direct-write path: the response is buffered into the caller's slice and

crates/vespera_jni/src/jni_impl_direct.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,24 @@ fn ranges_overlap(a0: usize, a_len: usize, b0: usize, b_len: usize) -> bool {
5050
a0 < b1 && b0 < a1
5151
}
5252

53-
fn write_response_to_out(out_addr: *mut u8, out_cap: usize, response: &[u8]) -> jint {
53+
/// Copy `response` into the caller's direct out buffer, returning the
54+
/// `dispatchDirect0` code (`>= 0` bytes written, `-(required)` on overflow,
55+
/// [`DIRECT_UNREPRESENTABLE`] when the size exceeds `i32::MAX`).
56+
///
57+
/// # Safety
58+
///
59+
/// `out_addr` must point to a writable region of at least `out_cap` bytes
60+
/// that stays valid for the whole call (a JNI direct buffer pinned by a
61+
/// live `JByteBuffer` local ref) and must NOT alias `response` (callers
62+
/// pass a Rust-owned wire `Vec`). Encoded as `unsafe fn` so every call
63+
/// site acknowledges the raw-pointer contract instead of it being an
64+
/// unchecked promise on a safe function.
65+
unsafe fn write_response_to_out(out_addr: *mut u8, out_cap: usize, response: &[u8]) -> jint {
5466
if response.len() <= out_cap {
55-
// SAFETY: `response.len() <= out_cap` and the caller
56-
// guarantees `out_addr..out_addr+out_cap` is writable.
57-
// Source and destination cannot overlap: `response` is a
58-
// Rust-owned Vec, the destination is a Java direct buffer.
67+
// SAFETY: `response.len() <= out_cap` and the caller's `# Safety`
68+
// contract guarantees `out_addr..out_addr+out_cap` is writable and
69+
// non-aliasing with `response` (a Rust-owned Vec → a Java direct
70+
// buffer).
5971
unsafe {
6072
std::ptr::copy_nonoverlapping(response.as_ptr(), out_addr, response.len());
6173
}
@@ -151,7 +163,7 @@ pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_dispatchDir
151163
400,
152164
"invalid in_len (negative or exceeds buffer capacity)",
153165
);
154-
return Ok(write_response_to_out(out_addr, out_cap, &err));
166+
return Ok(unsafe { write_response_to_out(out_addr, out_cap, &err) });
155167
}
156168
};
157169

@@ -169,7 +181,7 @@ pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_dispatchDir
169181
400,
170182
"in_buf and out_buf must not overlap (aliasing would be undefined behavior)",
171183
);
172-
return Ok(write_response_to_out(out_addr, out_cap, &err));
184+
return Ok(unsafe { write_response_to_out(out_addr, out_cap, &err) });
173185
}
174186

175187
let dispatched = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
@@ -197,7 +209,7 @@ pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_dispatchDir
197209
}
198210
Err(_) => {
199211
let err = vespera_inprocess::error_wire(500, "panic in Rust engine");
200-
write_response_to_out(out_addr, out_cap, &err)
212+
unsafe { write_response_to_out(out_addr, out_cap, &err) }
201213
}
202214
};
203215
Ok(code)

0 commit comments

Comments
 (0)