bugfix: tighten validation, drop dead sendBeacon, NUL byte (0.34.1)
Five small fixes from REVIEW.md §2/§4/§8: - attach_tag: 64-char cap at the handler so the validation error envelope matches username/collection-name. - create_token: same 64-char cap on bot token names. - LocalStorage::resolve rejects NUL bytes explicitly so callers see BadKey instead of an opaque IO error. - sendBeacon dropped from the reader's pagehide flush — it's POST-only and the server's read-progress route is PUT, so every page-close was logging a 405 then falling through to the same keepalive fetch anyway. Keepalive fetch is now the only path. - Frontend logout sets content-type: application/json for symmetry with the other mutation helpers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1470,7 +1470,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.34.0"
|
||||
version = "0.34.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.34.0"
|
||||
version = "0.34.1"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -230,8 +230,24 @@ async fn create_token(
|
||||
Json(input): Json<CreateTokenInput>,
|
||||
) -> AppResult<impl IntoResponse> {
|
||||
let name = input.name.trim();
|
||||
// Both arms use `ValidationFailed` (422 with field details) to
|
||||
// match the structured-error shape `attach_tag` returns for the
|
||||
// same kind of free-form-identifier validation. The other
|
||||
// /auth/* handlers in this file use `InvalidInput` (400); the
|
||||
// divergence is pre-existing and would warrant a project-wide
|
||||
// pass to flip them all if the client side wants uniform per-
|
||||
// field error rendering.
|
||||
if name.is_empty() {
|
||||
return Err(AppError::InvalidInput("token name is required".into()));
|
||||
return Err(AppError::ValidationFailed {
|
||||
message: "token name is required".into(),
|
||||
details: serde_json::json!({ "name": "required" }),
|
||||
});
|
||||
}
|
||||
if name.chars().count() > 64 {
|
||||
return Err(AppError::ValidationFailed {
|
||||
message: "token name too long".into(),
|
||||
details: serde_json::json!({ "name": "max 64 characters" }),
|
||||
});
|
||||
}
|
||||
let (raw, hash) = generate_token();
|
||||
let token = repo::api_token::create(&state.db, user.id, name, &hash).await?;
|
||||
|
||||
@@ -348,6 +348,7 @@ async fn attach_tag(
|
||||
Path(id): Path<Uuid>,
|
||||
Json(body): Json<AttachTagBody>,
|
||||
) -> AppResult<(StatusCode, Json<TagRef>)> {
|
||||
validate_tag_name(&body.name)?;
|
||||
if !repo::manga::exists(&state.db, id).await? {
|
||||
return Err(AppError::NotFound);
|
||||
}
|
||||
@@ -394,6 +395,27 @@ async fn detach_tag(
|
||||
}
|
||||
}
|
||||
|
||||
/// Request-side validation for `POST /mangas/:id/tags` body. Mirrors
|
||||
/// the repo-level cap in `repo::tag::upsert_by_name` (max 64 chars
|
||||
/// after trim) but surfaces the failure at the handler boundary with
|
||||
/// the same envelope shape other validations use.
|
||||
fn validate_tag_name(name: &str) -> AppResult<()> {
|
||||
let trimmed = name.trim();
|
||||
if trimmed.is_empty() {
|
||||
return Err(AppError::ValidationFailed {
|
||||
message: "tag name cannot be empty".into(),
|
||||
details: json!({ "name": "required" }),
|
||||
});
|
||||
}
|
||||
if trimmed.chars().count() > 64 {
|
||||
return Err(AppError::ValidationFailed {
|
||||
message: "tag name too long".into(),
|
||||
details: json!({ "name": "max 64 characters" }),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_new_manga(input: &NewManga) -> AppResult<()> {
|
||||
if input.title.trim().is_empty() {
|
||||
return Err(AppError::ValidationFailed {
|
||||
|
||||
@@ -16,6 +16,13 @@ impl LocalStorage {
|
||||
}
|
||||
|
||||
fn resolve(&self, key: &str) -> Result<PathBuf, StorageError> {
|
||||
// NUL bytes are rejected by the Linux syscall layer, but the
|
||||
// error surfaces as an opaque IO failure rather than the
|
||||
// explicit `BadKey` the rest of the contract uses. Catch it
|
||||
// here so the error path is consistent.
|
||||
if key.contains('\0') {
|
||||
return Err(StorageError::BadKey);
|
||||
}
|
||||
let key = key.trim_start_matches('/');
|
||||
if key.is_empty() {
|
||||
return Err(StorageError::BadKey);
|
||||
@@ -114,6 +121,9 @@ mod tests {
|
||||
assert!(matches!(s.get(".").await, Err(StorageError::BadKey)));
|
||||
// Empty segment via doubled slash.
|
||||
assert!(matches!(s.get("a//b").await, Err(StorageError::BadKey)));
|
||||
// NUL byte (rejected explicitly so callers see BadKey rather
|
||||
// than an opaque IO error from the kernel).
|
||||
assert!(matches!(s.put("a\0b", b"x").await, Err(StorageError::BadKey)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -581,3 +581,27 @@ async fn delete_unknown_token_is_404(pool: PgPool) {
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
|
||||
}
|
||||
|
||||
/// Bot token names are user-supplied free-form strings; a 10 MB name
|
||||
/// was accepted before. Cap at 64 chars to match the other free-form
|
||||
/// identifier caps (tags, collection names). The response uses
|
||||
/// `ValidationFailed` (422 with per-field details) so clients can
|
||||
/// render the same shape they already handle for `attach_tag`.
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn create_token_rejects_name_over_64_chars(pool: PgPool) {
|
||||
let h = common::harness(pool);
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::post_json_with_cookie(
|
||||
"/api/v1/auth/tokens",
|
||||
json!({ "name": "x".repeat(65) }),
|
||||
&cookie,
|
||||
))
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY);
|
||||
let body = common::body_json(resp).await;
|
||||
assert_eq!(body["error"]["code"], "validation_failed");
|
||||
assert!(body["error"]["details"]["name"].is_string());
|
||||
}
|
||||
|
||||
@@ -59,6 +59,31 @@ async fn reattach_same_tag_is_idempotent_and_returns_200(pool: PgPool) {
|
||||
assert_eq!(second.status(), StatusCode::OK);
|
||||
}
|
||||
|
||||
/// Tag names over 64 chars are rejected at the handler boundary. The
|
||||
/// repo enforces the same cap, but doing it at the handler keeps the
|
||||
/// envelope consistent with the other validation paths
|
||||
/// (username, collection name, etc.).
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn attach_rejects_tag_name_over_64_chars(pool: PgPool) {
|
||||
let h = common::harness(pool);
|
||||
let (_, cookie) = common::register_user(&h.app).await;
|
||||
let manga_id = common::seed_manga_via_api(&h.app, &cookie, "Berserk").await;
|
||||
|
||||
let long_name: String = "x".repeat(65);
|
||||
let resp = h
|
||||
.app
|
||||
.oneshot(common::post_json_with_cookie(
|
||||
&format!("/api/v1/mangas/{manga_id}/tags"),
|
||||
json!({ "name": long_name }),
|
||||
&cookie,
|
||||
))
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::UNPROCESSABLE_ENTITY);
|
||||
let body = common::body_json(resp).await;
|
||||
assert_eq!(body["error"]["code"], "validation_failed");
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn tag_names_dedup_case_insensitively(pool: PgPool) {
|
||||
let h = common::harness(pool);
|
||||
|
||||
Reference in New Issue
Block a user