Merge pull request #128 from ltratt/more_testing

Add more tests

tests/actions.rs

@@ -0,0 +1,146 @@
+use std::{fs::read_to_string, thread::sleep};
+use tempfile::Builder;
+
+mod common;
+use common::{run_success, SNARE_PAUSE};
+
+#[test]
+fn multiple() {
+    // This tests that when there are multiple actions, the correct one takes effect.
+
+    let td = Builder::new()
+        .tempdir_in(env!("CARGO_TARGET_TMPDIR"))
+        .unwrap();
+    let mut tp1 = td.path().to_owned();
+    tp1.push("t1");
+    let tp1s = tp1.as_path().to_str().unwrap();
+    let mut tp2 = td.path().to_owned();
+    tp2.push("t2");
+    let tp2s = tp2.as_path().to_str().unwrap();
+    let mut tp3 = td.path().to_owned();
+    tp3.push("t3");
+    let tp3s = tp3.as_path().to_str().unwrap();
+
+    run_success(
+        &format!(
+            r#"listen = "127.0.0.1:0";
+github {{
+  // Should match
+  match "*" {{
+    secret = "secretsecret";
+  }}
+  // Shouldn't match
+  match "testuser/testrep" {{
+    cmd = "touch {tp1s}";
+    secret = "secretsecretsecret";
+  }}
+  // Should match but will be overridden by the following entry
+  match "testuser/testrepo" {{
+    cmd = "touch {tp2s}";
+  }}
+  // Should match and override the previous entry
+  match "testuser/testrepo" {{
+    cmd = "touch {tp3s}";
+  }}
+}}"#
+        ),
+        &[(
+            move |port| {
+                Ok(format!(
+                    r#"POST /payload HTTP/1.1
+Host: 127.0.0.1:{port}
+Content-Length: 104
+X-GitHub-Delivery: 72d3162e-cc78-11e3-81ab-4c9367dc0958
+X-Hub-Signature-256: sha256=292e1ce3568fecd98589c71938e19afee9b04b7fe11886d5478d802416bbde66
+User-Agent: GitHub-Hookshot/044aadd
+Content-Type: application/json
+X-GitHub-Event: issues
+X-GitHub-Hook-ID: 292430182
+X-GitHub-Hook-Installation-Target-ID: 79929171
+X-GitHub-Hook-Installation-Target-Type: repository
+
+payload={{
+  "repository": {{
+    "owner": {{
+      "login": "testuser"
+    }},
+    "name": "testrepo"
+  }}
+}}"#
+                ))
+            },
+            move |response: String| {
+                if response.starts_with("HTTP/1.1 200 OK") {
+                    sleep(SNARE_PAUSE);
+                    assert!(!tp1.is_file());
+                    assert!(!tp2.is_file());
+                    assert!(tp3.is_file());
+                    Ok(())
+                } else {
+                    Err(format!("Received HTTP response '{response}'").into())
+                }
+            },
+        )],
+    )
+    .unwrap();
+}
+
+#[test]
+fn errorcmd() {
+    // This tests both large stdout/stderr output from `cmd` as well as that `errorcmd` works.
+
+    let td = Builder::new()
+        .tempdir_in(env!("CARGO_TARGET_TMPDIR"))
+        .unwrap();
+    let mut tp = td.path().to_owned();
+    tp.push("t");
+    let tps = tp.as_path().to_str().unwrap();
+
+    run_success(
+        &format!(
+            r#"listen = "127.0.0.1:0";
+github {{
+  match ".*" {{
+    cmd = "dd if=/dev/zero bs=1k count=256 status=none && dd if=/dev/zero of=/dev/stderr bs=1k count=256 status=none && exit 1";
+    errorcmd = "cp %s {tps}";
+    secret = "secretsecret";
+  }}
+}}"#
+        ),
+        &[(
+            move |port| {
+                Ok(format!(
+                    r#"POST /payload HTTP/1.1
+Host: 127.0.0.1:{port}
+Content-Length: 104
+X-GitHub-Delivery: 72d3162e-cc78-11e3-81ab-4c9367dc0958
+X-Hub-Signature-256: sha256=292e1ce3568fecd98589c71938e19afee9b04b7fe11886d5478d802416bbde66
+User-Agent: GitHub-Hookshot/044aadd
+Content-Type: application/json
+X-GitHub-Event: issues
+X-GitHub-Hook-ID: 292430182
+X-GitHub-Hook-Installation-Target-ID: 79929171
+X-GitHub-Hook-Installation-Target-Type: repository
+
+payload={{
+  "repository": {{
+    "owner": {{
+      "login": "testuser"
+    }},
+    "name": "testrepo"
+  }}
+}}"#
+                ))
+            },
+            move |response: String| {
+                if response.starts_with("HTTP/1.1 200 OK") {
+                    sleep(SNARE_PAUSE);
+                    assert_eq!(read_to_string(&tp).unwrap().len(), 2 * 256 * 1024);
+                    Ok(())
+                } else {
+                    Err(format!("Received HTTP response '{response}'").into())
+                }
+            },
+        )],
+    ).unwrap();
+}

