summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--Cargo.lock1
-rw-r--r--Cargo.toml1
-rw-r--r--cli/Cargo.toml2
-rw-r--r--ext/fs/Cargo.toml1
-rw-r--r--ext/fs/ops.rs91
-rw-r--r--tests/unit/make_temp_test.ts22
6 files changed, 92 insertions, 26 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 01f2ce4a7..20613ec66 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1402,6 +1402,7 @@ name = "deno_fs"
version = "0.48.0"
dependencies = [
"async-trait",
+ "base32",
"deno_core",
"deno_io",
"filetime",
diff --git a/Cargo.toml b/Cargo.toml
index 6bcf3a222..5ea993f05 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -85,6 +85,7 @@ deno_webstorage = { version = "0.133.0", path = "./ext/webstorage" }
aes = "=0.8.3"
anyhow = "1.0.57"
async-trait = "0.1.73"
+base32 = "=0.4.0"
base64 = "0.21.4"
bencher = "0.1"
brotli = "3.3.4"
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index a8b7a0b25..67795b3c2 100644
--- a/cli/Cargo.toml
+++ b/cli/Cargo.toml
@@ -80,7 +80,7 @@ eszip = "=0.64.0"
napi_sym.workspace = true
async-trait.workspace = true
-base32 = "=0.4.0"
+base32.workspace = true
base64.workspace = true
bincode = "=1.3.3"
bytes.workspace = true
diff --git a/ext/fs/Cargo.toml b/ext/fs/Cargo.toml
index 81dab267f..8bdc5cf38 100644
--- a/ext/fs/Cargo.toml
+++ b/ext/fs/Cargo.toml
@@ -18,6 +18,7 @@ sync_fs = []
[dependencies]
async-trait.workspace = true
+base32.workspace = true
deno_core.workspace = true
deno_io.workspace = true
filetime.workspace = true
diff --git a/ext/fs/ops.rs b/ext/fs/ops.rs
index bc9f75a3c..d688f619e 100644
--- a/ext/fs/ops.rs
+++ b/ext/fs/ops.rs
@@ -8,6 +8,7 @@ use std::path::Path;
use std::path::PathBuf;
use std::rc::Rc;
+use deno_core::anyhow::bail;
use deno_core::error::custom_error;
use deno_core::error::type_error;
use deno_core::error::AnyError;
@@ -880,7 +881,8 @@ pub fn op_fs_make_temp_dir_sync<P>(
where
P: FsPermissions + 'static,
{
- let (dir, fs) = make_temp_check_sync::<P>(state, dir)?;
+ let (dir, fs) =
+ make_temp_check_sync::<P>(state, dir, "Deno.makeTempDirSync()")?;
let mut rng = thread_rng();
@@ -914,7 +916,7 @@ pub async fn op_fs_make_temp_dir_async<P>(
where
P: FsPermissions + 'static,
{
- let (dir, fs) = make_temp_check_async::<P>(state, dir)?;
+ let (dir, fs) = make_temp_check_async::<P>(state, dir, "Deno.makeTempDir()")?;
let mut rng = thread_rng();
@@ -948,7 +950,8 @@ pub fn op_fs_make_temp_file_sync<P>(
where
P: FsPermissions + 'static,
{
- let (dir, fs) = make_temp_check_sync::<P>(state, dir)?;
+ let (dir, fs) =
+ make_temp_check_sync::<P>(state, dir, "Deno.makeTempFileSync()")?;
let open_opts = OpenOptions {
write: true,
@@ -989,7 +992,8 @@ pub async fn op_fs_make_temp_file_async<P>(
where
P: FsPermissions + 'static,
{
- let (dir, fs) = make_temp_check_async::<P>(state, dir)?;
+ let (dir, fs) =
+ make_temp_check_async::<P>(state, dir, "Deno.makeTempFile()")?;
let open_opts = OpenOptions {
write: true,
@@ -1021,6 +1025,7 @@ where
fn make_temp_check_sync<P>(
state: &mut OpState,
dir: Option<String>,
+ api_name: &str,
) -> Result<(PathBuf, FileSystemRc), AnyError>
where
P: FsPermissions + 'static,
@@ -1029,18 +1034,14 @@ where
let dir = match dir {
Some(dir) => {
let dir = PathBuf::from(dir);
- state
- .borrow_mut::<P>()
- .check_write(&dir, "Deno.makeTempFile()")?;
+ state.borrow_mut::<P>().check_write(&dir, api_name)?;
dir
}
None => {
let dir = fs.tmp_dir().context("tmpdir")?;
- state.borrow_mut::<P>().check_write_blind(
- &dir,
- "TMP",
- "Deno.makeTempFile()",
- )?;
+ state
+ .borrow_mut::<P>()
+ .check_write_blind(&dir, "TMP", api_name)?;
dir
}
};
@@ -1050,6 +1051,7 @@ where
fn make_temp_check_async<P>(
state: Rc<RefCell<OpState>>,
dir: Option<String>,
+ api_name: &str,
) -> Result<(PathBuf, FileSystemRc), AnyError>
where
P: FsPermissions + 'static,
@@ -1059,24 +1061,57 @@ where
let dir = match dir {
Some(dir) => {
let dir = PathBuf::from(dir);
- state
- .borrow_mut::<P>()
- .check_write(&dir, "Deno.makeTempFile()")?;
+ state.borrow_mut::<P>().check_write(&dir, api_name)?;
dir
}
None => {
let dir = fs.tmp_dir().context("tmpdir")?;
- state.borrow_mut::<P>().check_write_blind(
- &dir,
- "TMP",
- "Deno.makeTempFile()",
- )?;
+ state
+ .borrow_mut::<P>()
+ .check_write_blind(&dir, "TMP", api_name)?;
dir
}
};
Ok((dir, fs))
}
+/// Identify illegal filename characters before attempting to use them in a filesystem
+/// operation. We're a bit stricter with tempfile and tempdir names than with regular
+/// files.
+fn validate_temporary_filename_component(
+ component: &str,
+ #[allow(unused_variables)] suffix: bool,
+) -> Result<(), AnyError> {
+ // Ban ASCII and Unicode control characters: these will often fail
+ if let Some(c) = component.matches(|c: char| c.is_control()).next() {
+ bail!("Invalid control character in prefix or suffix: {:?}", c);
+ }
+ // Windows has the most restrictive filenames. As temp files aren't normal files, we just
+ // use this set of banned characters for all platforms because wildcard-like files can also
+ // be problematic in unix-like shells.
+
+ // The list of banned characters in Windows:
+ // https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions
+
+ // You might ask why <, >, and " are included in the Windows list? It's because they are
+ // special wildcard implemented in the filesystem driver!
+ // https://learn.microsoft.com/en-ca/archive/blogs/jeremykuhne/wildcards-in-windows
+ if let Some(c) = component
+ .matches(|c: char| "<>:\"/\\|?*".contains(c))
+ .next()
+ {
+ bail!("Invalid character in prefix or suffix: {:?}", c);
+ }
+
+ // This check is only for Windows
+ #[cfg(windows)]
+ if suffix && component.ends_with(|c: char| ". ".contains(c)) {
+ bail!("Invalid trailing character in suffix");
+ }
+
+ Ok(())
+}
+
fn tmp_name(
rng: &mut ThreadRng,
dir: &Path,
@@ -1084,12 +1119,18 @@ fn tmp_name(
suffix: Option<&str>,
) -> Result<PathBuf, AnyError> {
let prefix = prefix.unwrap_or("");
+ validate_temporary_filename_component(prefix, false)?;
let suffix = suffix.unwrap_or("");
-
- let mut path = dir.join("_");
-
- let unique = rng.gen::<u32>();
- path.set_file_name(format!("{prefix}{unique:08x}{suffix}"));
+ validate_temporary_filename_component(suffix, true)?;
+
+ // If we use a 32-bit number, we only need ~70k temp files before we have a 50%
+ // chance of collision. By bumping this up to 64-bits, we require ~5 billion
+ // before hitting a 50% chance. We also base32-encode this value so the entire
+ // thing is 1) case insensitive and 2) slightly shorter than the equivalent hex
+ // value.
+ let unique = rng.gen::<u64>();
+ base32::encode(base32::Alphabet::Crockford, &unique.to_le_bytes());
+ let path = dir.join(format!("{prefix}{unique:08x}{suffix}"));
Ok(path)
}
diff --git a/tests/unit/make_temp_test.ts b/tests/unit/make_temp_test.ts
index cbbae8dfe..2c771177b 100644
--- a/tests/unit/make_temp_test.ts
+++ b/tests/unit/make_temp_test.ts
@@ -155,3 +155,25 @@ Deno.test(
}
},
);
+
+Deno.test(
+ { permissions: { read: true, write: true } },
+ function makeTempInvalidCharacter() {
+ // Various control and ASCII characters that are disallowed on all platforms
+ for (const invalid of ["\0", "*", "\x9f"]) {
+ assertThrows(() => Deno.makeTempFileSync({ prefix: invalid }));
+ assertThrows(() => Deno.makeTempDirSync({ prefix: invalid }));
+ assertThrows(() => Deno.makeTempFileSync({ suffix: invalid }));
+ assertThrows(() => Deno.makeTempDirSync({ suffix: invalid }));
+ }
+
+ // On Windows, files can't end with space or period
+ if (Deno.build.os === "windows") {
+ assertThrows(() => Deno.makeTempFileSync({ suffix: "." }));
+ assertThrows(() => Deno.makeTempFileSync({ suffix: " " }));
+ } else {
+ Deno.makeTempFileSync({ suffix: "." });
+ Deno.makeTempFileSync({ suffix: " " });
+ }
+ },
+);