Skip to content

Commit 9a6d7f8

Browse files
committed
Fix jni
1 parent b5eea88 commit 9a6d7f8

27 files changed

Lines changed: 711 additions & 105 deletions

File tree

crates/vespera/src/multipart.rs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,9 @@ impl IntoResponse for TypedMultipartError {
310310
| Self::TooManyFields { .. } => StatusCode::PAYLOAD_TOO_LARGE,
311311
Self::Other { .. } => StatusCode::INTERNAL_SERVER_ERROR,
312312
};
313-
// Serialize the canonical 422 envelope (see `error_body` /
314-
// module-scope `MultipartErrorEnvelope`).
313+
// Serialize the canonical JSON error envelope (see `error_body` /
314+
// module-scope `MultipartErrorEnvelope`); the status varies (400/413/
315+
// 422/500) but the body shape is identical.
315316
let body = self.error_body();
316317
(
317318
status,
@@ -580,7 +581,16 @@ tokio::task_local! {
580581
static MULTIPART_AGGREGATE: RefCell<MultipartAggregateState>;
581582
}
582583

583-
fn register_multipart_field() -> Result<(), TypedMultipartError> {
584+
/// Count one multipart PART against the request-wide `max_fields` limit.
585+
///
586+
/// Invoked by the derived `TryFromMultipart` loop **once per wire part** —
587+
/// before the field name is resolved — so EVERY part (known, unknown, or
588+
/// nameless) is counted exactly once. Counting inside the per-known-field
589+
/// parsers instead let unknown parts in non-strict mode (the `_ => {}`
590+
/// dispatch arm) slip past the cap entirely, so a request with thousands of
591+
/// unknown parts could burn unbounded parser/boundary-scan work without ever
592+
/// tripping `TooManyFields`.
593+
pub fn register_multipart_part() -> Result<(), TypedMultipartError> {
584594
MULTIPART_AGGREGATE
585595
.try_with(|state| {
586596
let mut state = state.borrow_mut();
@@ -592,9 +602,8 @@ fn register_multipart_field() -> Result<(), TypedMultipartError> {
592602
}
593603
Ok(())
594604
})
595-
// Field parsers can be unit-tested outside the extractor. In that shape
596-
// there is no request aggregate to update, so per-field limits remain the
597-
// only active guard instead of failing spuriously.
605+
// The derived impl can be unit-tested outside the extractor scope; with
606+
// no request aggregate present, counting no-ops rather than failing.
598607
.unwrap_or(Ok(()))
599608
}
600609

@@ -705,7 +714,8 @@ async fn read_field_data(
705714
limit: Option<usize>,
706715
initial_capacity: usize,
707716
) -> Result<(Field<'_>, Vec<u8>), TypedMultipartError> {
708-
register_multipart_field()?;
717+
// Part counting now happens once per part in the derived loop
718+
// (`register_multipart_part`), so the field parsers no longer count.
709719
// Initial capacity is independent from the hard byte limit: tiny scalar
710720
// fields keep the 256B cap without preallocating 256B per bool/number.
711721
let capacity = limit.map_or(initial_capacity, |limit| initial_capacity.min(limit));
@@ -940,7 +950,8 @@ impl<S: Send + Sync> TryFromFieldWithState<S> for tempfile::NamedTempFile {
940950
limit_bytes: Option<usize>,
941951
_state: &S,
942952
) -> Result<Self, TypedMultipartError> {
943-
register_multipart_field()?;
953+
// Part counting happens once per part in the derived loop
954+
// (`register_multipart_part`); the temp-file parser no longer counts.
944955
// Temp-file creation AND reopen() are both blocking syscalls —
945956
// run them together on the blocking pool so neither stalls the
946957
// async worker (the reopen previously ran inline on the async

crates/vespera/tests/multipart_wire.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,35 @@ fn typed_multipart_aggregate_field_count_cap_rejected_413() {
424424
assert_eq!(header["status"].as_u64(), Some(413), "header={header:#}");
425425
}
426426

427+
#[test]
428+
fn typed_multipart_unknown_fields_count_toward_max_fields() {
429+
// Regression: in non-strict mode an UNKNOWN part (the generated `_ => {}`
430+
// dispatch arm) must still count against `max_fields`. Before the fix,
431+
// counting happened only inside the per-known-field parsers, so a flood of
432+
// unknown parts bypassed the cap entirely (a DoS-adjacent gap) and this
433+
// request would instead fail later with a 400 missing-field error. With
434+
// `MAX_FIELDS = 0`, even one part — known OR unknown — must be rejected 413.
435+
install_router_once();
436+
let runtime = Builder::new_current_thread()
437+
.enable_all()
438+
.build()
439+
.expect("tokio runtime");
440+
441+
let wire = encode_multipart_text(
442+
"----UnknownFieldCountBoundary",
443+
"/text-aggregate-field-count",
444+
"definitely_not_a_known_field",
445+
b"x",
446+
);
447+
let resp = dispatch_from_bytes(wire, &runtime);
448+
let (header, _body) = decode_wire(&resp);
449+
assert_eq!(
450+
header["status"].as_u64(),
451+
Some(413),
452+
"an unknown multipart part must count against max_fields, got header={header:#}"
453+
);
454+
}
455+
427456
#[test]
428457
fn typed_multipart_aggregate_under_limit_passes() {
429458
install_router_once();

crates/vespera/tests/validated_extractor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ async fn multiple_per_rule_violations_all_appear_in_envelope() {
442442
// - Consumed by Java decoders and client libraries
443443
//
444444
// Multi-error coverage: triggers 2+ field errors to verify the full
445-
// envelope structure (path before message, array ordering, etc.).
445+
// envelope structure (message before path, array ordering, etc.).
446446

447447
#[tokio::test]
448448
async fn byte_snapshot_422_envelope_multi_error() {

crates/vespera_inprocess/src/dispatch.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,11 @@ async fn finish_buffered_wire(
286286
// with zero reallocations.
287287
let header_cap = header_capacity_estimate(&headers, &metadata).max(WIRE_HEADER_RESERVE);
288288
let body_cap = usize::try_from(body.size_hint().exact().unwrap_or(0)).unwrap_or(0);
289-
let mut out = Vec::with_capacity(4 + header_cap + body_cap);
289+
// Saturating so a pathological/oversized exact body hint cannot wrap the
290+
// capacity arithmetic (debug panic / release wrap → under-reserve); the
291+
// common case computes the identical value, and `finish_direct_write`
292+
// already uses the same saturating accounting for its overflow reporting.
293+
let mut out = Vec::with_capacity(4usize.saturating_add(header_cap).saturating_add(body_cap));
290294
if !write_wire_header_into_vec(&mut out, status, &headers, &metadata) {
291295
// Unreachable for a real `HeaderMap` (4 GiB+ of header JSON); never
292296
// panic on the response path — emit a 500 wire response instead.

crates/vespera_inprocess/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
//! (request) { "v":1, "method", "path",
3636
//! "query"?, "headers"? }
3737
//! (response) { "v":1, "status", "headers",
38-
//! "metadata" }
38+
//! "metadata", "validation_errors"? }
3939
//! bytes 4+N..end : raw body bytes (UTF-8 text or binary —
4040
//! no encoding applied)
4141
//! ```

crates/vespera_inprocess/src/wire/hoist.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@ fn body_is_json(headers: &http::HeaderMap) -> bool {
4545
/// This is the **fast strict path**: the common, framework-generated envelope
4646
/// has all-string fields, so the plain derive parses it with no per-field
4747
/// visitor overhead. A body with a wrong-typed field (`"code": 123`) fails
48-
/// this strict parse and is retried via [`LenientHoistEnvelope`], so the
49-
/// hoist stays genuinely best-effort without taxing the common case.
48+
/// this strict parse and is retried via the inline `serde_json::Value`
49+
/// fallback walk in [`try_hoist_validation_errors`], so the hoist stays
50+
/// genuinely best-effort without taxing the common case.
5051
#[derive(Deserialize)]
5152
struct HoistEnvelope {
5253
errors: Vec<HoistErrorIn>,

crates/vespera_jni/src/jni_impl.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -402,10 +402,15 @@ pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_dispatchAsy
402402
/// first to commit the HTTP status + response headers, then
403403
/// continue serving the streamed body bytes.
404404
///
405-
/// Failure modes mirror [`Java_...dispatchBytes`]: malformed wire,
406-
/// version mismatch, no app registered, or Rust panic produce a
407-
/// regular `error_wire(...)` response (header + small body) and
408-
/// the `OutputStream` is **not** written to.
405+
/// Failure modes mirror [`Java_...dispatchBytes`]: a **pre-streaming**
406+
/// failure (malformed wire, version mismatch, no app registered, or a panic
407+
/// before the first body frame) produces a regular `error_wire(...)` response
408+
/// (header + small body) and the `OutputStream` is **not** written to. A
409+
/// failure that occurs **after** the first body frame (the host
410+
/// `OutputStream` erroring mid-drain, or a body-stream error) may leave
411+
/// partial bytes already written to the `OutputStream`; it is still reported
412+
/// as a `500` `error_wire(...)` header return, so the caller must treat a
413+
/// `5xx` header returned after streaming has begun as a truncated response.
409414
#[unsafe(no_mangle)]
410415
pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_dispatchStreaming<'local>(
411416
mut unowned_env: EnvUnowned<'local>,

crates/vespera_macro/src/cron_impl.rs

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,36 @@ pub struct StoredCronInfo {
4747
pub static CRON_STORAGE: LazyLock<Mutex<HashMap<String, Vec<StoredCronInfo>>>> =
4848
LazyLock::new(|| Mutex::new(HashMap::new()));
4949

50-
/// Append a `#[cron]` metadata entry to the current crate's bucket.
50+
fn same_cron_source(left: &StoredCronInfo, right: &StoredCronInfo) -> bool {
51+
left.fn_name == right.fn_name
52+
&& left
53+
.file_path
54+
.as_deref()
55+
.unwrap_or_default()
56+
.replace('\\', "/")
57+
== right
58+
.file_path
59+
.as_deref()
60+
.unwrap_or_default()
61+
.replace('\\', "/")
62+
}
63+
64+
/// Replace-insert a `#[cron]` metadata entry in the current crate's bucket.
5165
pub fn register_cron(info: StoredCronInfo) {
52-
CRON_STORAGE
66+
let mut guard = CRON_STORAGE
5367
.lock()
54-
.unwrap_or_else(std::sync::PoisonError::into_inner)
68+
.unwrap_or_else(std::sync::PoisonError::into_inner);
69+
let bucket = guard
5570
.entry(crate::schema_impl::current_crate_key())
56-
.or_default()
57-
.push(info);
71+
.or_default();
72+
if let Some(existing) = bucket
73+
.iter_mut()
74+
.find(|existing| same_cron_source(existing, &info))
75+
{
76+
*existing = info;
77+
} else {
78+
bucket.push(info);
79+
}
5880
}
5981

6082
/// Snapshot (clone) of the current crate's registered cron jobs, so the
@@ -305,6 +327,29 @@ mod tests {
305327
assert!(err.contains("must take no parameters"));
306328
}
307329

330+
#[test]
331+
fn test_register_cron_replaces_same_file_and_function() {
332+
let file_path = Some("/tmp/vespera/tasks/replaced.rs".to_string());
333+
let fn_name = "__test_replace_cron".to_string();
334+
register_cron(StoredCronInfo {
335+
fn_name: fn_name.clone(),
336+
expression: "0 */5 * * * *".to_string(),
337+
file_path: file_path.clone(),
338+
});
339+
register_cron(StoredCronInfo {
340+
fn_name: fn_name.clone(),
341+
expression: "0 */10 * * * *".to_string(),
342+
file_path,
343+
});
344+
345+
let matches: Vec<_> = current_crate_crons()
346+
.into_iter()
347+
.filter(|entry| entry.fn_name == fn_name)
348+
.collect();
349+
assert_eq!(matches.len(), 1, "same source cron should replace");
350+
assert_eq!(matches[0].expression, "0 */10 * * * *");
351+
}
352+
308353
// ===== Compile-time cron-syntax validation (gated by the `cron` feature) =====
309354

310355
#[cfg(feature = "cron")]

crates/vespera_macro/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ pub fn derive_schema(input: TokenStream) -> TokenStream {
121121

122122
let input = syn::parse_macro_input!(input as syn::DeriveInput);
123123
let (metadata, expanded) = schema_impl::process_derive_schema(&input);
124+
let Some(metadata) = metadata else {
125+
return TokenStream::from(expanded);
126+
};
124127
let name = metadata.name.clone();
125128

126129
// Register into the current crate's bucket (see `current_crate_key`).

crates/vespera_macro/src/metadata.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ pub struct StructMetadata {
8383
/// Populated by `#[derive(Schema)]` to avoid AST re-parsing in `vespera!()`.
8484
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
8585
pub field_defaults: BTreeMap<String, serde_json::Value>,
86+
/// Stable source identity for proc-macro-server re-expansions.
87+
///
88+
/// This is not part of the OpenAPI output. It lets `#[derive(Schema)]`
89+
/// replace metadata for the same source item after an IDE edit while still
90+
/// rejecting two distinct items that claim the same OpenAPI schema name.
91+
#[serde(default, skip_serializing_if = "Option::is_none")]
92+
pub source_identity: Option<String>,
8693
}
8794

8895
const fn default_include_in_openapi() -> bool {
@@ -96,6 +103,7 @@ impl Default for StructMetadata {
96103
definition: String::new(),
97104
include_in_openapi: true,
98105
field_defaults: BTreeMap::new(),
106+
source_identity: None,
99107
}
100108
}
101109
}
@@ -108,6 +116,7 @@ impl StructMetadata {
108116
definition,
109117
include_in_openapi: true,
110118
field_defaults: BTreeMap::new(),
119+
source_identity: None,
111120
}
112121
}
113122

@@ -118,8 +127,16 @@ impl StructMetadata {
118127
definition,
119128
include_in_openapi: false,
120129
field_defaults: BTreeMap::new(),
130+
source_identity: None,
121131
}
122132
}
133+
134+
/// Attach the source identity used for same-item replacement in global storage.
135+
#[must_use]
136+
pub fn with_source_identity(mut self, source_identity: String) -> Self {
137+
self.source_identity = Some(source_identity);
138+
self
139+
}
123140
}
124141

125142
/// Cron job metadata

0 commit comments

Comments
 (0)