Skip to content

Commit 215b591

Browse files
committed
fix(redirect): fd3 redirection pattern 3>&1 >file now routes correctly
Use interpreter-level pending_fd_output buffer to carry fd3+ content from inner 1>&N redirects to compound-level fd-table routing. Changes: - Interpreter.pending_fd_output: buffer for fd3+ output - apply_redirections fast path: 1>&N (N>=3) moves stdout to pending buffer - apply_redirections_fd_table: supports fd3+ via extra_fd_targets + pending - route_fd_table_content: extracted non-async helper to avoid state machine bloat Closes #1115
1 parent 7f642b0 commit 215b591

2 files changed

Lines changed: 157 additions & 38 deletions

File tree

crates/bashkit/src/interpreter/mod.rs

Lines changed: 148 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,13 @@ 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+
/// Populated by `1>&N` (N>=3) in apply_redirections, consumed by
490+
/// apply_redirections_fd_table for compound redirect routing.
491+
pending_fd_output: HashMap<i32, String>,
492+
/// Fd3+ targets from compound redirect processing (e.g. `3>&1` maps fd3→Stdout).
493+
/// Populated during apply_redirections_fd_table redirect loop, consumed during routing.
494+
pending_fd_targets: Vec<(i32, FdTarget)>,
488495
/// Cancellation token: when set to `true`, execution aborts at the next
489496
/// command boundary with `Error::Cancelled`.
490497
cancelled: Arc<AtomicBool>,
@@ -804,6 +811,8 @@ impl Interpreter {
804811
coproc_buffers: HashMap::new(),
805812
coproc_next_fd: 63,
806813
exec_fd_table: HashMap::new(),
814+
pending_fd_output: HashMap::new(),
815+
pending_fd_targets: Vec::new(),
807816
cancelled: Arc::new(AtomicBool::new(false)),
808817
deferred_proc_subs: Vec::new(),
809818
}
@@ -2889,6 +2898,87 @@ enum FdTarget {
28892898
DevNull,
28902899
}
28912900