tests/auth.rs

@@ -1,8 +1,8 @@
-use std::{error::Error, path::PathBuf};
+use std::{error::Error, path::PathBuf, thread::sleep};
 use tempfile::{Builder, TempDir};
 
 mod common;
-use common::run_success;
+use common::{run_success, SNARE_PAUSE};
 
 fn cfg(correct_secret: bool) -> Result<(String, TempDir, PathBuf), Box<dyn Error>> {
     let secret = if correct_secret {
@@ -73,15 +73,18 @@ fn successful_auth() -> Result<(), Box<dyn Error>> {
     // https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries#testing-the-webhook-payload-validation
     run_success(
         &cfg,
-        move |port| Ok(req(port, true)),
-        move |response| {
-            if response.starts_with("HTTP/1.1 200 OK") {
-                assert!(tp.is_file());
-                Ok(())
-            } else {
-                Err(format!("Received HTTP response '{response}'").into())
-            }
-        },
+        &[(
+            move |port| Ok(req(port, true)),
+            move |response| {
+                if response.starts_with("HTTP/1.1 200 OK") {
+                    sleep(SNARE_PAUSE);
+                    assert!(tp.is_file());
+                    Ok(())
+                } else {
+                    Err(format!("Received HTTP response '{response}'").into())
+                }
+            },
+        )],
     )
 }
 
@@ -97,15 +100,18 @@ fn bad_sha256() -> Result<(), Box<dyn Error>> {
     // https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries#testing-the-webhook-payload-validation
     run_success(
         &cfg,
-        move |port| Ok(req(port, false)),
-        move |response| {
-            if response.starts_with("HTTP/1.1 400") {
-                assert!(!tp.is_file());
-                Ok(())
-            } else {
-                Err(format!("Received HTTP response '{response}'").into())
-            }
-        },
+        &[(
+            move |port| Ok(req(port, false)),
+            move |response| {
+                if response.starts_with("HTTP/1.1 400") {
+                    sleep(SNARE_PAUSE);
+                    assert!(!tp.is_file());
+                    Ok(())
+                } else {
+                    Err(format!("Received HTTP response '{response}'").into())
+                }
+            },
+        )],
     )
 }
 
@@ -120,14 +126,17 @@ fn wrong_secret() -> Result<(), Box<dyn Error>> {
     // https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries#testing-the-webhook-payload-validation
     run_success(
         &cfg,
-        move |port| Ok(req(port, true)),
-        move |response| {
-            if response.starts_with("HTTP/1.1 400") {
-                assert!(!tp.is_file());
-                Ok(())
-            } else {
-                Err(format!("Received HTTP response '{response}'").into())
-            }
-        },
+        &[(
+            move |port| Ok(req(port, true)),
+            move |response| {
+                if response.starts_with("HTTP/1.1 400") {
+                    sleep(SNARE_PAUSE);
+                    assert!(!tp.is_file());
+                    Ok(())
+                } else {
+                    Err(format!("Received HTTP response '{response}'").into())
+                }
+            },
+        )],
     )
 }

tests/common/mod.rs

@@ -9,8 +9,9 @@ use std::{
     io::{Read, Write},
     net::{Shutdown, TcpStream},
     os::unix::process::ExitStatusExt,
-    panic::{catch_unwind, resume_unwind, UnwindSafe},
+    panic::{catch_unwind, resume_unwind, RefUnwindSafe, UnwindSafe},
     process::{Child, Stdio},
+    rc::Rc,
     thread::sleep,
     time::Duration,
 };
@@ -24,66 +25,87 @@ use wait_timeout::ChildExt;
 /// always have a box which (perhaps because it's loaded) causes arbitrarily long pauses. So we set
 /// a fairly high threshold, hoping that will deal with most reasonable cases, and then cross our
 /// fingers!
-static SNARE_PAUSE: Duration = Duration::from_secs(1);
+pub static SNARE_PAUSE: Duration = Duration::from_secs(1);
 /// When we send SIGTERM to a snare instance, what is the maximum time we should wait for the
 /// process to exit? We don't expect this maximum time to be reached often, so a fairly high
 /// threshold is tolerable, and doing so maximises the chance that we get something useful printed
 /// to stdout/stderr.
 static SNARE_WAIT_TIMEOUT: Duration = Duration::from_secs(5);
 
-pub fn run_success<F, G>(cfg: &str, req: F, check_response: G) -> Result<(), Box<dyn Error>>
+#[allow(dead_code)]
+pub fn run_success<F, G>(cfg: &str, req_check: &[(F, G)]) -> Result<(), Box<dyn Error>>
 where
-    F: FnOnce(u16) -> Result<String, Box<dyn Error>> + UnwindSafe + 'static,
-    G: FnOnce(String) -> Result<(), Box<dyn Error>> + UnwindSafe + 'static,
+    F: Fn(u16) -> Result<String, Box<dyn Error>> + RefUnwindSafe + UnwindSafe + 'static,
+    G: Fn(String) -> Result<(), Box<dyn Error>> + RefUnwindSafe + UnwindSafe + 'static,
 {
     let (mut sn, tp) = snare_command(cfg)?;
+    match sn.try_wait() {
+        Ok(None) => (),
+        _ => todo!(),
+    }
+    let tp = Rc::new(tp);
 
-    // Try as hard as possible not to leave snare processes lurking around after the tests are run,
-    // by sending them SIGTERM in as many cases as we reasonably can. Note that `catch_unwind` does
-    // not guarantee to catch all panic-y situations, so this can never be perfect.
-    let r = catch_unwind(move || {
-        let port = read_to_string(tp.path()).unwrap().parse::<u16>().unwrap();
+    for (req, check) in req_check {
+        let tp = Rc::clone(&tp);
+        let r = catch_unwind(move || {
+            let port = read_to_string(tp.path()).unwrap().parse::<u16>().unwrap();
 
-        let req = req(port).unwrap();
-        let mut stream = TcpStream::connect(("127.0.0.1", port)).unwrap();
-        stream.write_all(req.as_bytes()).unwrap();
-        stream.shutdown(Shutdown::Write).unwrap();
-        let mut response = String::new();
-        stream.read_to_string(&mut response).unwrap();
+            let req = req(port).unwrap();
+            let mut stream = TcpStream::connect(("127.0.0.1", port)).unwrap();
+            stream.write_all(req.as_bytes()).unwrap();
+            stream.shutdown(Shutdown::Write).unwrap();
+            let mut response = String::new();
+            stream.read_to_string(&mut response).unwrap();
 
-        // We want to wait for snare to execute any actions that might come with the request. Again, we
-        // have no way of doing that other than waiting and hoping.
-        sleep(SNARE_PAUSE);
-        check_response(response).unwrap();
-    });
+            check(response).unwrap();
+        });
 
-    kill(Pid::from_raw(sn.id().try_into().unwrap()), Signal::SIGTERM).unwrap();
-    if r.is_err() {
-        let ec = match sn.wait_timeout(SNARE_WAIT_TIMEOUT) {
-            Err(e) => e.to_string(),
-            Ok(None) => "stalled without exiting".to_owned(),
-            Ok(Some(ec)) => ec
-                .code()
-                .map(|i| i.to_string())
-                .unwrap_or_else(|| "<no exit code>".to_owned()),
-        };
-        let mut stdout = String::new();
-        let mut stderr = String::new();
-        sn.stdout.as_mut().unwrap().read_to_string(&mut stdout).ok();
-        if !stdout.is_empty() {
-            stdout.push('\n');
+        // Try as hard as possible not to leave snare processes lurking around after the tests are run,
+        // by sending them SIGTERM in as many cases as we reasonably can. Note that `catch_unwind` does
+        // not guarantee to catch all panic-y situations, so this can never be perfect.
+        if let Err(r) = r {
+            kill(Pid::from_raw(sn.id().try_into().unwrap()), Signal::SIGTERM).unwrap();
+            let ec = match sn.wait_timeout(SNARE_WAIT_TIMEOUT) {
+                Err(e) => e.to_string(),
+                Ok(None) => "stalled without exiting".to_owned(),
+                Ok(Some(ec)) => ec
+                    .code()
+                    .map(|i| i.to_string())
+                    .unwrap_or_else(|| "<no exit code>".to_owned()),
+            };
+            let mut stdout = String::new();
+            let mut stderr = String::new();
+            sn.stdout.as_mut().unwrap().read_to_string(&mut stdout).ok();
+            if !stdout.is_empty() {
+                stdout.push('\n');
+            }
+            sn.stderr.as_mut().unwrap().read_to_string(&mut stderr).ok();
+            eprintln!(
+                "snare child process:\n  Exit status: {ec}\n\n  stdout:\n{stdout}\n  stderr:\n{stderr}"
+            );
+            resume_unwind(r);
         }
-        sn.stderr.as_mut().unwrap().read_to_string(&mut stderr).ok();
-        eprintln!(
-            "snare child process:\n  Exit status: {ec}\n\n  stdout:\n{stdout}\n  stderr:\n{stderr}"
-        );
     }
 
-    match r {
-        Ok(()) => (),
-        Err(r) => resume_unwind(r),
+    kill(Pid::from_raw(sn.id().try_into().unwrap()), Signal::SIGTERM).unwrap();
+    match sn.wait_timeout(SNARE_WAIT_TIMEOUT) {
+        Err(e) => Err(e.into()),
+        Ok(Some(es)) => {
+            if let Some(Signal::SIGTERM) = es.signal().map(|x| Signal::try_from(x).unwrap()) {
+                Ok(())
+            } else {
+                Err(format!("Expected successful exit but got '{es:?}'").into())
+            }
+        }
+        Ok(None) => Err("timeout waiting for snare to exit".into()),
     }
+}
 
+#[allow(dead_code)]
+pub fn run_preserver_success(cfg: &str) -> Result<(), Box<dyn Error>> {
+    let (mut sn, _tp) = snare_command(cfg)?;
+    sleep(SNARE_PAUSE);
+    kill(Pid::from_raw(sn.id().try_into().unwrap()), Signal::SIGTERM).unwrap();
     match sn.wait_timeout(SNARE_WAIT_TIMEOUT) {
         Err(e) => Err(e.into()),
         Ok(Some(es)) => {
@@ -100,6 +122,7 @@ where
 #[allow(dead_code)]
 pub fn run_preserver_error(cfg: &str) -> Result<(), Box<dyn Error>> {
     let (mut sn, _tp) = snare_command(cfg)?;
+    sleep(SNARE_PAUSE);
     match sn.wait_timeout(SNARE_WAIT_TIMEOUT) {
         Err(e) => Err(e.into()),
         Ok(Some(es)) => {