Skip to content

Commit ef930ba

Browse files
committed
fix(redirect): fd3 redirection pattern 3>&1 >file now routes correctly
Use interpreter-level pending_fd_output buffer (not ExecResult) to carry fd3+ content from inner 1>&N redirects to compound-level fd-table routing. This avoids growing ExecResult (which would cause stack overflow in deep recursive execution). Changes: - Interpreter.pending_fd_output: HashMap<i32, String> for fd3+ output - apply_redirections fast path: 1>&N (N>=3) moves stdout to pending buffer - apply_redirections_fd_table: generalized to HashMap-based fd table supporting arbitrary fds, consumes pending_fd_output for routing Closes #1115
1 parent 7f642b0 commit ef930ba

File tree

2 files changed

+105
-35
lines changed

2 files changed

+105
-35
lines changed

crates/bashkit/src/interpreter/mod.rs

Lines changed: 96 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,10 @@ pub struct Interpreter {
485485
/// Persistent fd output table set by `exec N>/path` redirections.
486486
/// Maps fd number to its output target. Used by `>&N` redirections.
487487
exec_fd_table: HashMap<i32, FdTarget>,
488+
/// Temporary buffer for fd3+ output during compound body execution.
489+
/// When `1>&N` (N>=3) is encountered in a simple command, stdout moves here
490+
/// so that compound-level redirects (e.g. `3>&1 >file`) can route it correctly.
491+
pending_fd_output: HashMap<i32, String>,
488492
/// Cancellation token: when set to `true`, execution aborts at the next
489493
/// command boundary with `Error::Cancelled`.
490494
cancelled: Arc<AtomicBool>,
@@ -804,6 +808,7 @@ impl Interpreter {
804808
coproc_buffers: HashMap::new(),
805809
coproc_next_fd: 63,
806810
exec_fd_table: HashMap::new(),
811+
pending_fd_output: HashMap::new(),
807812
cancelled: Arc::new(AtomicBool::new(false)),
808813
deferred_proc_subs: Vec::new(),
809814
}
@@ -5928,6 +5933,20 @@ impl Interpreter {
59285933
result.stderr.push_str(&result.stdout);
59295934
result.stdout = String::new();
59305935
}
5936+
(src, dst) if dst >= 3 => {
5937+
// Move content to pending_fd_output for compound
5938+
// redirect routing (e.g. `echo msg 1>&3` inside
5939+
// `{ ... } 3>&1 >file`).
5940+
let data = if src == 2 {
5941+
std::mem::take(&mut result.stderr)
5942+
} else {
5943+
std::mem::take(&mut result.stdout)
5944+
};
5945+
self.pending_fd_output
5946+
.entry(dst)
5947+
.or_default()
5948+
.push_str(&data);
5949+
}
59315950
_ => {}
59325951
}
59335952
}
@@ -5950,9 +5969,13 @@ impl Interpreter {
59505969
mut result: ExecResult,
59515970
redirects: &[Redirect],
59525971
) -> Result<ExecResult> {
5953-
// Build fd table: fd1 = stdout pipe, fd2 = stderr pipe
5954-
let mut fd1 = FdTarget::Stdout;
5955-
let mut fd2 = FdTarget::Stderr;
5972+
// Build fd table with support for arbitrary fds (fd1, fd2, fd3+).
5973+
// Bash processes redirects left-to-right: `3>&1 >file` means
5974+
// fd3 captures fd1's *current* target (stdout) before fd1 is
5975+
// redirected to the file.
5976+
let mut fd_table: HashMap<i32, FdTarget> = HashMap::new();
5977+
fd_table.insert(1, FdTarget::Stdout);
5978+
fd_table.insert(2, FdTarget::Stderr);
59565979

59575980
for redirect in redirects {
59585981
match redirect.kind {
@@ -5977,10 +6000,7 @@ impl Interpreter {
59776000
} else {
59786001
FdTarget::WriteFile(path, target_path)
59796002
};
5980-
match redirect.fd {
5981-
Some(2) => fd2 = target,
5982-
_ => fd1 = target,
5983-
}
6003+
fd_table.insert(redirect.fd.unwrap_or(1), target);
59846004
}
59856005
RedirectKind::Append => {
59866006
let target_path = self.expand_word(&redirect.target).await?;
@@ -5990,10 +6010,7 @@ impl Interpreter {
59906010
} else {
59916011
FdTarget::AppendFile(path, target_path)
59926012
};
5993-
match redirect.fd {
5994-
Some(2) => fd2 = target,
5995-
_ => fd1 = target,
5996-
}
6013+
fd_table.insert(redirect.fd.unwrap_or(1), target);
59976014
}
59986015
RedirectKind::OutputBoth => {
59996016
let target_path = self.expand_word(&redirect.target).await?;
@@ -6003,27 +6020,22 @@ impl Interpreter {
60036020
} else {
60046021
FdTarget::WriteFile(path, target_path)
60056022
};
6006-
fd1 = target.clone();
6007-
fd2 = target;
6023+
fd_table.insert(1, target.clone());
6024+
fd_table.insert(2, target);
60086025
}
60096026
RedirectKind::DupOutput => {
60106027
let target = self.expand_word(&redirect.target).await?;
60116028
let target_fd: i32 = target.parse().unwrap_or(1);
60126029
let src_fd = redirect.fd.unwrap_or(1);
60136030

6014-
// Look up exec_fd_table for persistent fd targets
6015-
if let Some(exec_target) = self.exec_fd_table.get(&target_fd).cloned() {
6016-
match src_fd {
6017-
2 => fd2 = exec_target,
6018-
_ => fd1 = exec_target,
6019-
}
6020-
} else {
6021-
match (src_fd, target_fd) {
6022-
(2, 1) => fd2 = fd1.clone(),
6023-
(1, 2) => fd1 = fd2.clone(),
6024-
_ => {}
6025-
}
6026-
}
6031+
// Resolve: exec_fd_table first, then fd_table
6032+
let resolved = self
6033+
.exec_fd_table
6034+
.get(&target_fd)
6035+
.cloned()
6036+
.or_else(|| fd_table.get(&target_fd).cloned())
6037+
.unwrap_or(FdTarget::Stdout);
6038+
fd_table.insert(src_fd, resolved);
60276039
}
60286040
RedirectKind::Input
60296041
| RedirectKind::HereString
@@ -6033,21 +6045,23 @@ impl Interpreter {
60336045
}
60346046
}
60356047

6036-
// Route original stdout/stderr based on final fd table.
6037-
// Collect file writes to avoid double-borrow issues.
6048+
// Route content based on final fd table.
60386049
let orig_stdout = std::mem::take(&mut result.stdout);
60396050
let orig_stderr = std::mem::take(&mut result.stderr);
6051+
let pending = std::mem::take(&mut self.pending_fd_output);
60406052

6041-
// Determine what goes where
60426053
let mut new_stdout = String::new();
60436054
let mut new_stderr = String::new();
6044-
// file_path -> (content, is_append, display_path)
6045-
let mut file_writes: std::collections::HashMap<PathBuf, (String, bool, String)> =
6046-
std::collections::HashMap::new();
6047-
6048-
for (data, fd_target) in [(&orig_stdout, &fd1), (&orig_stderr, &fd2)].iter() {
6055+
let mut file_writes: HashMap<PathBuf, (String, bool, String)> = HashMap::new();
6056+
6057+
// Route helper
6058+
let route = |data: &str,
6059+
fd_target: &FdTarget,
6060+
file_writes: &mut HashMap<PathBuf, (String, bool, String)>,
6061+
new_stdout: &mut String,
6062+
new_stderr: &mut String| {
60496063
if data.is_empty() {
6050-
continue;
6064+
return;
60516065
}
60526066
match fd_target {
60536067
FdTarget::Stdout => new_stdout.push_str(data),
@@ -6068,6 +6082,36 @@ impl Interpreter {
60686082
.push_str(data);
60696083
}
60706084
}
6085+
};
6086+
6087+
// Route fd1 (stdout) and fd2 (stderr)
6088+
let fd1_target = fd_table.get(&1).cloned().unwrap_or(FdTarget::Stdout);
6089+
let fd2_target = fd_table.get(&2).cloned().unwrap_or(FdTarget::Stderr);
6090+
route(
6091+
&orig_stdout,
6092+
&fd1_target,
6093+
&mut file_writes,
6094+
&mut new_stdout,
6095+
&mut new_stderr,
6096+
);
6097+
route(
6098+
&orig_stderr,
6099+
&fd2_target,
6100+
&mut file_writes,
6101+
&mut new_stdout,
6102+
&mut new_stderr,
6103+
);
6104+
6105+
// Route pending fd3+ output from inner `1>&3` redirects
6106+
for (fd_num, data) in &pending {
6107+
let target = fd_table.get(fd_num).cloned().unwrap_or(FdTarget::Stdout);
6108+
route(
6109+
&data,
6110+
&target,
6111+
&mut file_writes,
6112+
&mut new_stdout,
6113+
&mut new_stderr,
6114+
);
60716115
}
60726116

60736117
// Write files
@@ -10898,4 +10942,21 @@ echo "count=$COUNT"
1089810942
let result = run_script("shopt -s failglob; ls ./*.html 2>/dev/null; echo exit:$?").await;
1089910943
assert_eq!(result.stderr, "", "failglob: stderr should be suppressed");
1090010944
}
10945+
10946+
#[tokio::test]
10947+
async fn test_fd3_redirect_pattern() {
10948+
// Issue #1115: { echo "progress" 1>&3; echo "file content"; } 3>&1 >file
10949+
// fd3 captures original stdout before >file redirects fd1
10950+
let result = run_script(
10951+
r#"{ echo "progress" 1>&3; echo "file content"; } 3>&1 > /tmp/test_fd.txt
10952+
cat /tmp/test_fd.txt"#,
10953+
)
10954+
.await;
10955+
let lines: Vec<&str> = result.stdout.lines().collect();
10956+
assert_eq!(
10957+
lines,
10958+
vec!["progress", "file content"],
10959+
"fd3 content should appear first on stdout, file content from cat second"
10960+
);
10961+
}
1090110962
}

crates/bashkit/tests/spec_cases/bash/exec-fd-redirect.test.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,12 @@ echo "closed ok"
3535
### expect
3636
closed ok
3737
### end
38+
39+
### fd3_redirect_pattern
40+
# { cmd 1>&3; cmd; } 3>&1 >file — fd3 captures original stdout (issue #1115)
41+
{ echo "progress" 1>&3; echo "file content"; } 3>&1 > /tmp/test_fd.txt
42+
cat /tmp/test_fd.txt
43+
### expect
44+
progress
45+
file content
46+
### end

0 commit comments

Comments
 (0)