2901+
/// Route fd1/fd2/fd3+ content to their targets. Extracted from the async
2902+
/// `apply_redirections_fd_table` to keep these locals out of the async state machine.
2903+
#[inline(never)]
2904+
fn route_fd_table_content(
2905+
orig_stdout: &str,
2906+
orig_stderr: &str,
2907+
fd1: &FdTarget,
2908+
fd2: &FdTarget,
2909+
extra_fd_targets: &[(i32, FdTarget)],
2910+
pending: &HashMap<i32, String>,
2911+
) -> (
2912+
String,
2913+
String,
2914+
std::collections::HashMap<PathBuf, (String, bool, String)>,
2915+
) {
2916+
let mut new_stdout = String::new();
2917+
let mut new_stderr = String::new();
2918+
let mut file_writes: std::collections::HashMap<PathBuf, (String, bool, String)> =
2919+
std::collections::HashMap::new();
2920+
2921+
let route = |data: &str,
2922+
target: &FdTarget,
2923+
fw: &mut std::collections::HashMap<PathBuf, (String, bool, String)>,
2924+
out: &mut String,
2925+
err: &mut String| {
2926+
if data.is_empty() {
2927+
return;
2928+
}
2929+
match target {
2930+
FdTarget::Stdout => out.push_str(data),
2931+
FdTarget::Stderr => err.push_str(data),
2932+
FdTarget::DevNull => {}
2933+
FdTarget::WriteFile(p, d) => {
2934+
fw.entry(p.clone())
2935+
.or_insert_with(|| (String::new(), false, d.clone()))
2936+
.0
2937+
.push_str(data);
2938+
}
2939+
FdTarget::AppendFile(p, d) => {
2940+
fw.entry(p.clone())
2941+
.or_insert_with(|| (String::new(), true, d.clone()))
2942+
.0
2943+
.push_str(data);
2944+
}
2945+
}
2946+
};
2947+
2948+
route(
2949+
orig_stdout,
2950+
fd1,
2951+
&mut file_writes,
2952+
&mut new_stdout,
2953+
&mut new_stderr,
2954+
);
2955+
route(
2956+
orig_stderr,
2957+
fd2,
2958+
&mut file_writes,
2959+
&mut new_stdout,
2960+
&mut new_stderr,
2961+
);
2962+
2963+
// Route pending fd3+ output
2964+
for (fd_num, data) in pending {
2965+
let target = extra_fd_targets
2966+
.iter()
2967+
.find(|(n, _)| n == fd_num)
2968+
.map(|(_, t)| t)
2969+
.unwrap_or(&FdTarget::Stdout);
2970+
route(
2971+
data,
2972+
target,
2973+
&mut file_writes,
2974+
&mut new_stdout,
2975+
&mut new_stderr,
2976+
);
2977+
}
2978+
2979+
(new_stdout, new_stderr, file_writes)
2980+
}
2981+
28922982
impl Interpreter {
28932983
/// Execute a sequence of commands (with errexit checking)
28942984
async fn execute_command_sequence(&mut self, commands: &[Command]) -> Result<ExecResult> {
@@ -5928,6 +6018,20 @@ impl Interpreter {
59286018
result.stderr.push_str(&result.stdout);
59296019
result.stdout = String::new();
59306020
}
6021+
(src, dst) if dst >= 3 => {
6022+
// Move content to pending_fd_output for compound
6023+
// redirect routing (e.g. `echo msg 1>&3` inside
6024+
// `{ ... } 3>&1 >file`).
6025+
let data = if src == 2 {
6026+
std::mem::take(&mut result.stderr)
6027+
} else {
6028+
std::mem::take(&mut result.stdout)
6029+
};
6030+
self.pending_fd_output
6031+
.entry(dst)
6032+
.or_default()
6033+
.push_str(&data);
6034+
}
59316035
_ => {}
59326036
}
59336037
}
@@ -5953,6 +6057,7 @@ impl Interpreter {
59536057
// Build fd table: fd1 = stdout pipe, fd2 = stderr pipe
59546058
let mut fd1 = FdTarget::Stdout;
59556059
let mut fd2 = FdTarget::Stderr;
6060+
self.pending_fd_targets.clear();
59566061

59576062
for redirect in redirects {
59586063
match redirect.kind {
@@ -6018,10 +6123,22 @@ impl Interpreter {
60186123
_ => fd1 = exec_target,
60196124
}
60206125
} else {
6021-
match (src_fd, target_fd) {
6022-
(2, 1) => fd2 = fd1.clone(),
6023-
(1, 2) => fd1 = fd2.clone(),
6024-
_ => {}
6126+
// Resolve target from current fd table state
6127+
let resolved = match target_fd {
6128+
1 => Some(fd1.clone()),
6129+
2 => Some(fd2.clone()),
6130+
_ => None,
6131+
};
6132+
if let Some(target) = resolved {
6133+
match src_fd {
6134+
1 => fd1 = target,
6135+
2 => fd2 = target,
6136+
n if n >= 3 => {
6137+
// Store fd3+ target for routing pending_fd_output later
6138+
self.pending_fd_targets.push((n, target));
6139+
}
6140+
_ => {}
6141+
}
60256142
}
60266143
}
60276144
}
@@ -6033,42 +6150,19 @@ impl Interpreter {
60336150
}
60346151
}
60356152

6036-
// Route original stdout/stderr based on final fd table.
6037-
// Collect file writes to avoid double-borrow issues.
6153+
// Route stdout/stderr/fd3+ to their targets (non-async to avoid state machine bloat)
60386154
let orig_stdout = std::mem::take(&mut result.stdout);
60396155
let orig_stderr = std::mem::take(&mut result.stderr);
6040-
6041-
// Determine what goes where
6042-
let mut new_stdout = String::new();
6043-
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() {
6049-
if data.is_empty() {
6050-
continue;
6051-
}
6052-
match fd_target {
6053-
FdTarget::Stdout => new_stdout.push_str(data),
6054-
FdTarget::Stderr => new_stderr.push_str(data),
6055-
FdTarget::DevNull => {}
6056-
FdTarget::WriteFile(path, display) => {
6057-
file_writes
6058-
.entry(path.clone())
6059-
.or_insert_with(|| (String::new(), false, display.clone()))
6060-
.0
6061-
.push_str(data);
6062-
}
6063-
FdTarget::AppendFile(path, display) => {
6064-
file_writes
6065-
.entry(path.clone())
6066-
.or_insert_with(|| (String::new(), true, display.clone()))
6067-
.0
6068-
.push_str(data);
6069-
}
6070-
}
6071-
}
6156+
let (new_stdout, mut new_stderr, file_writes) = route_fd_table_content(
6157+
&orig_stdout,
6158+
&orig_stderr,
6159+
&fd1,
6160+
&fd2,
6161+
&self.pending_fd_targets,
6162+
&self.pending_fd_output,
6163+
);
6164+
self.pending_fd_output.clear();
6165+
self.pending_fd_targets.clear();
60726166

60736167
// Write files
60746168
for (path, (content, is_append, display_path)) in &file_writes {
@@ -10898,4 +10992,20 @@ echo "count=$COUNT"
1089810992
let result = run_script("shopt -s failglob; ls ./*.html 2>/dev/null; echo exit:$?").await;
1089910993
assert_eq!(result.stderr, "", "failglob: stderr should be suppressed");
1090010994
}
10995+
10996+
#[tokio::test]
10997+
async fn test_fd3_redirect_pattern() {
10998+
// Issue #1115: { echo "progress" 1>&3; echo "file content"; } 3>&1 >file
10999+
let result = run_script(
11000+
r#"{ echo "progress" 1>&3; echo "file content"; } 3>&1 > /tmp/test_fd.txt
11001+
cat /tmp/test_fd.txt"#,
11002+
)
11003+
.await;
11004+
let lines: Vec<&str> = result.stdout.lines().collect();
11005+
assert_eq!(
11006+
lines,
11007+
vec!["progress", "file content"],
11008+
"fd3 → stdout, fd1 → file"
11009+
);
11010+
}
1090111011
}

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)