Skip to content

Commit

Permalink
Allow per-crate max upload sizes
Browse files Browse the repository at this point in the history
There's still a global limit on the nginx server but each crate can now have its
own maximum limit as well which is larger than the standard limit.

Closes #40
Closes #195
  • Loading branch information
alexcrichton committed Nov 19, 2015
1 parent 555a75b commit a147011
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 56 deletions.
5 changes: 2 additions & 3 deletions config/nginx.conf.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ http {
default_type application/octet-stream;
sendfile on;

#Must read the body in 5 seconds.
client_body_timeout 5;
client_max_body_size 10m;
client_body_timeout 30;
client_max_body_size 50m;

upstream app_server {
server localhost:8888 fail_timeout=0;
Expand Down
2 changes: 2 additions & 0 deletions src/bin/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,8 @@ fn migrations() -> Vec<Migration> {
UNIQUE (owner_id, crate_id)", &[]));
Ok(())
}),
Migration::add_column(20151118135514, "crates", "max_upload_size",
"INTEGER"),
];
// NOTE: Generate a new id via `date +"%Y%m%d%H%M%S"`

Expand Down
6 changes: 3 additions & 3 deletions src/bin/update-downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ mod test {
let user = user(&tx);
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None, &None,
&None, &None, &[], &None, &None,
&None).unwrap();
&None, None).unwrap();
let version = Version::insert(&tx, krate.id,
&semver::Version::parse("1.0.0").unwrap(),
&HashMap::new(), &[]).unwrap();
Expand All @@ -189,7 +189,7 @@ mod test {
let user = user(&tx);
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None,
&None, &None, &None, &[], &None,
&None, &None).unwrap();
&None, &None, None).unwrap();
let version = Version::insert(&tx, krate.id,
&semver::Version::parse("1.0.0").unwrap(),
&HashMap::new(), &[]).unwrap();
Expand All @@ -212,7 +212,7 @@ mod test {
let user = user(&tx);
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None,
&None, &None, &None, &[], &None,
&None, &None).unwrap();
&None, &None, None).unwrap();
let version = Version::insert(&tx, krate.id,
&semver::Version::parse("1.0.0").unwrap(),
&HashMap::new(), &[]).unwrap();
Expand Down
45 changes: 27 additions & 18 deletions src/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::cmp;
use std::collections::HashMap;
use std::io::prelude::*;
use std::io;
use std::iter::repeat;
use std::mem;
use std::sync::Arc;

Expand Down Expand Up @@ -52,6 +51,7 @@ pub struct Crate {
pub keywords: Vec<String>,
pub license: Option<String>,
pub repository: Option<String>,
pub max_upload_size: Option<i32>,
}

#[derive(RustcEncodable, RustcDecodable)]
Expand Down Expand Up @@ -101,7 +101,9 @@ impl Crate {
keywords: &[String],
repository: &Option<String>,
license: &Option<String>,
license_file: &Option<String>) -> CargoResult<Crate> {
license_file: &Option<String>,
max_upload_size: Option<i32>)
-> CargoResult<Crate> {
let description = description.as_ref().map(|s| &s[..]);
let homepage = homepage.as_ref().map(|s| &s[..]);
let documentation = documentation.as_ref().map(|s| &s[..]);
Expand Down Expand Up @@ -161,15 +163,16 @@ impl Crate {
(name, user_id, created_at,
updated_at, downloads, max_version,
description, homepage, documentation,
readme, keywords, repository, license)
readme, keywords, repository, license,
max_upload_size)
VALUES ($1, $2, $3, $3, 0, '0.0.0',
$4, $5, $6, $7, $8, $9, $10)
$4, $5, $6, $7, $8, $9, $10, $11)
RETURNING *"));
let now = ::now();
let rows = try!(stmt.query(&[&name, &user_id, &now,
&description, &homepage,
&documentation, &readme, &keywords,
&repository, &license]));
&repository, &license, &max_upload_size]));
let ret: Crate = Model::from_row(&try!(rows.iter().next().chain_error(|| {
internal("no crate returned")
})));
Expand Down Expand Up @@ -241,7 +244,7 @@ impl Crate {
let Crate {
name, created_at, updated_at, downloads, max_version, description,
homepage, documentation, keywords, license, repository,
readme: _, id: _, user_id: _,
readme: _, id: _, user_id: _, max_upload_size: _,
} = self;
let versions_link = match versions {
Some(..) => None,
Expand Down Expand Up @@ -453,6 +456,7 @@ impl Model for Crate {
.map(|s| s.to_string()).collect(),
license: row.get("license"),
repository: row.get("repository"),
max_upload_size: row.get("max_upload_size"),
}
}
fn table_name(_: Option<Crate>) -> &'static str { "crates" }
Expand Down Expand Up @@ -675,7 +679,8 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
&keywords,
&new_crate.repository,
&new_crate.license,
&new_crate.license_file));
&new_crate.license_file,
None));

