Skip to content

Commit 8dc37b3

Browse files
authored
fix(vfs): handle ./ prefix in path resolution (#1142)
## Summary - Normalize `.` and `..` path components in `Interpreter::resolve_path`, glob directory resolution, and `PosixFs` methods - Ensures `./filename` resolves identically to `filename` relative to cwd across all VFS operations (ls, cat, redirections, test builtin, globs) ## Changes - **`Interpreter::resolve_path`**: Now calls `normalize_path` on the joined result, fixing redirections (`> ./file`), script execution (`./script.sh`), and the `test` builtin (`[ -f ./file ]`) - **`Interpreter` test-command lambda**: Same normalization for inline path resolution in `[`/`test` - **`glob::expand_glob`**: Normalizes the directory path when pattern has a relative prefix like `./` - **`PosixFs`**: All `FileSystem` trait methods now normalize input paths before delegating to the backend, ensuring custom `FsBackend` implementations work correctly with `.` and `..` components ## Test plan - [x] `test_dot_slash_prefix_ls` — `ls ./file` works - [x] `test_dot_slash_prefix_glob` — `echo ./*.html` works - [x] `test_dot_slash_prefix_cat` — `cat ./file` works - [x] `test_dot_slash_prefix_redirect` — `echo x > ./file` works - [x] `test_dot_slash_prefix_test_builtin` — `test -f ./file` works - [x] `test_posix_normalize_dot_slash_prefix` — PosixFs handles `./` paths - [x] `test_posix_normalize_preserves_semantics` — parent-exists checks still work - [x] All 2036 existing tests pass Closes #1114
1 parent 45e3583 commit 8dc37b3

File tree

4 files changed

+219
-27
lines changed

4 files changed

+219
-27
lines changed

crates/bashkit/src/fs/posix.rs

Lines changed: 88 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ use std::sync::Arc;
5353

5454
use super::backend::FsBackend;
5555
use super::limits::{FsLimits, FsUsage};
56+
use super::normalize_path;
5657
use super::traits::{DirEntry, FileSystem, FileSystemExt, Metadata, fs_errors};
5758
use crate::error::Result;
5859

@@ -100,6 +101,11 @@ impl<B: FsBackend> PosixFs<B> {
100101
&self.backend
101102
}
102103

104+
/// Normalize a path for consistent lookups.
105+
fn normalize(path: &Path) -> PathBuf {
106+
normalize_path(path)
107+
}
108+
103109
/// Check if parent directory exists.
104110
async fn check_parent_exists(&self, path: &Path) -> Result<()> {
105111
if let Some(parent) = path.parent()
@@ -116,43 +122,47 @@ impl<B: FsBackend> PosixFs<B> {
116122
#[async_trait]
117123
impl<B: FsBackend + 'static> FileSystem for PosixFs<B> {
118124
async fn read_file(&self, path: &Path) -> Result<Vec<u8>> {
125+
let path = Self::normalize(path);
119126
// Check if it's a directory
120-
if let Ok(meta) = self.backend.stat(path).await
127+
if let Ok(meta) = self.backend.stat(&path).await
121128
&& meta.file_type.is_dir()
122129
{
123130
return Err(fs_errors::is_a_directory());
124131
}
125-
self.backend.read(path).await
132+
self.backend.read(&path).await
126133
}
127134

128135
async fn write_file(&self, path: &Path, content: &[u8]) -> Result<()> {
136+
let path = Self::normalize(path);
129137
// Check parent exists
130-
self.check_parent_exists(path).await?;
138+
self.check_parent_exists(&path).await?;
131139

132140
// Check if path is a directory
133-
if let Ok(meta) = self.backend.stat(path).await
141+
if let Ok(meta) = self.backend.stat(&path).await
134142
&& meta.file_type.is_dir()
135143
{
136144
return Err(fs_errors::is_a_directory());
137145
}
138146

139-
self.backend.write(path, content).await
147+
self.backend.write(&path, content).await
140148
}
141149

142150
async fn append_file(&self, path: &Path, content: &[u8]) -> Result<()> {
151+
let path = Self::normalize(path);
143152
// Check if path is a directory
144-
if let Ok(meta) = self.backend.stat(path).await
153+
if let Ok(meta) = self.backend.stat(&path).await
145154
&& meta.file_type.is_dir()
146155
{
147156
return Err(fs_errors::is_a_directory());
148157
}
149158

150-
self.backend.append(path, content).await
159+
self.backend.append(&path, content).await
151160
}
152161

153162
async fn mkdir(&self, path: &Path, recursive: bool) -> Result<()> {
163+
let path = Self::normalize(path);
154164
// Check if something already exists at this path
155-
if let Ok(meta) = self.backend.stat(path).await {
165+
if let Ok(meta) = self.backend.stat(&path).await {
156166
if meta.file_type.is_dir() {
157167
// Directory exists
158168
if recursive {
@@ -181,58 +191,70 @@ impl<B: FsBackend + 'static> FileSystem for PosixFs<B> {
181191
}
182192
} else {
183193
// Non-recursive: parent must exist
184-
self.check_parent_exists(path).await?;
194+
self.check_parent_exists(&path).await?;
185195
}
186196

187-
self.backend.mkdir(path, recursive).await
197+
self.backend.mkdir(&path, recursive).await
188198
}
189199

190200
async fn remove(&self, path: &Path, recursive: bool) -> Result<()> {
191-
self.backend.remove(path, recursive).await
201+
let path = Self::normalize(path);
202+
self.backend.remove(&path, recursive).await
192203
}
193204

194205
async fn stat(&self, path: &Path) -> Result<Metadata> {
195-
self.backend.stat(path).await
206+
let path = Self::normalize(path);
207+
self.backend.stat(&path).await
196208
}
197209

198210
async fn read_dir(&self, path: &Path) -> Result<Vec<DirEntry>> {
211+
let path = Self::normalize(path);
199212
// Check if it's actually a directory
200-
if let Ok(meta) = self.backend.stat(path).await
213+
if let Ok(meta) = self.backend.stat(&path).await
201214
&& !meta.file_type.is_dir()
202215
{
203216
return Err(fs_errors::not_a_directory());
204217
}
205-
self.backend.read_dir(path).await
218+
self.backend.read_dir(&path).await
206219
}
207220

208221
async fn exists(&self, path: &Path) -> Result<bool> {
209-
self.backend.exists(path).await
222+
let path = Self::normalize(path);
223+
self.backend.exists(&path).await
210224
}
211225

212226
async fn rename(&self, from: &Path, to: &Path) -> Result<()> {
213-
self.backend.rename(from, to).await
227+
let from = Self::normalize(from);
228+
let to = Self::normalize(to);
229+
self.backend.rename(&from, &to).await
214230
}
215231

216232
async fn copy(&self, from: &Path, to: &Path) -> Result<()> {
233+
let from = Self::normalize(from);
234+
let to = Self::normalize(to);
217235
// Check source is not a directory
218-
if let Ok(meta) = self.backend.stat(from).await
236+
if let Ok(meta) = self.backend.stat(&from).await
219237
&& meta.file_type.is_dir()
220238
{
221239
return Err(IoError::other("cannot copy directory").into());
222240
}
223-
self.backend.copy(from, to).await
241+
self.backend.copy(&from, &to).await
224242
}
225243

226244
async fn symlink(&self, target: &Path, link: &Path) -> Result<()> {
227-
self.backend.symlink(target, link).await
245+
let target = Self::normalize(target);
246+
let link = Self::normalize(link);
247+
self.backend.symlink(&target, &link).await
228248
}
229249

230250
async fn read_link(&self, path: &Path) -> Result<PathBuf> {
231-
self.backend.read_link(path).await
251+
let path = Self::normalize(path);
252+
self.backend.read_link(&path).await
232253
}
233254

234255
async fn chmod(&self, path: &Path, mode: u32) -> Result<()> {
235-
self.backend.chmod(path, mode).await
256+
let path = Self::normalize(path);
257+
self.backend.chmod(&path, mode).await
236258
}
237259
}
238260

@@ -295,4 +317,49 @@ mod tests {
295317
let result = fs.mkdir(Path::new("/tmp/testfile"), false).await;
296318
assert!(result.is_err());
297319
}
320+
321+
#[tokio::test]
322+
async fn test_posix_normalize_dot_slash_prefix() {
323+
// Issue #1114: paths with ./ prefix should resolve correctly
324+
let fs = InMemoryFs::new();
325+
326+
// Create /tmp/dir and a file
327+
fs.mkdir(Path::new("/tmp/dir"), true).await.unwrap();
328+
fs.write_file(Path::new("/tmp/dir/file.txt"), b"content")
329+
.await
330+
.unwrap();
331+
332+
// Access via ./ style path (as if cwd.join("./file.txt"))
333+
let dot_path = Path::new("/tmp/dir/./file.txt");
334+
assert!(
335+
fs.exists(dot_path).await.unwrap(),
336+
"exists with ./ should work"
337+
);
338+
339+
let content = fs.read_file(dot_path).await.unwrap();
340+
assert_eq!(content, b"content");
341+
342+
// stat with ./ prefix
343+
let meta = fs.stat(dot_path).await;
344+
assert!(meta.is_ok(), "stat with ./ should work");
345+
346+
// write via ./ prefix
347+
fs.write_file(Path::new("/tmp/dir/./new.txt"), b"new")
348+
.await
349+
.unwrap();
350+
let content = fs.read_file(Path::new("/tmp/dir/new.txt")).await.unwrap();
351+
assert_eq!(content, b"new");
352+
}
353+
354+
#[tokio::test]
355+
async fn test_posix_normalize_preserves_semantics() {
356+
// Verify normalization doesn't break parent-exists checks
357+
let fs = InMemoryFs::new();
358+
359+
// /tmp exists, /tmp/nonexistent does not
360+
let result = fs
361+
.write_file(Path::new("/tmp/nonexistent/./file.txt"), b"content")
362+
.await;
363+
assert!(result.is_err(), "should fail when parent doesn't exist");
364+
}
298365
}

crates/bashkit/src/interpreter/glob.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ impl Interpreter {
616616
if p.as_os_str().is_empty() {
617617
(self.cwd.clone(), name)
618618
} else {
619-
(self.cwd.join(p), name)
619+
(crate::fs::normalize_path(&self.cwd.join(p)), name)
620620
}
621621
} else {
622622
(self.cwd.clone(), name)

crates/bashkit/src/interpreter/mod.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2033,11 +2033,12 @@ impl Interpreter {
20332033
// Unary operators
20342034
let resolve = |p: &str| -> std::path::PathBuf {
20352035
let path = std::path::Path::new(p);
2036-
if path.is_absolute() {
2036+
let joined = if path.is_absolute() {
20372037
path.to_path_buf()
20382038
} else {
20392039
self.cwd.join(path)
2040-
}
2040+
};
2041+
crate::fs::normalize_path(&joined)
20412042
};
20422043
match args[0].as_str() {
20432044
"-z" => args[1].is_empty(),
@@ -6185,14 +6186,15 @@ impl Interpreter {
61856186
Ok(result)
61866187
}
61876188

6188-
/// Resolve a path relative to cwd
6189+
/// Resolve a path relative to cwd, normalizing `.` and `..` components.
61896190
fn resolve_path(&self, path: &str) -> PathBuf {
61906191
let p = Path::new(path);
6191-
if p.is_absolute() {
6192+
let joined = if p.is_absolute() {
61926193
p.to_path_buf()
61936194
} else {
61946195
self.cwd.join(p)
6195-
}
6196+
};
6197+
crate::fs::normalize_path(&joined)
61966198
}
61976199

61986200
/// Expand an array access expression (`${arr[index]}`).

crates/bashkit/src/lib.rs

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5439,4 +5439,127 @@ echo missing fi"#,
54395439
*stderr_chunks
54405440
);
54415441
}
5442+
5443+
#[tokio::test]
5444+
async fn test_dot_slash_prefix_ls() {
5445+
// Issue #1114: ./filename should resolve identically to filename
5446+
let mut bash = Bash::new();
5447+
bash.exec("mkdir -p /tmp/blogtest && cd /tmp/blogtest && echo hello > tag_hello.html")
5448+
.await
5449+
.unwrap();
5450+
5451+
// ls without ./ prefix should work
5452+
let result = bash
5453+
.exec("cd /tmp/blogtest && ls tag_hello.html")
5454+
.await
5455+
.unwrap();
5456+
assert_eq!(
5457+
result.exit_code, 0,
5458+
"ls tag_hello.html should succeed: {}",
5459+
result.stderr
5460+
);
5461+
assert!(result.stdout.contains("tag_hello.html"));
5462+
5463+
// ls with ./ prefix should also work
5464+
let result = bash
5465+
.exec("cd /tmp/blogtest && ls ./tag_hello.html")
5466+
.await
5467+
.unwrap();
5468+
assert_eq!(
5469+
result.exit_code, 0,
5470+
"ls ./tag_hello.html should succeed: {}",
5471+
result.stderr
5472+
);
5473+
assert!(result.stdout.contains("tag_hello.html"));
5474+
}
5475+
5476+
#[tokio::test]
5477+
async fn test_dot_slash_prefix_glob() {
5478+
// Issue #1114: ./*.html should resolve identically to *.html
5479+
let mut bash = Bash::new();
5480+
bash.exec("mkdir -p /tmp/globtest && cd /tmp/globtest && echo hello > tag_hello.html")
5481+
.await
5482+
.unwrap();
5483+
5484+
// glob without ./ prefix
5485+
let result = bash.exec("cd /tmp/globtest && echo *.html").await.unwrap();
5486+
assert_eq!(
5487+
result.exit_code, 0,
5488+
"echo *.html should succeed: {}",
5489+
result.stderr
5490+
);
5491+
assert!(result.stdout.contains("tag_hello.html"));
5492+
5493+
// glob with ./ prefix
5494+
let result = bash
5495+
.exec("cd /tmp/globtest && echo ./*.html")
5496+
.await
5497+
.unwrap();
5498+
assert_eq!(
5499+
result.exit_code, 0,
5500+
"echo ./*.html should succeed: {}",
5501+
result.stderr
5502+
);
5503+
assert!(result.stdout.contains("tag_hello.html"));
5504+
}
5505+
5506+
#[tokio::test]
5507+
async fn test_dot_slash_prefix_cat() {
5508+
// Issue #1114: cat ./filename should work
5509+
let mut bash = Bash::new();
5510+
bash.exec("mkdir -p /tmp/cattest && cd /tmp/cattest && echo content123 > myfile.txt")
5511+
.await
5512+
.unwrap();
5513+
5514+
let result = bash
5515+
.exec("cd /tmp/cattest && cat ./myfile.txt")
5516+
.await
5517+
.unwrap();
5518+
assert_eq!(
5519+
result.exit_code, 0,
5520+
"cat ./myfile.txt should succeed: {}",
5521+
result.stderr
5522+
);
5523+
assert!(result.stdout.contains("content123"));
5524+
}
5525+
5526+
#[tokio::test]
5527+
async fn test_dot_slash_prefix_redirect() {
5528+
// Issue #1114: redirecting to ./filename should work
5529+
let mut bash = Bash::new();
5530+
bash.exec("mkdir -p /tmp/redirtest && cd /tmp/redirtest")
5531+
.await
5532+
.unwrap();
5533+
5534+
let result = bash
5535+
.exec("cd /tmp/redirtest && echo hello > ./output.txt && cat ./output.txt")
5536+
.await
5537+
.unwrap();
5538+
assert_eq!(
5539+
result.exit_code, 0,
5540+
"redirect to ./output.txt should succeed: {}",
5541+
result.stderr
5542+
);
5543+
assert!(result.stdout.contains("hello"));
5544+
}
5545+
5546+
#[tokio::test]
5547+
async fn test_dot_slash_prefix_test_builtin() {
5548+
// Issue #1114: test -f ./filename should work
5549+
let mut bash = Bash::new();
5550+
bash.exec("mkdir -p /tmp/testbuiltin && cd /tmp/testbuiltin && echo x > myfile.txt")
5551+
.await
5552+
.unwrap();
5553+
5554+
let result = bash
5555+
.exec("cd /tmp/testbuiltin && test -f ./myfile.txt && echo yes")
5556+
.await
5557+
.unwrap();
5558+
assert_eq!(
5559+
result.exit_code, 0,
5560+
"test -f ./myfile.txt should succeed: {}",
5561+
result.stderr
5562+
);
5563+
assert!(result.stdout.contains("yes"));
5564+
}
54425565
}

0 commit comments

Comments
 (0)