fix(upload): a NUL byte in a caption returned 500 and cost the guest the photo
Postgres refuses a NUL in a TEXT column outright — `invalid byte sequence for encoding "UTF8": 0x00` — and that arrived here as an anonymous `sqlx::Error`, became `AppError::Internal`, and returned a 500. The photo went with it: the transaction rolls back with the file already streamed to disk, so the guest loses the upload as well as the caption, with nothing in the message telling them why. Only `display_name` was validated for control characters; `caption`, the hashtag CSV and `comment.body` were length-checked alone. Rejected at the edge with a 400 instead, and deliberately narrower than `validate_display_name`: a caption legitimately carries newlines and emoji, and every control character except NUL stores and renders harmlessly, so this refuses exactly the byte that cannot work. Rejected rather than stripped — silently rewriting what a guest wrote is the worse failure, and no real client emits a NUL by accident. Found by the event simulation's abuse suite; it was the only input of 37 that produced a 5xx. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -201,6 +201,9 @@ pub async fn add_comment(
|
||||
"Kommentar muss zwischen 1 und 500 Zeichen lang sein.".into(),
|
||||
));
|
||||
}
|
||||
// Same reason as the caption path: a NUL is the one character Postgres will not store,
|
||||
// and without this the INSERT below fails as an anonymous 500.
|
||||
crate::handlers::upload::reject_nul(text, "Kommentar")?;
|
||||
|
||||
// Insert the comment and link its hashtags atomically, so a crash mid-loop
|
||||
// can't leave a committed comment with only some of its tags indexed.
|
||||
|
||||
@@ -17,6 +17,29 @@ use crate::state::AppState;
|
||||
|
||||
const MAX_CAPTION_LENGTH: usize = 2000;
|
||||
|
||||
/// Reject free text carrying a NUL byte, before it reaches Postgres.
|
||||
///
|
||||
/// A NUL is the one character a `TEXT` column refuses outright: the driver forwards it and
|
||||
/// Postgres answers `invalid byte sequence for encoding "UTF8": 0x00`, which arrives here as an
|
||||
/// anonymous `sqlx::Error`, becomes `AppError::Internal`, and costs the guest a 500 AND the
|
||||
/// photo they just spent a minute uploading — the transaction rolls back with the file already
|
||||
/// streamed to disk.
|
||||
///
|
||||
/// Deliberately narrower than `validate_display_name`, which refuses control characters
|
||||
/// wholesale. A caption legitimately contains newlines and emoji, and every other control
|
||||
/// character stores and renders harmlessly; this rejects exactly the byte that cannot work.
|
||||
///
|
||||
/// Rejected rather than stripped. Silently rewriting what a guest wrote is the worse failure,
|
||||
/// and no real client emits a NUL by accident — it only arrives from a broken or hostile one.
|
||||
pub(crate) fn reject_nul(value: &str, field_de: &str) -> Result<(), AppError> {
|
||||
if value.contains('\0') {
|
||||
return Err(AppError::BadRequest(format!(
|
||||
"{field_de} enthält ein ungültiges Zeichen und wurde nicht gespeichert."
|
||||
)));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Byte ceiling for the caption field, enforced WHILE reading it.
|
||||
///
|
||||
/// `Field::text()` buffers the entire field before returning, and this is the one route whose
|
||||
@@ -458,6 +481,12 @@ pub async fn upload(
|
||||
MAX_CAPTION_LENGTH
|
||||
)));
|
||||
}
|
||||
if let Some(ref cap) = caption {
|
||||
reject_nul(cap, "Beschreibung")?;
|
||||
}
|
||||
if let Some(ref csv) = hashtags_csv {
|
||||
reject_nul(csv, "Hashtags")?;
|
||||
}
|
||||
|
||||
// Determine the file type from its magic bytes and require it to be on the
|
||||
// allowlist. `infer` returns None for text-based payloads (SVG/HTML/JS), so
|
||||
@@ -970,6 +999,14 @@ pub async fn edit_upload(
|
||||
"Beschreibung ist zu lang. Maximum: {MAX_CAPTION_LENGTH} Zeichen."
|
||||
)));
|
||||
}
|
||||
if let Some(ref caption) = body.caption {
|
||||
reject_nul(caption, "Beschreibung")?;
|
||||
}
|
||||
if let Some(ref tags) = body.hashtags {
|
||||
for tag in tags {
|
||||
reject_nul(tag, "Hashtags")?;
|
||||
}
|
||||
}
|
||||
let normalized_tags = body
|
||||
.hashtags
|
||||
.as_ref()
|
||||
|
||||
Reference in New Issue
Block a user