let owners = try!(krate.owners(try!(req.tx())));
if try!(rights(req.app(), &owners, &user)) < Rights::Publish {
Expand All @@ -687,6 +692,16 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
return Err(human(format!("crate was previously named `{}`", krate.name)))
}

let length = try!(req.content_length().chain_error(|| {
human("missing header: Content-Length")
}));
let max = krate.max_upload_size.map(|m| m as u64)
.unwrap_or(app.config.max_upload_size);
if length > max {
println!("guth");
return Err(human(format!("max upload size is: {}", max)))
}

// Persist the new version of this crate
let mut version = try!(krate.add_version(try!(req.tx()), vers, &features,
&new_crate.authors));
Expand All @@ -706,7 +721,7 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
let path = krate.s3_path(&vers.to_string());
let (resp, cksum) = {
let length = try!(read_le_u32(req.body()));
let body = LimitErrorReader::new(req.body(), app.config.max_upload_size);
let body = LimitErrorReader::new(req.body(), max);
let mut body = HashingReader::new(body);
let resp = {
let s3req = app.bucket.put(&mut handle, &path, &mut body,
Expand Down Expand Up @@ -761,19 +776,13 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
}

fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> {
// Make sure the tarball being uploaded looks sane
let length = try!(req.content_length().chain_error(|| {
human("missing header: Content-Length")
}));
// Read the json upload request
let amt = try!(read_le_u32(req.body())) as u64;
let max = req.app().config.max_upload_size;
if length > max {
if amt > max {
return Err(human(format!("max upload size is: {}", max)))
}

// Read the json upload request
let amt = try!(read_le_u32(req.body())) as u64;
if amt > max { return Err(human(format!("max upload size is: {}", max))) }
let mut json = repeat(0).take(amt as usize).collect::<Vec<_>>();
let mut json = vec![0; amt as usize];
try!(read_fill(req.body(), &mut json));
let json = try!(String::from_utf8(json).map_err(|_| {
human("json body was not valid utf-8")
Expand Down
30 changes: 19 additions & 11 deletions src/tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ fn krate(name: &str) -> Crate {
keywords: Vec::new(),
license: None,
repository: None,
max_upload_size: None,
}
}

Expand All @@ -230,14 +231,15 @@ fn mock_crate_vers(req: &mut Request, krate: Crate, v: &semver::Version)
-> (Crate, Version) {
let user = req.extensions().find::<User>().unwrap();
let mut krate = Crate::find_or_insert(req.tx().unwrap(), &krate.name,
user.id, &krate.description,
&krate.homepage,
&krate.documentation,
&krate.readme,
&krate.keywords,
&krate.repository,
&krate.license,
&None).unwrap();
user.id, &krate.description,
&krate.homepage,
&krate.documentation,
&krate.readme,
&krate.keywords,
&krate.repository,
&krate.license,
&None,
krate.max_upload_size).unwrap();
Keyword::update_crate(req.tx().unwrap(), &krate,
&krate.keywords).unwrap();
let v = krate.add_version(req.tx().unwrap(), v, &HashMap::new(), &[]);
Expand Down Expand Up @@ -292,10 +294,10 @@ fn new_req_body(krate: Crate, version: &str, deps: Vec<u::CrateDependency>)
license: Some("MIT".to_string()),
license_file: None,
repository: krate.repository,
})
}, &[])
}

fn new_crate_to_body(new_crate: &u::NewCrate) -> Vec<u8> {
fn new_crate_to_body(new_crate: &u::NewCrate, krate: &[u8]) -> Vec<u8> {
let json = json::encode(&new_crate).unwrap();
let mut body = Vec::new();
body.extend([
Expand All @@ -305,6 +307,12 @@ fn new_crate_to_body(new_crate: &u::NewCrate) -> Vec<u8> {
(json.len() >> 24) as u8,
].iter().cloned());
body.extend(json.as_bytes().iter().cloned());
body.extend([0, 0, 0, 0].iter().cloned());
body.extend(&[
(krate.len() >> 0) as u8,
(krate.len() >> 8) as u8,
(krate.len() >> 16) as u8,
(krate.len() >> 24) as u8,
]);
body.extend(krate);
body
}
21 changes: 21 additions & 0 deletions src/tests/http-data/krate_new_krate_too_big_but_whitelisted
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
===REQUEST 2334
PUT http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.1.0.crate HTTP/1.1
Accept: */*
Proxy-Connection: Keep-Alive
Content-Type: application/x-tar
Date: Wed, 18 Nov 2015 14:29:12 -0800
Authorization: AWS AKIAI3HZFJJPEQRCEPYQ:WQPwbR4xY3GNBJC7TQ+sys8zVKw=
Host: alexcrichton-test.s3.amazonaws.com
Content-Length: 2000

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
===RESPONSE 258
HTTP/1.1 200
server: AmazonS3
etag: "7c1c566ab4cdb11ac8971191694e8bec"
content-length: 0
date: Wed, 18 Nov 2015 22:29:13 GMT
x-amz-request-id: 0F61951F1BCD0727
x-amz-id-2: JLgACuYs2DRQQf3vqYu6a3Bv0+PBuor+HcEGwKsnQjYtupRNAn/X/7KSytGWnTJS7VH+BPLFVJU=


61 changes: 40 additions & 21 deletions src/tests/krate.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::collections::HashMap;
use std::io::prelude::*;
use std::fs::{self, File};
use std::iter::repeat;

use conduit::{Handler, Request, Method};
use conduit_test::MockRequest;
Expand Down Expand Up @@ -35,6 +34,24 @@ struct RevDeps { dependencies: Vec<EncodableDependency>, meta: CrateMeta }
#[derive(RustcDecodable)]
struct Downloads { version_downloads: Vec<EncodableVersionDownload> }

fn new_crate(name: &str) -> u::NewCrate {
u::NewCrate {
name: u::CrateName(name.to_string()),
vers: u::CrateVersion(semver::Version::parse("1.1.0").unwrap()),
features: HashMap::new(),
deps: Vec::new(),
authors: vec!["foo".to_string()],
description: Some("desc".to_string()),
homepage: None,
documentation: None,
readme: None,
keywords: None,
license: Some("MIT".to_string()),
license_file: None,
repository: None,
}
}

#[test]
fn index() {
let (_b, _app, mut middle) = ::app();
Expand Down Expand Up @@ -332,8 +349,21 @@ fn new_krate_too_big() {
let (_b, app, middle) = ::app();
let mut req = ::new_req(app, "foo", "1.0.0");
::mock_user(&mut req, ::user("foo"));
req.with_body(repeat("a").take(1000 * 1000).collect::<String>().as_bytes());
bad_resp!(middle.call(&mut req));
let body = ::new_crate_to_body(&new_crate("foo"), &[b'a'; 2000]);
bad_resp!(middle.call(req.with_body(&body)));
}

#[test]
fn new_krate_too_big_but_whitelisted() {
let (_b, app, middle) = ::app();
let mut req = ::new_req(app, "foo", "1.1.0");
::mock_user(&mut req, ::user("foo"));
let mut krate = ::krate("foo");
krate.max_upload_size = Some(2 * 1000 * 1000);
::mock_crate(&mut req, krate);
let body = ::new_crate_to_body(&new_crate("foo"), &[b'a'; 2000]);
let mut response = ok_resp!(middle.call(req.with_body(&body)));
::json::<GoodCrate>(&mut response);
}

#[test]
Expand Down Expand Up @@ -759,22 +789,11 @@ fn author_license_and_description_required() {
::user("foo");

let mut req = ::req(app, Method::Put, "/api/v1/crates/new");
let mut new_crate = u::NewCrate {
name: u::CrateName("foo".to_string()),
vers: u::CrateVersion(semver::Version::parse("1.0.0").unwrap()),
features: HashMap::new(),
deps: Vec::new(),
authors: Vec::new(),
description: None,
homepage: None,
documentation: None,
readme: None,
keywords: None,
license: None,
license_file: None,
repository: None,
};
req.with_body(&::new_crate_to_body(&new_crate));
let mut new_crate = new_crate("foo");
new_crate.license = None;
new_crate.description = None;
new_crate.authors = Vec::new();
req.with_body(&::new_crate_to_body(&new_crate, &[]));
let json = bad_resp!(middle.call(&mut req));
assert!(json.errors[0].detail.contains("author") &&
json.errors[0].detail.contains("description") &&
Expand All @@ -783,7 +802,7 @@ fn author_license_and_description_required() {

new_crate.license = Some("MIT".to_string());
new_crate.authors.push("".to_string());
req.with_body(&::new_crate_to_body(&new_crate));
req.with_body(&::new_crate_to_body(&new_crate, &[]));
let json = bad_resp!(middle.call(&mut req));
assert!(json.errors[0].detail.contains("author") &&
json.errors[0].detail.contains("description") &&
Expand All @@ -793,7 +812,7 @@ fn author_license_and_description_required() {
new_crate.license = None;
new_crate.license_file = Some("foo".to_string());
new_crate.authors.push("foo".to_string());
req.with_body(&::new_crate_to_body(&new_crate));
req.with_body(&::new_crate_to_body(&new_crate, &[]));
let json = bad_resp!(middle.call(&mut req));
assert!(!json.errors[0].detail.contains("author") &&
json.errors[0].detail.contains("description") &&
Expand Down

0 comments on commit a147011

Please sign in to comment.