Skip to content

Commit 01c2c9c

Browse files
committed
Avoid ID clashes
Currently the the ID for a new paste is randomly generated in the caller of the database insert() function. Then the insert() function tries to insert a new row into the database with that passed ID. There can however already exists a paste in the database with the same ID leading to an insert failure, due to a constraint violation due to the PRIMARY KEY attribute. Checking prior the the INSERT via a SELECT query would open the window for a race condition. A failure to push a new paste is quite severe, since the user might have spent some some to format the input. Generate the ID in a loop inside, until the INSERT succeeds.
1 parent e9a6850 commit 01c2c9c

File tree

3 files changed

+49
-35
lines changed

3 files changed

+49
-35
lines changed

crates/wastebin_core/src/db.rs

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -326,32 +326,51 @@ impl Database {
326326
Ok(Self { conn })
327327
}
328328

329-
/// Insert `entry` under `id` into the database and optionally set owner to `uid`.
330-
pub async fn insert(&self, id: Id, entry: write::Entry) -> Result<(), Error> {
329+
/// Insert `entry` with a new generated `id` into the database and optionally set owner to `uid`.
330+
pub async fn insert(&self, entry: write::Entry) -> Result<(Id, write::Entry), Error> {
331331
let conn = self.conn.clone();
332332
let write::DatabaseEntry { entry, data, nonce } = entry.compress().await?.encrypt().await?;
333333

334-
spawn_blocking(move || match entry.expires {
335-
None => conn.lock().execute(
336-
"INSERT INTO entries (id, uid, data, burn_after_reading, nonce, title) VALUES (?1, ?2, ?3, ?4, ?5, ?6)",
337-
params![id.to_i64(), entry.uid, data, entry.burn_after_reading, nonce, entry.title],
338-
),
339-
Some(expires) => conn.lock().execute(
340-
"INSERT INTO entries (id, uid, data, burn_after_reading, nonce, expires, title) VALUES (?1, ?2, ?3, ?4, ?5, datetime('now', ?6), ?7)",
341-
params![
342-
id.to_i64(),
343-
entry.uid,
344-
data,
345-
entry.burn_after_reading,
346-
nonce,
347-
format!("{expires} seconds"),
348-
entry.title,
349-
],
350-
),
351-
})
352-
.await??;
334+
let title = entry.title.clone();
335+
336+
let id = spawn_blocking(move || {
337+
let mut counter = 0;
338+
339+
loop {
340+
let id = Id::rand();
341+
342+
let result = match entry.expires {
343+
None => conn.lock().execute(
344+
"INSERT INTO entries (id, uid, data, burn_after_reading, nonce, title) VALUES (?1, ?2, ?3, ?4, ?5, ?6)",
345+
params![id.to_i64(), entry.uid, data, entry.burn_after_reading, nonce, title],
346+
),
347+
Some(expires) => conn.lock().execute(
348+
"INSERT INTO entries (id, uid, data, burn_after_reading, nonce, expires, title) VALUES (?1, ?2, ?3, ?4, ?5, datetime('now', ?6), ?7)",
349+
params![
350+
id.to_i64(),
351+
entry.uid,
352+
data,
353+
entry.burn_after_reading,
354+
nonce,
355+
format!("{expires} seconds"),
356+
title,
357+
],
358+
),
359+
};
360+
361+
match result {
362+
Err(rusqlite::Error::SqliteFailure(rusqlite::ffi::Error { code, extended_code }, Some(ref _message))) if code == rusqlite::ErrorCode::ConstraintViolation && extended_code == 1555 && counter < 10 => {
363+
/* Retry if ID is already existent */
364+
counter += 1;
365+
continue;
366+
},
367+
Err(err) => break Err(err),
368+
Ok(rows) => { debug_assert!(rows == 1); break Ok(id) },
369+
}
370+
}
371+
}).await??;
353372

354-
Ok(())
373+
Ok((id, entry))
355374
}
356375

357376
/// Get entire entry for `id`.
@@ -532,8 +551,7 @@ mod tests {
532551
..Default::default()
533552
};
534553

535-
let id = Id::from(1234u32);
536-
db.insert(id, entry).await?;
554+
let (id, _entry) = db.insert(entry).await?;
537555

538556
let entry = db.get(id, None).await?.unwrap_inner();
539557
assert_eq!(entry.text, "hello world");
@@ -555,8 +573,7 @@ mod tests {
555573
..Default::default()
556574
};
557575

558-
let id = Id::from(1234u32);
559-
db.insert(id, entry).await?;
576+
let (id, _entry) = db.insert(entry).await?;
560577

561578
tokio::time::sleep(tokio::time::Duration::from_secs(2)).await;
562579

@@ -570,8 +587,7 @@ mod tests {
570587
async fn delete() -> Result<(), Box<dyn std::error::Error>> {
571588
let db = new_db()?;
572589

573-
let id = Id::from(1234u32);
574-
db.insert(id, write::Entry::default()).await?;
590+
let (id, _entry) = db.insert(write::Entry::default()).await?;
575591

576592
assert!(db.get(id, None).await.is_ok());
577593
assert!(db.delete(id).await.is_ok());
@@ -589,14 +605,13 @@ mod tests {
589605
..Default::default()
590606
};
591607

592-
let id = Id::from(1234u32);
593-
db.insert(id, entry).await?;
608+
let (id, _entry) = db.insert(entry).await?;
594609

595610
tokio::time::sleep(tokio::time::Duration::from_secs(2)).await;
596611

597612
let ids = db.purge()?;
598613
assert_eq!(ids.len(), 1);
599-
assert_eq!(ids[0].to_i64(), 1234);
614+
assert_eq!(ids[0], id);
600615

601616
Ok(())
602617
}

crates/wastebin_server/src/handlers/insert/api.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,9 @@ pub async fn post(
3939
State(db): State<Database>,
4040
Json(entry): Json<Entry>,
4141
) -> Result<Json<RedirectResponse>, JsonErrorResponse> {
42-
let id = Id::rand();
4342
let entry: write::Entry = entry.into();
43+
let (id, entry) = db.insert(entry).await.map_err(Error::Database)?;
4444
let path = format!("/{}", id.to_url_path(&entry));
45-
db.insert(id, entry).await.map_err(Error::Database)?;
4645

4746
Ok(Json::from(RedirectResponse { path }))
4847
}

crates/wastebin_server/src/handlers/insert/form.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,14 @@ pub async fn post<E: std::fmt::Debug>(
7979
let mut entry: write::Entry = entry.into();
8080
entry.uid = Some(uid);
8181

82-
let id = Id::rand();
82+
let (id, entry) = db.insert(entry).await?;
83+
8384
let mut url = id.to_url_path(&entry);
8485

8586
if entry.burn_after_reading.unwrap_or(false) {
8687
url = format!("burn/{url}");
8788
}
8889

89-
db.insert(id, entry).await?;
9090
let url = format!("/{url}");
9191

9292
let cookie = Cookie::build(("uid", uid.to_string()))

0 commit comments

Comments
 (0)