Skip to content

Commit 967c162

Browse files
authored
Merge pull request #6851 from Turbo87/fix-download-urls
Fix download URL generation
2 parents 94ae0ee + d6b14d0 commit 967c162

File tree

2 files changed

+95
-34
lines changed

2 files changed

+95
-34
lines changed

src/storage.rs

Lines changed: 91 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,15 @@ const CACHE_CONTROL_README: &str = "public,max-age=604800";
3232

3333
type StdPath = std::path::Path;
3434

35+
#[derive(Debug)]
36+
pub struct StorageConfig {
37+
backend: StorageBackend,
38+
pub cdn_prefix: Option<String>,
39+
}
40+
3541
#[derive(Debug)]
3642
#[allow(clippy::large_enum_variant)]
37-
pub enum StorageConfig {
43+
pub enum StorageBackend {
3844
S3 { default: S3Config, index: S3Config },
3945
LocalFileSystem { path: PathBuf },
4046
InMemory,
@@ -46,10 +52,16 @@ pub struct S3Config {
4652
region: Option<String>,
4753
access_key: String,
4854
secret_key: SecretString,
49-
cdn_prefix: Option<String>,
5055
}
5156

5257
impl StorageConfig {
58+
pub fn in_memory() -> Self {
59+
Self {
60+
backend: StorageBackend::InMemory,
61+
cdn_prefix: None,
62+
}
63+
}
64+
5365
pub fn from_environment() -> Self {
5466
if let Ok(bucket) = dotenvy::var("S3_BUCKET") {
5567
let region = dotenvy::var("S3_REGION").ok();
@@ -66,18 +78,21 @@ impl StorageConfig {
6678
region,
6779
access_key: access_key.clone(),
6880
secret_key: secret_key.clone(),
69-
cdn_prefix,
7081
};
7182

7283
let index = S3Config {
7384
bucket: index_bucket,
7485
region: index_region,
7586
access_key,
7687
secret_key,
77-
cdn_prefix: None,
7888
};
7989

80-
return Self::S3 { default, index };
90+
let backend = StorageBackend::S3 { default, index };
91+
92+
return Self {
93+
backend,
94+
cdn_prefix,
95+
};
8196
}
8297

8398
let current_dir = std::env::current_dir()
@@ -86,16 +101,22 @@ impl StorageConfig {
86101

87102
let path = current_dir.join("local_uploads");
88103

89-
Self::LocalFileSystem { path }
104+
let backend = StorageBackend::LocalFileSystem { path };
105+
106+
Self {
107+
backend,
108+
cdn_prefix: None,
109+
}
90110
}
91111
}
92112

93113
pub struct Storage {
114+
cdn_prefix: Option<String>,
115+
94116
store: Box<dyn ObjectStore>,
95117
crate_upload_store: Box<dyn ObjectStore>,
96118
readme_upload_store: Box<dyn ObjectStore>,
97119
db_dump_upload_store: Box<dyn ObjectStore>,
98-
cdn_prefix: String,
99120

100121
index_store: Box<dyn ObjectStore>,
101122
index_upload_store: Box<dyn ObjectStore>,
@@ -107,8 +128,10 @@ impl Storage {
107128
}
108129

109130
pub fn from_config(config: &StorageConfig) -> Self {
110-
match config {
111-
StorageConfig::S3 { default, index } => {
131+
let cdn_prefix = config.cdn_prefix.clone();
132+
133+
match &config.backend {
134+
StorageBackend::S3 { default, index } => {
112135
let options = ClientOptions::default();
113136
let store = build_s3(default, options);
114137

@@ -122,20 +145,16 @@ impl Storage {
122145
ClientOptions::default().with_default_content_type(CONTENT_TYPE_DB_DUMP);
123146
let db_dump_upload_store = build_s3(default, options);
124147

125-
let cdn_prefix = match default.cdn_prefix.as_ref() {
126-
None => panic!("Missing S3_CDN environment variable"),
127-
Some(cdn_prefix) if !cdn_prefix.starts_with("https://") => {
128-
format!("https://{cdn_prefix}")
129-
}
130-
Some(cdn_prefix) => cdn_prefix.clone(),
131-
};
132-
133148
let options = ClientOptions::default();
134149
let index_store = build_s3(index, options);
135150

136151
let options = client_options(CONTENT_TYPE_INDEX, CACHE_CONTROL_INDEX);
137152
let index_upload_store = build_s3(index, options);
138153

154+
if cdn_prefix.is_none() {
155+
panic!("Missing S3_CDN environment variable");
156+
}
157+
139158
Self {
140159
store: Box::new(store),
141160
crate_upload_store: Box::new(crate_upload_store),
@@ -147,7 +166,7 @@ impl Storage {
147166
}
148167
}
149168

150-
StorageConfig::LocalFileSystem { path } => {
169+
StorageBackend::LocalFileSystem { path } => {
151170
warn!(?path, "Using local file system for file storage");
152171

153172
let index_path = path.join("index");
@@ -172,13 +191,13 @@ impl Storage {
172191
crate_upload_store: Box::new(store.clone()),
173192
readme_upload_store: Box::new(store.clone()),
174193
db_dump_upload_store: Box::new(store),
175-
cdn_prefix: "/".into(),
194+
cdn_prefix,
176195
index_store: Box::new(index_store.clone()),
177196
index_upload_store: Box::new(index_store),
178197
}
179198
}
180199

181-
StorageConfig::InMemory => {
200+
StorageBackend::InMemory => {
182201
warn!("Using in-memory file storage");
183202
let store = ArcStore::new(InMemory::new());
184203

@@ -187,7 +206,7 @@ impl Storage {
187206
crate_upload_store: Box::new(store.clone()),
188207
readme_upload_store: Box::new(store.clone()),
189208
db_dump_upload_store: Box::new(store.clone()),
190-
cdn_prefix: "/".into(),
209+
cdn_prefix,
191210
index_store: Box::new(PrefixStore::new(store.clone(), "index")),
192211
index_upload_store: Box::new(PrefixStore::new(store, "index")),
193212
}
@@ -199,14 +218,14 @@ impl Storage {
199218
///
200219
/// The function doesn't check for the existence of the file.
201220
pub fn crate_location(&self, name: &str, version: &str) -> String {
202-
format!("{}{}", self.cdn_prefix, crate_file_path(name, version)).replace('+', "%2B")
221+
apply_cdn_prefix(&self.cdn_prefix, &crate_file_path(name, version)).replace('+', "%2B")
203222
}
204223

205224
/// Returns the URL of an uploaded crate's version readme.
206225
///
207226
/// The function doesn't check for the existence of the file.
208227
pub fn readme_location(&self, name: &str, version: &str) -> String {
209-
format!("{}{}", self.cdn_prefix, readme_path(name, version)).replace('+', "%2B")
228+
apply_cdn_prefix(&self.cdn_prefix, &readme_path(name, version)).replace('+', "%2B")
210229
}
211230

212231
#[instrument(skip(self))]
@@ -326,14 +345,24 @@ fn readme_path(name: &str, version: &str) -> Path {
326345
format!("{PREFIX_READMES}/{name}/{name}-{version}.html").into()
327346
}
328347

348+
fn apply_cdn_prefix(cdn_prefix: &Option<String>, path: &Path) -> String {
349+
match cdn_prefix {
350+
Some(cdn_prefix) if !cdn_prefix.starts_with("https://") => {
351+
format!("https://{cdn_prefix}/{path}")
352+
}
353+
Some(cdn_prefix) => format!("{cdn_prefix}/{path}"),
354+
None => format!("/{path}"),
355+
}
356+
}
357+
329358
#[cfg(test)]
330359
mod tests {
331360
use super::*;
332361
use hyper::body::Bytes;
333362
use tempfile::NamedTempFile;
334363

335364
pub async fn prepare() -> Storage {
336-
let storage = Storage::from_config(&StorageConfig::InMemory);
365+
let storage = Storage::from_config(&StorageConfig::in_memory());
337366

338367
let files_to_create = vec![
339368
"crates/bar/bar-2.0.0.crate",
@@ -361,33 +390,62 @@ mod tests {
361390

362391
#[test]
363392
fn locations() {
364-
let storage = Storage::from_config(&StorageConfig::InMemory);
393+
let mut config = StorageConfig::in_memory();
394+
config.cdn_prefix = Some("static.crates.io".to_string());
395+
396+
let storage = Storage::from_config(&config);
365397

366398
let crate_tests = vec![
367-
("foo", "1.2.3", "/crates/foo/foo-1.2.3.crate"),
399+
("foo", "1.2.3", "https://static.crates.io/crates/foo/foo-1.2.3.crate"),
368400
(
369401
"some-long-crate-name",
370402
"42.0.5-beta.1+foo",
371-
"/crates/some-long-crate-name/some-long-crate-name-42.0.5-beta.1%2Bfoo.crate",
403+
"https://static.crates.io/crates/some-long-crate-name/some-long-crate-name-42.0.5-beta.1%2Bfoo.crate",
372404
),
373405
];
374406
for (name, version, expected) in crate_tests {
375407
assert_eq!(storage.crate_location(name, version), expected);
376408
}
377409

378410
let readme_tests = vec![
379-
("foo", "1.2.3", "/readmes/foo/foo-1.2.3.html"),
411+
("foo", "1.2.3", "https://static.crates.io/readmes/foo/foo-1.2.3.html"),
380412
(
381413
"some-long-crate-name",
382414
"42.0.5-beta.1+foo",
383-
"/readmes/some-long-crate-name/some-long-crate-name-42.0.5-beta.1%2Bfoo.html",
415+
"https://static.crates.io/readmes/some-long-crate-name/some-long-crate-name-42.0.5-beta.1%2Bfoo.html",
384416
),
385417
];
386418
for (name, version, expected) in readme_tests {
387419
assert_eq!(storage.readme_location(name, version), expected);
388420
}
389421
}
390422

423+
#[test]
424+
fn cdn_prefix() {
425+
assert_eq!(apply_cdn_prefix(&None, &"foo".into()), "/foo");
426+
assert_eq!(
427+
apply_cdn_prefix(&Some("static.crates.io".to_string()), &"foo".into()),
428+
"https://static.crates.io/foo"
429+
);
430+
assert_eq!(
431+
apply_cdn_prefix(
432+
&Some("https://fastly-static.crates.io".to_string()),
433+
&"foo".into()
434+
),
435+
"https://fastly-static.crates.io/foo"
436+
);
437+
438+
assert_eq!(
439+
apply_cdn_prefix(&Some("static.crates.io".to_string()), &"/foo/bar".into()),
440+
"https://static.crates.io/foo/bar"
441+
);
442+
443+
assert_eq!(
444+
apply_cdn_prefix(&Some("static.crates.io/".to_string()), &"/foo/bar".into()),
445+
"https://static.crates.io//foo/bar"
446+
);
447+
}
448+
391449
#[tokio::test]
392450
async fn delete_all_crate_files() {
393451
let storage = prepare().await;
@@ -452,7 +510,7 @@ mod tests {
452510

453511
#[tokio::test]
454512
async fn upload_crate_file() {
455-
let s = Storage::from_config(&StorageConfig::InMemory);
513+
let s = Storage::from_config(&StorageConfig::in_memory());
456514

457515
s.upload_crate_file("foo", "1.2.3", Bytes::new())
458516
.await
@@ -474,7 +532,7 @@ mod tests {
474532

475533
#[tokio::test]
476534
async fn upload_readme() {
477-
let s = Storage::from_config(&StorageConfig::InMemory);
535+
let s = Storage::from_config(&StorageConfig::in_memory());
478536

479537
let bytes = Bytes::from_static(b"hello world");
480538
s.upload_readme("foo", "1.2.3", bytes.clone())
@@ -495,7 +553,7 @@ mod tests {
495553

496554
#[tokio::test]
497555
async fn sync_index() {
498-
let s = Storage::from_config(&StorageConfig::InMemory);
556+
let s = Storage::from_config(&StorageConfig::in_memory());
499557

500558
assert!(stored_files(&s.store).await.is_empty());
501559

@@ -512,7 +570,7 @@ mod tests {
512570

513571
#[tokio::test]
514572
async fn upload_db_dump() {
515-
let s = Storage::from_config(&StorageConfig::InMemory);
573+
let s = Storage::from_config(&StorageConfig::in_memory());
516574

517575
assert!(stored_files(&s.store).await.is_empty());
518576

src/tests/util/test_app.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,14 +380,17 @@ fn simple_config() -> config::Server {
380380
dl_only_at_percentage: 80,
381381
};
382382

383+
let mut storage = StorageConfig::in_memory();
384+
storage.cdn_prefix = Some("static.crates.io".to_string());
385+
383386
config::Server {
384387
base,
385388
ip: [127, 0, 0, 1].into(),
386389
port: 8888,
387390
max_blocking_threads: None,
388391
use_nginx_wrapper: false,
389392
db,
390-
storage: StorageConfig::InMemory,
393+
storage,
391394
session_key: cookie::Key::derive_from("test this has to be over 32 bytes long".as_bytes()),
392395
gh_client_id: ClientId::new(dotenvy::var("GH_CLIENT_ID").unwrap_or_default()),
393396
gh_client_secret: ClientSecret::new(dotenvy::var("GH_CLIENT_SECRET").unwrap_or_default()),

0 commit comments

Comments
 (0)