Deuxfleurs/garage
44
56
Fork 74
Code Issues 100 Pull requests 14 Releases 30 Wiki Activity

Support website publishing #7

Merged
lx merged 61 commits from feature/website into master 2021-01-15 16:49:51 +00:00
Conversation 3 Commits 61 Files changed 17 +10 -5
1 changed files with 10 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit fad7bc405b - Show all commits

15
src/web/web_server.rs
View file

@ -87,9 +87,8 @@ async fn serve_file(garage: Arc<Garage>, req: Request<Body>) -> Result<Response<
.ok_or(Error::NotFound)?; .ok_or(Error::NotFound)?;
match bucket_desc.state.get() { match bucket_desc.state.get() {
BucketState::Deleted => Err(Error::NotFound), BucketState::Present(params) if *params.website.get() => Ok(()),
BucketState::Present(params) if !params.website.get() => Err(Error::NotFound), _ => Err(Error::NotFound),
_ => Ok(()),
}?; }?;
quentin marked this conversation as resolved
lx commented 2021-01-13 21:14:43 +00:00
Review
Copy link

Rewrite as only two cases:

BucketState::Present(params) if params.website.get() => Ok(()),
_ => Err(Error::NotFound),
Rewrite as only two cases: ``` BucketState::Present(params) if params.website.get() => Ok(()), _ => Err(Error::NotFound), ```
// Get path // Get path
@ -123,8 +122,10 @@ fn authority_to_host(authority: &str) -> Result<&str, Error> {
let split = match first_char { let split = match first_char {
'[' => { '[' => {
let mut iter = iter.skip_while(|(_, c)| c != &']'); let mut iter = iter.skip_while(|(_, c)| c != &']');
iter.next().expect("Authority parsing logic error"); match iter.next() {
iter.next() Some((_, ']')) => iter.next(),
quentin marked this conversation as resolved Outdated
lx commented 2021-01-13 21:17:16 +00:00
Outdated
Review
Copy link

NO!! expect makes us crash (panic) if the condition is not verified, this is not acceptable (we don't want to assume that authority is well formed, maybe hyper checks it for us but maybe not and we don't want to take risks)

NO!! `expect` makes us crash (panic) if the condition is not verified, this is not acceptable (we don't want to assume that authority is well formed, maybe hyper checks it for us but maybe not and we don't want to take risks)
_ => None,
}
} }
_ => iter.skip_while(|(_, c)| c != &':').next(), _ => iter.skip_while(|(_, c)| c != &':').next(),
}; };
@ -214,6 +215,10 @@ mod tests {
assert_eq!(domain2, "garage.tld"); assert_eq!(domain2, "garage.tld");
let domain3 = authority_to_host("127.0.0.1")?; let domain3 = authority_to_host("127.0.0.1")?;
assert_eq!(domain3, "127.0.0.1"); assert_eq!(domain3, "127.0.0.1");
let domain4 = authority_to_host("[")?;
assert_eq!(domain4, "[");
let domain5 = authority_to_host("[hello")?;
assert_eq!(domain5, "[hello");
Ok(()) Ok(())
} }