diff options
author | Florian Klink <flokli@flokli.de> | 2024-08-21T08·06+0300 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2024-08-21T09·40+0000 |
commit | e03ea11badbf40971d6bf87eede33fe3b046c98b (patch) | |
tree | d520dedadeb8193b2da323adb2623a269217951c /tvix/nar-bridge/src | |
parent | 2357079891819c679821ea58a7715de7be431aaa (diff) |
feat(nix-compat/nix_http): init parse_nar[info]_str r/8547
This moves the URL component parsing code we had in nar-bridge to nix-compat. We change the function signature to return an Option, not a Result<_, StatusCode>. This allows returning more appropriate error codes, as we can ok_or(…) at the callsite, which we now do: on an upload to an invalid path, we now return "unauthorized", while on a GET/HEAD, we return "not found". This also adds support to parse compression suffixes. While not supported in nar-bridge, other users of nix-compat might very well want to parse these paths. Also fix the error message when parsing NAR urls, it mentioned 32, not 52, which is a copypasta error from the narinfo URL parsing code. Change-Id: Id1be9a8044814b54ce68b125c52dfe933c9c4f74 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12260 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
Diffstat (limited to 'tvix/nar-bridge/src')
-rw-r--r-- | tvix/nar-bridge/src/nar.rs | 62 | ||||
-rw-r--r-- | tvix/nar-bridge/src/narinfo.rs | 54 |
2 files changed, 13 insertions, 103 deletions
diff --git a/tvix/nar-bridge/src/nar.rs b/tvix/nar-bridge/src/nar.rs index e43b220c5a0e..f4471e0735f1 100644 --- a/tvix/nar-bridge/src/nar.rs +++ b/tvix/nar-bridge/src/nar.rs @@ -5,7 +5,7 @@ use axum::response::Response; use bytes::Bytes; use data_encoding::BASE64URL_NOPAD; use futures::TryStreamExt; -use nix_compat::nixbase32; +use nix_compat::{nix_http, nixbase32}; use serde::Deserialize; use std::io; use tokio_util::io::ReaderStream; @@ -92,7 +92,14 @@ pub async fn put( }): axum::extract::State<AppState>, request: axum::extract::Request, ) -> Result<&'static str, StatusCode> { - let nar_hash_expected = parse_nar_str(&nar_str)?; + let (nar_hash_expected, compression_suffix) = + nix_http::parse_nar_str(&nar_str).ok_or(StatusCode::UNAUTHORIZED)?; + + // No paths with compression suffix are supported. + if !compression_suffix.is_empty() { + warn!(%compression_suffix, "invalid compression suffix requested"); + return Err(StatusCode::UNAUTHORIZED); + } let s = request.into_body().into_data_stream(); @@ -133,54 +140,3 @@ pub async fn put( // FUTUREWORK: maybe head by narhash. Though not too critical, as we do // implement HEAD for .narinfo. - -/// Parses a `14cx20k6z4hq508kqi2lm79qfld5f9mf7kiafpqsjs3zlmycza0k.nar` -/// string and returns the nixbase32-decoded digest. -/// No compression is supported. -fn parse_nar_str(s: &str) -> Result<[u8; 32], StatusCode> { - if !s.is_char_boundary(52) { - warn!("invalid string, no char boundary at 32"); - return Err(StatusCode::NOT_FOUND); - } - - Ok(match s.split_at(52) { - (hash_str, ".nar") => { - // we know this is 52 bytes - let hash_str_fixed: [u8; 52] = hash_str.as_bytes().try_into().unwrap(); - nixbase32::decode_fixed(hash_str_fixed).map_err(|e| { - warn!(err=%e, "invalid digest"); - StatusCode::NOT_FOUND - })? - } - _ => { - warn!("invalid string"); - return Err(StatusCode::BAD_REQUEST); - } - }) -} - -#[cfg(test)] -mod test { - use super::parse_nar_str; - use hex_literal::hex; - - #[test] - fn success() { - assert_eq!( - hex!("13a8cf7ca57f68a9f1752acee36a72a55187d3a954443c112818926f26109d91"), - parse_nar_str("14cx20k6z4hq508kqi2lm79qfld5f9mf7kiafpqsjs3zlmycza0k.nar").unwrap() - ) - } - - #[test] - fn failure() { - assert!( - parse_nar_str("14cx20k6z4hq508kqi2lm79qfld5f9mf7kiafpqsjs3zlmycza0k.nar.x").is_err() - ); - assert!( - parse_nar_str("14cx20k6z4hq508kqi2lm79qfld5f9mf7kiafpqsjs3zlmycza0k.nar.xz").is_err() - ); - assert!(parse_nar_str("14cx20k6z4hq508kqi2lm79qfld5f9mf7kiafpqsjs3zlmycza0").is_err()); - assert!(parse_nar_str("14cx20k6z4hq508kqi2lm79qfld5f9mf7kiafpqsjs3zlmycza0🦊.nar").is_err()) - } -} diff --git a/tvix/nar-bridge/src/narinfo.rs b/tvix/nar-bridge/src/narinfo.rs index f97ee970819d..afc4e650cbc6 100644 --- a/tvix/nar-bridge/src/narinfo.rs +++ b/tvix/nar-bridge/src/narinfo.rs @@ -1,6 +1,6 @@ use axum::http::StatusCode; use bytes::Bytes; -use nix_compat::{narinfo::NarInfo, nixbase32}; +use nix_compat::{narinfo::NarInfo, nix_http, nixbase32}; use prost::Message; use tracing::{instrument, warn, Span}; use tvix_castore::proto::{self as castorepb}; @@ -18,7 +18,7 @@ pub async fn head( path_info_service, .. }): axum::extract::State<AppState>, ) -> Result<&'static str, StatusCode> { - let digest = parse_narinfo_str(&narinfo_str)?; + let digest = nix_http::parse_narinfo_str(&narinfo_str).ok_or(StatusCode::NOT_FOUND)?; Span::current().record("path_info.digest", &narinfo_str[0..32]); if path_info_service @@ -44,7 +44,7 @@ pub async fn get( path_info_service, .. }): axum::extract::State<AppState>, ) -> Result<String, StatusCode> { - let digest = parse_narinfo_str(&narinfo_str)?; + let digest = nix_http::parse_narinfo_str(&narinfo_str).ok_or(StatusCode::NOT_FOUND)?; Span::current().record("path_info.digest", &narinfo_str[0..32]); // fetch the PathInfo @@ -101,7 +101,7 @@ pub async fn put( }): axum::extract::State<AppState>, request: axum::extract::Request, ) -> Result<&'static str, StatusCode> { - let _narinfo_digest = parse_narinfo_str(&narinfo_str)?; + let _narinfo_digest = nix_http::parse_narinfo_str(&narinfo_str).ok_or(StatusCode::UNAUTHORIZED); Span::current().record("path_info.digest", &narinfo_str[0..32]); let narinfo_bytes: Bytes = axum::body::to_bytes(request.into_body(), NARINFO_LIMIT) @@ -157,49 +157,3 @@ pub async fn put( } } } - -/// Parses a `3mzh8lvgbynm9daj7c82k2sfsfhrsfsy.narinfo` string and returns the -/// nixbase32-decoded digest. -fn parse_narinfo_str(s: &str) -> Result<[u8; 20], StatusCode> { - if !s.is_char_boundary(32) { - warn!("invalid string, no char boundary at 32"); - return Err(StatusCode::NOT_FOUND); - } - - Ok(match s.split_at(32) { - (hash_str, ".narinfo") => { - // we know this is 32 bytes - let hash_str_fixed: [u8; 32] = hash_str.as_bytes().try_into().unwrap(); - nixbase32::decode_fixed(hash_str_fixed).map_err(|e| { - warn!(err=%e, "invalid digest"); - StatusCode::NOT_FOUND - })? - } - _ => { - warn!("invalid string"); - return Err(StatusCode::NOT_FOUND); - } - }) -} - -#[cfg(test)] -mod test { - use super::parse_narinfo_str; - use hex_literal::hex; - - #[test] - fn success() { - assert_eq!( - hex!("8a12321522fd91efbd60ebb2481af88580f61600"), - parse_narinfo_str("00bgd045z0d4icpbc2yyz4gx48ak44la.narinfo").unwrap() - ); - } - - #[test] - fn failure() { - assert!(parse_narinfo_str("00bgd045z0d4icpbc2yyz4gx48ak44la").is_err()); - assert!(parse_narinfo_str("/00bgd045z0d4icpbc2yyz4gx48ak44la").is_err()); - assert!(parse_narinfo_str("000000").is_err()); - assert!(parse_narinfo_str("00bgd045z0d4icpbc2yyz4gx48ak44l🦊.narinfo").is_err()); - } -} |