Skip to content

Commit e00d366

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 e00d366

File tree

2 files changed

+153
-38
lines changed

2 files changed

+153
-38
lines changed

crates/bashkit/src/interpreter/mod.rs

Lines changed: 144 additions & 38 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+
/// 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>,
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
}
@@ -2889,6 +2894,87 @@ enum FdTarget {
28892894
DevNull,
28902895
}
28912896

2897+
/// Route fd1/fd2/fd3+ content to their targets. Extracted from the async
2898+
/// `apply_redirections_fd_table` to keep these locals out of the async state machine.
2899+
#[inline(never)]
2900+
fn route_fd_table_content(
2901+
orig_stdout: &str,
2902+
orig_stderr: &str,
2903+
fd1: &FdTarget,
2904+
fd2: &FdTarget,
2905+
extra_fd_targets: &[(i32, FdTarget)],
2906+
pending: &HashMap<i32, String>,
2907+
) -> (
2908+
String,
2909+
String,
2910+
std::collections::HashMap<PathBuf, (String, bool, String)>,
2911+
) {
2912+
let mut new_stdout = String::new();
2913+
let mut new_stderr = String::new();
2914+
let mut file_writes: std::collections::HashMap<PathBuf, (String, bool, String)> =
2915+
std::collections::HashMap::new();
2916+
2917+
let route = |data: &str,
2918+
target: &FdTarget,
2919+
fw: &mut std::collections::HashMap<PathBuf, (String, bool, String)>,
2920+
out: &mut String,
2921+
err: &mut String| {
2922+
if data.is_empty() {
2923+
return;
2924+
}
2925+
match target {
2926+
FdTarget::Stdout => out.push_str(data),
2927+
FdTarget::Stderr => err.push_str(data),
2928+
FdTarget::DevNull => {}
2929+
FdTarget::WriteFile(p, d) => {
2930+
fw.entry(p.clone())
2931+
.or_insert_with(|| (String::new(), false, d.clone()))
2932+
.0
2933+
.push_str(data);
2934+
}
2935+
FdTarget::AppendFile(p, d) => {
2936+
fw.entry(p.clone())
2937+
.or_insert_with(|| (String::new(), true, d.clone()))
2938+
.0
2939+
.push_str(data);
2940+
}
2941+
}
2942+
};
2943+
2944+
route(
2945+
orig_stdout,
2946+
fd1,
2947+
&mut file_writes,
2948+
&mut new_stdout,
2949+
&mut new_stderr,
2950+
);
2951+
route(
2952+
orig_stderr,
2953+
fd2,
2954+
&mut file_writes,
2955+
&mut new_stdout,
2956+
&mut new_stderr,
2957+
);
2958+
2959+
// Route pending fd3+ output
2960+
for (fd_num, data) in pending {
2961+
let target = extra_fd_targets
2962+
.iter()
2963+
.find(|(n, _)| n == fd_num)
2964+
.map(|(_, t)| t)
2965+
.unwrap_or(&FdTarget::Stdout);
2966+
route(
2967+
data,
2968+
target,
2969+
&mut file_writes,
2970+
&mut new_stdout,
2971+
&mut new_stderr,
2972+
);
2973+
}
2974+
2975+
(new_stdout, new_stderr, file_writes)
2976+
}
2977+
28922978
impl Interpreter {
28932979
/// Execute a sequence of commands (with errexit checking)
28942980
async fn execute_command_sequence(&mut self, commands: &[Command]) -> Result<ExecResult> {
@@ -5928,6 +6014,20 @@ impl Interpreter {
59286014
result.stderr.push_str(&result.stdout);
59296015
result.stdout = String::new();
59306016
}
6017+
(src, dst) if dst >= 3 => {
6018+
// Move content to pending_fd_output for compound
6019+
// redirect routing (e.g. `echo msg 1>&3` inside
6020+
// `{ ... } 3>&1 >file`).
6021+
let data = if src == 2 {
6022+
std::mem::take(&mut result.stderr)
6023+
} else {
6024+
std::mem::take(&mut result.stdout)
6025+
};
6026+
self.pending_fd_output
6027+
.entry(dst)
6028+
.or_default()
6029+
.push_str(&data);
6030+
}
59316031
_ => {}
59326032
}
59336033
}
@@ -5951,8 +6051,10 @@ impl Interpreter {
59516051
redirects: &[Redirect],
59526052
) -> Result<ExecResult> {
59536053
// Build fd table: fd1 = stdout pipe, fd2 = stderr pipe
6054+
// extra_fd_targets: fd3+ mappings (small vec, not HashMap to avoid state machine bloat)
59546055
let mut fd1 = FdTarget::Stdout;
59556056
let mut fd2 = FdTarget::Stderr;
6057+
let mut extra_fd_targets: Vec<(i32, FdTarget)> = Vec::new();
59566058

59576059
for redirect in redirects {
59586060
match redirect.kind {
@@ -6018,10 +6120,22 @@ impl Interpreter {
60186120
_ => fd1 = exec_target,
60196121
}
60206122
} else {
6021-
match (src_fd, target_fd) {
6022-
(2, 1) => fd2 = fd1.clone(),
6023-
(1, 2) => fd1 = fd2.clone(),
6024-
_ => {}
6123+
// Resolve target from current fd table state
6124+
let resolved = match target_fd {
6125+
1 => Some(fd1.clone()),
6126+
2 => Some(fd2.clone()),
6127+
_ => None,
6128+
};
6129+
if let Some(target) = resolved {
6130+
match src_fd {
6131+
1 => fd1 = target,
6132+
2 => fd2 = target,
6133+
n if n >= 3 => {
6134+
// Store fd3+ target for routing pending_fd_output later
6135+
extra_fd_targets.push((n, target));
6136+
}
6137+
_ => {}
6138+
}
60256139
}
60266140
}
60276141
}
@@ -6033,42 +6147,18 @@ impl Interpreter {
60336147
}
60346148
}
60356149

6036-
// Route original stdout/stderr based on final fd table.
6037-
// Collect file writes to avoid double-borrow issues.
6150+
// Route stdout/stderr/fd3+ to their targets (non-async to avoid state machine bloat)
60386151
let orig_stdout = std::mem::take(&mut result.stdout);
60396152
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-
}
6153+
let pending = std::mem::take(&mut self.pending_fd_output);
6154+
let (new_stdout, mut new_stderr, file_writes) = route_fd_table_content(
6155+
&orig_stdout,
6156+
&orig_stderr,
6157+
&fd1,
6158+
&fd2,
6159+
&extra_fd_targets,
6160+
&pending,
6161+
);
60726162

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

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)