fix: Resolve panic for long pathnames in npm tarballs (#32)

This fix addresses a panic during npm tarball creation caused by not
accounting for path length limits. Specifically, we already required the
total path length must be under 160 characters to be compatible with
Windows, but we need to limit it further to 155 to accommodate tarball
constraints. Additionally, the final path component is restricted to
under 100 characters, which we've reduced to 95 to permit extension
modifications (e.g., adding '.d.ts').

api/src/ids.rs

@@ -460,7 +460,7 @@ pub enum VersionValidateError {
 /// The path must not contain any double slashes, dot segments, or dot dot
 /// segments.
 ///
-/// The path must be less than 160 characters long, including the slash prefix.
+/// The path must be less than 155 characters long, including the slash prefix.
 ///
 /// The path must not contain any windows reserved characters, like CON, PRN,
 /// AUX, NUL, or COM1.
@@ -482,7 +482,10 @@ pub struct PackagePath {
 impl PackagePath {
   pub fn new(path: String) -> Result<Self, PackagePathValidationError> {
     let len = path.len();
-    if len > 160 {
+    // The total length of the path must be less than 160 characters to support
+    // windows. We reduce this further to 155 to work around tarball
+    // restrictions.
+    if len > 155 {
       return Err(PackagePathValidationError::TooLong(len));
     }
 
@@ -491,6 +494,7 @@ impl PackagePath {
     }
 
     let mut components = path.split('/').peekable();
+
     let Some("") = components.next() else {
       return Err(PackagePathValidationError::MissingPrefix);
     };
@@ -502,7 +506,11 @@ impl PackagePath {
       }
       valid_char(c)
     };
+
+    let mut last = None;
+
     while let Some(component) = components.next() {
+      last = Some(component);
       if component.is_empty() {
         if components.peek().is_none() {
           return Err(PackagePathValidationError::TrailingSlash);
@@ -539,6 +547,17 @@ impl PackagePath {
       }
     }
 
+    // Due to restrictions in how tarballs are built, we need the ensure that
+    // the last path component is less than 100 characters long. We further
+    // reduce this to 95, to allow for modifying the extension (for example, we
+    // add d.ts in places).
+    let last = last.unwrap();
+    if last.len() > 95 {
+      return Err(PackagePathValidationError::LastPathComponentTooLong(
+        last.len(),
+      ));
+    }
+
     let lower = has_upper.then(|| path.to_ascii_lowercase());
 
     Ok(Self { path, lower })
@@ -701,11 +720,14 @@ fn valid_char(c: char) -> Option<PackagePathValidationError> {
   }
 }
 
-#[derive(Debug, Clone, Error)]
+#[derive(Debug, Clone, Error, PartialEq)]
 pub enum PackagePathValidationError {
-  #[error("package path must be at most 160 characters long, but is {0} characters long")]
+  #[error("package path must be at most 155 characters long, but is {0} characters long")]
   TooLong(usize),
 
+  #[error("the last path component must be at most 95 characters long, but is {0} characters long")]
+  LastPathComponentTooLong(usize),
+
   #[error("package path must be prefixed with a slash")]
   MissingPrefix,
 
@@ -850,6 +872,38 @@ mod tests {
     assert!(ScopedPackageName::new("@scope/foo/bar".to_string()).is_err());
   }
 
+  #[test]
+  fn test_package_path_lengths() {
+    fn mock_package_path(
+      path_segments: &[usize],
+    ) -> Result<PackagePath, PackagePathValidationError> {
+      let mut path = String::new();
+      for s in path_segments.iter() {
+        let path_segment = "a".repeat(*s);
+        path.push('/');
+        path.push_str(&path_segment);
+      }
+      PackagePath::new(path)
+    }
+
+    mock_package_path(&[58, 95]).unwrap();
+    assert_eq!(
+      mock_package_path(&[59, 95]).unwrap_err(),
+      PackagePathValidationError::TooLong(156)
+    );
+    assert_eq!(
+      mock_package_path(&[57, 96]).unwrap_err(),
+      PackagePathValidationError::LastPathComponentTooLong(96)
+    );
+    mock_package_path(&[56, 95, 1]).unwrap();
+    mock_package_path(&[30, 94]).unwrap();
+    assert_eq!(
+      mock_package_path(&[1, 96]).unwrap_err(),
+      PackagePathValidationError::LastPathComponentTooLong(96)
+    );
+    mock_package_path(&[96, 57]).unwrap();
+  }
+
   #[test]
   fn test_package_path() {
     // Test valid package paths

api/src/npm/tarball.rs

@@ -9,6 +9,7 @@ use deno_semver::package::PackageReqReference;
 use indexmap::IndexMap;
 use sha2::Digest;
 use tar::Header;
+use tracing::error;
 use tracing::info;
 use url::Url;
 
@@ -174,8 +175,18 @@ pub fn create_npm_tarball<'a>(
   let mtime = now.duration_since(std::time::UNIX_EPOCH).unwrap().as_secs();
 
   for (path, content) in transpiled_files.iter() {
-    let mut header = Header::new_gnu();
-    header.set_path(format!("./package{path}")).unwrap();
+    let mut header = Header::new_ustar();
+    header.set_path(format!("./package{path}")).map_err(|e| {
+      // Ideally we never hit this error, because package length should have been checked
+      // when creating PackagePath.
+      // TODO(ry) This is not the ideal way to pass PublishErrors up the stack
+      // because it will become anyhow::Error and wrapped in an NpmTarballError.
+      error!("bad npm tarball path {} {}", path, e);
+      crate::tarball::PublishError::InvalidPath {
+        path: path.to_string(),
+        error: crate::ids::PackagePathValidationError::TooLong(path.len()),
+      }
+    })?;
     header.set_size(content.len() as u64);
     header.set_mode(0o777);
     header.set_mtime(mtime);

api/src/publish.rs

@@ -1013,6 +1013,17 @@ pub mod tests {
     assert_eq!(error.code, "missingConfigFile");
   }
 
+  #[tokio::test]
+  async fn no_long_paths() {
+    let t = TestSetup::new().await;
+    let bytes = create_mock_tarball("no_long_paths");
+    let task = process_tarball_setup(&t, bytes).await;
+    assert_eq!(task.status, PublishingTaskStatus::Failure, "{task:#?}");
+    let error = task.error.unwrap();
+    assert_eq!(error.code, "invalidPath");
+    assert!(error.message.contains("a_very_long_filename_created_specifically_to_test_the_limitations_of_the_set_path_method_in_the_rust_tar_crate_exceeding_one_hundred_bytes"));
+  }
+
   #[tokio::test]
   async fn global_type_augmentation1() {
     let t = TestSetup::new().await;

api/testdata/tarballs/no_long_paths/a_very_long_filename_created_specifically_to_test_the_limitations_of_the_set_path_method_in_the_rust_tar_crate_exceeding_one_hundred_bytes.ts

@@ -0,0 +1,10 @@
+/**
+ * This is a test module.
+ * 
+ * @module
+ */
+
+/**
+ * This is a test constant.
+ */
+export const hello = "Hello, world!";

api/testdata/tarballs/no_long_paths/jsr.json

@@ -0,0 +1,5 @@
+{
+  "name": "@scope/foo",
+  "version": "1.2.3",
+  "exports": "./a_very_long_filename_created_specifically_to_test_the_limitations_of_the_set_path_method_in_the_rust_tar_crate_exceeding_one_hundred_bytes.ts"
+}

frontend/docs/troubleshooting.md

@@ -55,7 +55,8 @@ package, or [excluding it](/docs/publishing-packages#ignoring-files) in your
 
 Path rules are as follows:
 
-- Less than 160 chars
+- Less than 155 chars
+- The last component (filename) of the path must be less than 95 chars
 - Path must not end in a slash
 - Must not contain a double slash (`//`)
 - Must not contain a `.` or `..` path segment