Skip to content

Commit 94dba69

Browse files
authored
Do not use stdio to communicate with the sandbox. (#402)
This allows setting stdin/out/err FDs from the parent, which is useful to implement the Interactive task type.
1 parent 48d2178 commit 94dba69

3 files changed

Lines changed: 35 additions & 40 deletions

File tree

src/sandbox.rs

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
use std::io::{stdin, stdout};
1+
use std::env;
2+
use std::fs::File;
3+
use std::io::BufWriter;
24
use std::path::{Path, PathBuf};
3-
use std::process::{Command, Stdio};
5+
use std::process::Command;
46
use std::sync::atomic::{AtomicU32, Ordering};
57
use std::sync::Arc;
68

@@ -10,32 +12,32 @@ use tabox::result::SandboxExecutionResult;
1012
use tabox::{Sandbox, SandboxImplementation};
1113
use task_maker_exec::find_tools::find_tools_path;
1214
use task_maker_exec::{RawSandboxResult, SandboxRunner};
15+
use tempfile::NamedTempFile;
1316

14-
/// Actually parse the input and return the result.
15-
fn run_sandbox() -> Result<SandboxExecutionResult, Error> {
16-
let config =
17-
serde_json::from_reader(stdin()).context("Cannot read configuration from stdin")?;
17+
fn run_sandbox(config: &str) -> Result<SandboxExecutionResult, Error> {
18+
let config = serde_json::from_str(config).context("Cannot parse configuration")?;
1819
let sandbox = SandboxImplementation::run(config).context("Failed to create sandbox")?;
1920
let res = sandbox.wait().context("Failed to wait sandbox")?;
2021
Ok(res)
2122
}
2223

2324
/// Run the sandbox for an execution.
24-
///
25-
/// It takes a `SandboxConfiguration`, JSON serialized via standard input and prints to standard
26-
/// output a `RawSandboxResult`, JSON serialized.
2725
pub fn main_sandbox() {
28-
match run_sandbox() {
29-
Ok(res) => {
30-
serde_json::to_writer(stdout(), &RawSandboxResult::Success(res))
31-
.expect("Failed to print result");
32-
}
26+
let mut args = env::args().skip(2);
27+
let configuration = args.next().unwrap();
28+
let output_file = args.next().unwrap();
29+
let result = match run_sandbox(&configuration) {
30+
Ok(res) => RawSandboxResult::Success(res),
3331
Err(e) => {
3432
let err = format!("Error: {e:?}");
35-
serde_json::to_writer(stdout(), &RawSandboxResult::Error(err))
36-
.expect("Failed to print result");
33+
RawSandboxResult::Error(err)
3734
}
38-
}
35+
};
36+
let f = File::options()
37+
.write(true)
38+
.open(&output_file)
39+
.expect("Failed to create output file");
40+
serde_json::to_writer(BufWriter::new(f), &result).expect("Failed to print result");
3941
}
4042

4143
/// Run the sandbox integrated in the task-maker-tools binary.
@@ -68,27 +70,19 @@ fn tools_sandbox_internal(
6870
config: SandboxConfiguration,
6971
pid: Arc<AtomicU32>,
7072
) -> Result<RawSandboxResult, Error> {
73+
let config = serde_json::to_string(&config).context("Failed to serialize config")?;
74+
// TODO(veluca): it would be nice to write the result in the sandbox.
75+
let outfile = NamedTempFile::new().context("Failed creating output tempfile")?;
7176
let mut cmd = Command::new(tools_path)
7277
.arg("internal-sandbox")
73-
.stdin(Stdio::piped())
74-
.stdout(Stdio::piped())
75-
.stderr(Stdio::piped())
78+
.arg(config)
79+
.arg(outfile.path().as_os_str())
7680
.spawn()
7781
.context("Cannot spawn the sandbox")?;
7882
pid.store(cmd.id(), Ordering::SeqCst);
79-
{
80-
let stdin = cmd.stdin.as_mut().context("Failed to open stdin")?;
81-
serde_json::to_writer(stdin, &config.build()).context("Failed to write config to stdin")?;
82-
}
83-
let output = cmd
84-
.wait_with_output()
85-
.context("Failed to wait for the process")?;
86-
if !output.status.success() {
87-
bail!(
88-
"Sandbox process failed: {}\n{}",
89-
output.status.to_string(),
90-
String::from_utf8_lossy(&output.stderr)
91-
);
83+
let status = cmd.wait().context("Failed to wait for the process")?;
84+
if !status.success() {
85+
bail!("Sandbox process failed: {}", status.to_string());
9286
}
93-
serde_json::from_slice(&output.stdout).context("Invalid output from sandbox")
87+
serde_json::from_reader(outfile).context("Invalid output from sandbox")
9488
}

src/tools/main.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::env;
2+
13
use clap::Parser;
24
use task_maker_rust::error::NiceError;
35
use task_maker_rust::tools::add_solution_checks::main_add_solution_checks;
@@ -19,6 +21,11 @@ use task_maker_rust::tools::terry_statement::main_terry_statement;
1921
use task_maker_rust::tools::worker::main_worker;
2022

2123
fn main() {
24+
// We run before any other initialization, to avoid polluting stderr.
25+
if env::args().nth(1).as_deref() == Some("internal-sandbox") {
26+
return task_maker_rust::sandbox::main_sandbox();
27+
}
28+
2229
rustls::crypto::ring::default_provider()
2330
.install_default()
2431
.expect("Failed to install rustls crypto provider");
@@ -43,7 +50,6 @@ fn main() {
4350
Tool::ExportSolutionChecks(opt) => main_export_solution_checks(opt),
4451
Tool::ExportBooklet(opt) => main_export_booklet(opt),
4552
Tool::EvalServer(opt) => main_eval_server(opt),
46-
Tool::InternalSandbox => return task_maker_rust::main_sandbox(),
4753
}
4854
.nice_unwrap()
4955
}

src/tools/opt.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,4 @@ pub enum Tool {
6565
ExportBooklet(ExportBookletOpt),
6666
/// Start a web server for evaluating arbitrary code.
6767
EvalServer(EvalServerOpt),
68-
/// Run the sandbox instead of the normal task-maker.
69-
///
70-
/// This option is left as undocumented as it's not part of the public API.
71-
#[clap(hide = true)]
72-
InternalSandbox,
7368
}

0 commit comments

Comments
 (0)