Skip to content

Commit bc9263c

Browse files
committed
Allow per-user rate limit overrides
Since this is primarily an anti-spam mechanism, if a legitimate user really is just trying to publish 50 crates at once or something we want to be able to give them that ability. There's no UI for this, as it's not expected to be used often (if ever), so it'll require someone with prod access to add the override for now. I expect the bar for setting this to be pretty low. Mostly "you are clearly actually trying to upload a bunch of real crates and are not a spammer".
1 parent 69a0f1e commit bc9263c

File tree

7 files changed

+83
-23
lines changed

7 files changed

+83
-23
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DROP TABLE publish_rate_overrides;
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
CREATE TABLE publish_rate_overrides (
2+
user_id INTEGER PRIMARY KEY REFERENCES users,
3+
burst INTEGER NOT NULL
4+
);

src/publish_rate_limit.rs

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use diesel::data_types::PgInterval;
33
use diesel::prelude::*;
44
use std::time::Duration;
55

6-
use crate::schema::publish_limit_buckets;
6+
use crate::schema::{publish_limit_buckets, publish_rate_overrides};
77
use crate::util::errors::{CargoResult, TooManyRequests};
88

99
#[derive(Debug, Clone, Copy)]
@@ -68,6 +68,13 @@ impl PublishRateLimit {
6868
sql_function!(fn greatest<T>(x: T, y: T) -> T);
6969
sql_function!(fn least<T>(x: T, y: T) -> T);
7070

71+
let burst = publish_rate_overrides::table
72+
.find(uploader)
73+
.select(publish_rate_overrides::burst)
74+
.first::<i32>(conn)
75+
.optional()?
76+
.unwrap_or(self.burst);
77+
7178
// Interval division is poorly defined in general (what is 1 month / 30 days?)
7279
// However, for the intervals we're dealing with, it is always well
7380
// defined, so we convert to an f64 of seconds to represent this.
@@ -79,13 +86,13 @@ impl PublishRateLimit {
7986
diesel::insert_into(publish_limit_buckets)
8087
.values((
8188
user_id.eq(uploader),
82-
tokens.eq(self.burst),
89+
tokens.eq(burst),
8390
last_refill.eq(now),
8491
))
8592
.on_conflict(user_id)
8693
.do_update()
8794
.set((
88-
tokens.eq(least(self.burst, greatest(0, tokens - 1) + tokens_to_add)),
95+
tokens.eq(least(burst, greatest(0, tokens - 1) + tokens_to_add)),
8996
last_refill
9097
.eq(last_refill + self.refill_rate().into_sql::<Interval>() * tokens_to_add),
9198
))
@@ -288,6 +295,33 @@ mod tests {
288295
Ok(())
289296
}
290297

298+
#[test]
299+
fn override_is_used_instead_of_global_burst_if_present() -> CargoResult<()> {
300+
let conn = pg_connection();
301+
let now = Utc::now().naive_utc();
302+
303+
let rate = PublishRateLimit {
304+
rate: Duration::from_secs(1),
305+
burst: 10,
306+
};
307+
let user_id = new_user(&conn, "user1")?;
308+
let other_user_id = new_user(&conn, "user2")?;
309+
310+
diesel::insert_into(publish_rate_overrides::table)
311+
.values((
312+
publish_rate_overrides::user_id.eq(user_id),
313+
publish_rate_overrides::burst.eq(20),
314+
))
315+
.execute(&conn)?;
316+
317+
let bucket = rate.take_token(user_id, now, &conn)?;
318+
let other_bucket = rate.take_token(other_user_id, now, &conn)?;
319+
320+
assert_eq!(20, bucket.tokens);
321+
assert_eq!(10, other_bucket.tokens);
322+
Ok(())
323+
}
324+
291325
fn new_user(conn: &PgConnection, gh_login: &str) -> CargoResult<i32> {
292326
use crate::models::NewUser;
293327

src/schema.patch

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ index df884e4..18e08cd 100644
5555

5656
/// Representation of the `reserved_crate_names` table.
5757
///
58-
@@ -881,22 +901,24 @@ table! {
58+
@@ -881,23 +901,25 @@ table! {
5959
}
6060

6161
joinable!(api_tokens -> users (user_id));
@@ -74,6 +74,7 @@ index df884e4..18e08cd 100644
7474
joinable!(follows -> crates (crate_id));
7575
joinable!(follows -> users (user_id));
7676
joinable!(publish_limit_buckets -> users (user_id));
77+
joinable!(publish_rate_overrides -> users (user_id));
7778
joinable!(readme_renderings -> versions (version_id));
7879
+joinable!(recent_crate_downloads -> crates (crate_id));
7980
joinable!(version_authors -> users (user_id));
@@ -82,12 +83,12 @@ index df884e4..18e08cd 100644
8283
joinable!(versions -> crates (crate_id));
8384

8485
@@ -913,13 +935,14 @@ allow_tables_to_appear_in_same_query!(
85-
dependencies,
8686
emails,
8787
follows,
8888
keywords,
8989
metadata,
9090
publish_limit_buckets,
91+
publish_rate_overrides,
9192
readme_renderings,
9293
+ recent_crate_downloads,
9394
reserved_crate_names,

src/schema.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,29 @@ table! {
621621
}
622622
}
623623

624+
table! {
625+
use diesel::sql_types::*;
626+
use diesel_full_text_search::{TsVector as Tsvector};
627+
628+
/// Representation of the `publish_rate_overrides` table.
629+
///
630+
/// (Automatically generated by Diesel.)
631+
publish_rate_overrides (user_id) {
632+
/// The `user_id` column of the `publish_rate_overrides` table.
633+
///
634+
/// Its SQL type is `Int4`.
635+
///
636+
/// (Automatically generated by Diesel.)
637+
user_id -> Int4,
638+
/// The `burst` column of the `publish_rate_overrides` table.
639+
///
640+
/// Its SQL type is `Int4`.
641+
///
642+
/// (Automatically generated by Diesel.)
643+
burst -> Int4,
644+
}
645+
}
646+
624647
table! {
625648
use diesel::sql_types::*;
626649
use diesel_full_text_search::{TsVector as Tsvector};
@@ -964,6 +987,7 @@ joinable!(emails -> users (user_id));
964987
joinable!(follows -> crates (crate_id));
965988
joinable!(follows -> users (user_id));
966989
joinable!(publish_limit_buckets -> users (user_id));
990+
joinable!(publish_rate_overrides -> users (user_id));
967991
joinable!(readme_renderings -> versions (version_id));
968992
joinable!(recent_crate_downloads -> crates (crate_id));
969993
joinable!(version_authors -> users (user_id));
@@ -989,6 +1013,7 @@ allow_tables_to_appear_in_same_query!(
9891013
keywords,
9901014
metadata,
9911015
publish_limit_buckets,
1016+
publish_rate_overrides,
9921017
readme_renderings,
9931018
recent_crate_downloads,
9941019
reserved_crate_names,

src/tests/krate.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,7 +2112,7 @@ fn publish_new_crate_rate_limited() {
21122112

21132113
// Uploading a second crate is limited
21142114
let crate_to_publish = PublishBuilder::new("rate_limited2");
2115-
token.publish(crate_to_publish).assert_status(529);
2115+
token.publish(crate_to_publish).assert_status(429);
21162116

21172117
anon.get::<()>("/api/v1/crates/rate_limited2")
21182118
.assert_status(404);
@@ -2141,7 +2141,3 @@ fn publish_rate_limit_doesnt_affect_existing_crates() {
21412141
.version("1.0.1");
21422142
token.publish(new_version).good();
21432143
}
2144-
2145-
#[test]
2146-
#[ignore]
2147-
fn publish_rate_limit_per_user_override() {}

src/util/errors.rs

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use std::any::{Any, TypeId};
2-
use std::collections::HashMap;
32
use std::error::Error;
43
use std::fmt;
54

@@ -397,19 +396,19 @@ impl CargoError for TooManyRequests {
397396
}
398397

399398
fn response(&self) -> Option<Response> {
400-
use std::io::Cursor;
401-
402399
const HTTP_DATE_FORMAT: &str = "%a, %d %b %Y %H:%M:%S GMT";
403-
let mut headers = HashMap::new();
404-
headers.insert(
405-
"Retry-After".into(),
406-
vec![self.retry_after.format(HTTP_DATE_FORMAT).to_string()],
407-
);
408-
Some(Response {
409-
status: (529, "TOO MANY REQUESTS"),
410-
headers,
411-
body: Box::new(Cursor::new(Vec::new())),
412-
})
400+
let retry_after = self.retry_after.format(HTTP_DATE_FORMAT);
401+
402+
let mut response = json_response(&Bad {
403+
errors: vec![StringError {
404+
detail: format!("You have published too many crates in a \
405+
short period of time. Please try again after {} or email \
406+
[email protected] to have your limit increased.", retry_after),
407+
}],
408+
});
409+
response.status = (429, "TOO MANY REQUESTS");
410+
response.headers.insert("Retry-After".into(), vec![retry_after.to_string()]);
411+
Some(response)
413412
}
414413

415414
fn human(&self) -> bool {

0 commit comments

Comments
 (0)