Fix S3 collision, alias clearing, upload limit, TOCTOU; add tests
Bug fixes:
- S3 key is now emoji/{uuid}.{ext} instead of emoji/{filename},
preventing silent overwrites when two emotes share a filename
- UpdateEmoteRequest.alias uses Option<Option<String>> with a custom
deserializer so a JSON null clears the alias rather than being ignored;
the manage UI now sends null when the alias field is emptied
- POST /emotes is limited to 8 MiB via DefaultBodyLimit
- update_emote replaced the fetch-then-update pair with a single
UPDATE … RETURNING using COALESCE/CASE WHEN, eliminating the TOCTOU
race between concurrent edits
Refactoring:
- Extracted src/lib.rs so domain logic is a library crate; src/main.rs
is now a thin startup entry point
- auth::check decoupled from AppState — takes Option<&AuthConfig> directly
- Removed unused config field from Database struct
Tests (40 total):
- auth: 10 unit tests covering all check() branches
- models: 6 unit tests for timestamp parsing and alias deserialization
- db: 9 unit tests against in-memory SQLite covering full CRUD
- routes: 15 integration tests in tests/routes.rs covering auth
middleware, input validation, and all mutating endpoints
This commit is contained in:
@@ -1,25 +1,19 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use chrono::Utc;
|
||||
use sqlx::AnyPool;
|
||||
|
||||
use crate::{
|
||||
config::AppConfig,
|
||||
models::{EmoteRow, new_uuid},
|
||||
};
|
||||
use crate::models::EmoteRow;
|
||||
|
||||
/// Thin database abstraction that works with both SQLite and PostgreSQL
|
||||
/// through `sqlx::AnyPool`.
|
||||
#[derive(Clone)]
|
||||
pub struct Database {
|
||||
pub pool: AnyPool,
|
||||
pub config: Arc<AppConfig>,
|
||||
}
|
||||
|
||||
impl Database {
|
||||
pub async fn connect(config: Arc<AppConfig>) -> Result<Self, sqlx::Error> {
|
||||
let pool = AnyPool::connect(&config.database.url).await?;
|
||||
Ok(Self { pool, config })
|
||||
pub async fn connect(url: &str) -> Result<Self, sqlx::Error> {
|
||||
let pool = AnyPool::connect(url).await?;
|
||||
Ok(Self { pool })
|
||||
}
|
||||
|
||||
/// Run pending migrations.
|
||||
@@ -48,16 +42,16 @@ impl Database {
|
||||
|
||||
pub async fn create_emote(
|
||||
&self,
|
||||
uuid: &str,
|
||||
name: &str,
|
||||
alias: Option<&str>,
|
||||
image_key: &str,
|
||||
) -> Result<EmoteRow, sqlx::Error> {
|
||||
let id = new_uuid();
|
||||
let now = Utc::now().to_rfc3339();
|
||||
sqlx::query(
|
||||
"INSERT INTO emotes (uuid, name, alias, image_key, created, modified) VALUES ($1, $2, $3, $4, $5, $6)",
|
||||
)
|
||||
.bind(&id)
|
||||
.bind(uuid)
|
||||
.bind(name)
|
||||
.bind(alias)
|
||||
.bind(image_key)
|
||||
@@ -67,7 +61,7 @@ impl Database {
|
||||
.await?;
|
||||
|
||||
Ok(EmoteRow {
|
||||
uuid: id,
|
||||
uuid: uuid.to_string(),
|
||||
name: name.to_string(),
|
||||
alias: alias.map(|s| s.to_string()),
|
||||
image_key: image_key.to_string(),
|
||||
@@ -83,39 +77,32 @@ impl Database {
|
||||
alias: Option<Option<&str>>,
|
||||
image_key: Option<&str>,
|
||||
) -> Result<Option<EmoteRow>, sqlx::Error> {
|
||||
let existing = match self.get_emote_by_id(uuid).await? {
|
||||
Some(e) => e,
|
||||
None => return Ok(None),
|
||||
};
|
||||
|
||||
let new_name = name.unwrap_or(&existing.name);
|
||||
let new_image_key = image_key.unwrap_or(&existing.image_key);
|
||||
let new_alias: Option<String> = match alias {
|
||||
Some(Some(a)) => Some(a.to_string()),
|
||||
Some(None) => None,
|
||||
None => existing.alias.clone(),
|
||||
};
|
||||
let now = Utc::now().to_rfc3339();
|
||||
// alias has three states: None = keep, Some(None) = clear, Some(Some(v)) = set.
|
||||
// Pass a boolean flag so CASE WHEN can choose between the new value and the
|
||||
// existing column — all resolved atomically in a single statement.
|
||||
let alias_touch = alias.is_some();
|
||||
let alias_value: Option<&str> = alias.flatten();
|
||||
|
||||
sqlx::query(
|
||||
"UPDATE emotes SET name = $1, alias = $2, image_key = $3, modified = $4 WHERE uuid = $5",
|
||||
let row = sqlx::query_as::<_, EmoteRow>(
|
||||
"UPDATE emotes
|
||||
SET name = COALESCE($1, name),
|
||||
alias = CASE WHEN $2 THEN $3 ELSE alias END,
|
||||
image_key = COALESCE($4, image_key),
|
||||
modified = $5
|
||||
WHERE uuid = $6
|
||||
RETURNING uuid, name, alias, image_key, created, modified",
|
||||
)
|
||||
.bind(new_name)
|
||||
.bind(new_alias.as_deref())
|
||||
.bind(new_image_key)
|
||||
.bind(name)
|
||||
.bind(alias_touch)
|
||||
.bind(alias_value)
|
||||
.bind(image_key)
|
||||
.bind(&now)
|
||||
.bind(uuid)
|
||||
.execute(&self.pool)
|
||||
.fetch_optional(&self.pool)
|
||||
.await?;
|
||||
|
||||
Ok(Some(EmoteRow {
|
||||
uuid: uuid.to_string(),
|
||||
name: new_name.to_string(),
|
||||
alias: new_alias,
|
||||
image_key: new_image_key.to_string(),
|
||||
created: existing.created,
|
||||
modified: now,
|
||||
}))
|
||||
Ok(row)
|
||||
}
|
||||
|
||||
pub async fn delete_emote(&self, uuid: &str) -> Result<bool, sqlx::Error> {
|
||||
@@ -126,3 +113,129 @@ impl Database {
|
||||
Ok(result.rows_affected() > 0)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::models::new_uuid;
|
||||
|
||||
async fn test_db() -> Database {
|
||||
sqlx::any::install_default_drivers();
|
||||
let pool = sqlx::pool::PoolOptions::<sqlx::Any>::new()
|
||||
.max_connections(1)
|
||||
.connect("sqlite::memory:")
|
||||
.await
|
||||
.unwrap();
|
||||
let db = Database { pool };
|
||||
db.migrate().await.unwrap();
|
||||
db
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_and_get_by_id() {
|
||||
let db = test_db().await;
|
||||
let id = new_uuid();
|
||||
let row = db.create_emote(&id, "cat", Some("kitty"), "emoji/cat.png").await.unwrap();
|
||||
|
||||
assert_eq!(row.uuid, id);
|
||||
assert_eq!(row.name, "cat");
|
||||
assert_eq!(row.alias.as_deref(), Some("kitty"));
|
||||
assert_eq!(row.image_key, "emoji/cat.png");
|
||||
|
||||
let fetched = db.get_emote_by_id(&id).await.unwrap().unwrap();
|
||||
assert_eq!(fetched.uuid, id);
|
||||
assert_eq!(fetched.name, "cat");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_unknown_uuid_returns_none() {
|
||||
let db = test_db().await;
|
||||
let result = db.get_emote_by_id("does-not-exist").await.unwrap();
|
||||
assert!(result.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_returns_all_emotes_newest_first() {
|
||||
let db = test_db().await;
|
||||
let id1 = new_uuid();
|
||||
let id2 = new_uuid();
|
||||
db.create_emote(&id1, "alpha", None, "emoji/alpha.png").await.unwrap();
|
||||
db.create_emote(&id2, "beta", None, "emoji/beta.png").await.unwrap();
|
||||
|
||||
let rows = db.list_emotes().await.unwrap();
|
||||
assert_eq!(rows.len(), 2);
|
||||
// ORDER BY created DESC — beta was inserted last so it comes first
|
||||
assert_eq!(rows[0].name, "beta");
|
||||
assert_eq!(rows[1].name, "alpha");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_name() {
|
||||
let db = test_db().await;
|
||||
let id = new_uuid();
|
||||
db.create_emote(&id, "old", None, "emoji/x.png").await.unwrap();
|
||||
|
||||
let updated = db.update_emote(&id, Some("new"), None, None).await.unwrap().unwrap();
|
||||
assert_eq!(updated.name, "new");
|
||||
assert_eq!(updated.image_key, "emoji/x.png"); // unchanged
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_unspecified_fields_are_kept() {
|
||||
let db = test_db().await;
|
||||
let id = new_uuid();
|
||||
db.create_emote(&id, "name", Some("alias"), "emoji/x.png").await.unwrap();
|
||||
|
||||
// Pass None for all optional fields — nothing should change except modified timestamp.
|
||||
let updated = db.update_emote(&id, None, None, None).await.unwrap().unwrap();
|
||||
assert_eq!(updated.name, "name");
|
||||
assert_eq!(updated.alias.as_deref(), Some("alias"));
|
||||
assert_eq!(updated.image_key, "emoji/x.png");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_alias_set_and_clear() {
|
||||
let db = test_db().await;
|
||||
let id = new_uuid();
|
||||
db.create_emote(&id, "cat", None, "emoji/cat.png").await.unwrap();
|
||||
|
||||
// Set alias
|
||||
let with_alias = db
|
||||
.update_emote(&id, None, Some(Some("kitty")), None)
|
||||
.await
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(with_alias.alias.as_deref(), Some("kitty"));
|
||||
|
||||
// Clear alias
|
||||
let cleared = db
|
||||
.update_emote(&id, None, Some(None), None)
|
||||
.await
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert!(cleared.alias.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_unknown_uuid_returns_none() {
|
||||
let db = test_db().await;
|
||||
let result = db.update_emote("no-such-id", Some("x"), None, None).await.unwrap();
|
||||
assert!(result.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn delete_removes_emote() {
|
||||
let db = test_db().await;
|
||||
let id = new_uuid();
|
||||
db.create_emote(&id, "bye", None, "emoji/bye.png").await.unwrap();
|
||||
|
||||
assert!(db.delete_emote(&id).await.unwrap());
|
||||
assert!(db.get_emote_by_id(&id).await.unwrap().is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn delete_unknown_uuid_returns_false() {
|
||||
let db = test_db().await;
|
||||
assert!(!db.delete_emote("no-such-id").await.unwrap());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user