Skip to content

Allow per-crate max upload sizes #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 19, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
44 changes: 26 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,15 @@ 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 {
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 +720,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 +775,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