about summary refs log tree commit diff
path: root/tvix/nar-bridge
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2024-08-21T08·06+0300
committerclbot <clbot@tvl.fyi>2024-08-21T09·40+0000
commite03ea11badbf40971d6bf87eede33fe3b046c98b (patch)
treed520dedadeb8193b2da323adb2623a269217951c /tvix/nar-bridge
parent2357079891819c679821ea58a7715de7be431aaa (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')
-rw-r--r--tvix/nar-bridge/src/nar.rs62
-rw-r--r--tvix/nar-bridge/src/narinfo.rs54
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());
-    }
-}