Fix quoted and ${VAR} requirements includes (#994)

crates/socket-patch-cli/tests/scan_requirements_lock_only.rs

@@ -5,7 +5,10 @@
 //! release:
 //!
 //! * #523: whitespace around `==` and the legacy `name (==X)` form;
-//! * #412: pins reached through in-root `-r` includes.
+//! * #412: pins reached through in-root `-r` includes;
+//! * #994: include targets pip unquotes (`-r "dev reqs.txt"`,
+//!   `--requirement="dev.txt"`, `-r dev\ reqs.txt`) or expands
+//!   (`-r ${REQDIR}/dev.txt`).
 //!
 //! Driven through the built binary against a mock patch API; the
 //! assertion is what discovery sends to the batch endpoint and the
@@ -32,7 +35,12 @@ async fn mount_empty_batch(mock: &MockServer) {
         .await;
 }
 
-fn run_scan(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, serde_json::Value) {
+fn run_scan(
+    root: &Path,
+    mock_uri: &str,
+    extra: &[&str],
+    envs: &[(&str, &str)],
+) -> (i32, serde_json::Value) {
     let mut argv = vec![
         "scan",
         "--json",
@@ -51,6 +59,7 @@ fn run_scan(root: &Path, mock_uri: &str, extra: &[&str]) -> (i32, serde_json::Va
         .env("SOCKET_TELEMETRY_DISABLED", "1")
         .env_remove("VIRTUAL_ENV")
         .env_remove("CONDA_PREFIX")
+        .envs(envs.iter().copied())
         .output()
         .expect("run socket-patch");
     let stdout = String::from_utf8_lossy(&out.stdout);
@@ -87,6 +96,14 @@ async fn batch_purls(mock: &MockServer) -> Vec<String> {
 }
 
 async fn assert_lock_only_discovers(files: &[(&str, &str)], expected: &[&str]) {
+    assert_lock_only_discovers_with_env(files, &[], expected).await;
+}
+
+async fn assert_lock_only_discovers_with_env(
+    files: &[(&str, &str)],
+    envs: &[(&str, &str)],
+    expected: &[&str],
+) {
     for mode in [&[][..], &["--vendor"][..]] {
         let mock = MockServer::start().await;
         mount_empty_batch(&mock).await;
@@ -96,7 +113,7 @@ async fn assert_lock_only_discovers(files: &[(&str, &str)], expected: &[&str]) {
             std::fs::create_dir_all(p.parent().unwrap()).unwrap();
             std::fs::write(p, content).unwrap();
         }
-        let (code, v) = run_scan(tmp.path(), &mock.uri(), mode);
+        let (code, v) = run_scan(tmp.path(), &mock.uri(), mode, envs);
         assert_eq!(code, 0, "mode={mode:?}: {v}");
         assert_eq!(
             v["lockfileOnlyPackages"].as_u64(),
@@ -148,3 +165,45 @@ async fn lock_only_scan_discovers_included_pins() {
     )
     .await;
 }
+
+/// #994: pip `shlex`-splits an include line's options, so a quoted or
+/// backslash-escaped target names the file without its quotes, and a
+/// target with a space is one path, not two words.
+#[tokio::test]
+async fn lock_only_scan_discovers_quoted_include_targets() {
+    let cases: &[(&str, &str, &str)] = &[
+        ("dq", "-r \"dev reqs.txt\"\n", "dev reqs.txt"),
+        ("sq", "-r 'dev reqs.txt'\n", "dev reqs.txt"),
+        ("dq_nospace", "-r \"dev.txt\"\n", "dev.txt"),
+        ("bs", "-r dev\\ reqs.txt\n", "dev reqs.txt"),
+        ("longq", "--requirement \"dev.txt\"\n", "dev.txt"),
+        ("eqq", "--requirement=\"dev.txt\"\n", "dev.txt"),
+        ("attached", "-r\"dev reqs.txt\"\n", "dev reqs.txt"),
+    ];
+    for (case, root, include) in cases {
+        eprintln!("case {case}");
+        assert_lock_only_discovers(
+            &[
+                ("requirements.txt", root),
+                (include, "sp-fixture-quoted==1.0.0\n"),
+            ],
+            &["pkg:pypi/sp-fixture-quoted@1.0.0"],
+        )
+        .await;
+    }
+}
+
+/// #994: pip expands `${NAME}` from the environment before it parses the
+/// line, so `-r ${REQDIR}/dev.txt` follows `$REQDIR`.
+#[tokio::test]
+async fn lock_only_scan_discovers_env_var_include_target() {
+    assert_lock_only_discovers_with_env(
+        &[
+            ("requirements.txt", "-r ${SP_TEST_REQDIR}/dev.txt\n"),
+            ("sub/dev.txt", "sp-fixture-env==1.0.0\n"),
+        ],
+        &[("SP_TEST_REQDIR", "sub")],
+        &["pkg:pypi/sp-fixture-env@1.0.0"],
+    )
+    .await;
+}

crates/socket-patch-core/src/crawlers/gradle_cache.rs

@@ -70,8 +70,7 @@ pub fn hash_eq(dir_name: &str, sha1_hex: &str) -> bool {
 /// Whether `bytes` are the pristine download Gradle stored in the hash
 /// directory `dir_name` (their sha1 names it).
 pub fn pristine(dir_name: &str, bytes: &[u8]) -> bool {
-    use sha1::{Digest, Sha1};
-    hash_eq(dir_name, &hex::encode(Sha1::digest(bytes)))
+    hash_eq(dir_name, &crate::utils::digest::sha1_hex_of(bytes))
 }
 
 /// Whether `path` is a version directory of a `files-2.1` tree
@@ -432,8 +431,6 @@ impl DerivedIndex {
     /// The [`DerivedCopies`] of the jar `jar_leaf` whose pristine bytes
     /// hash to `pristine_sha1`.
     pub fn query(&self, jar_leaf: &str, pristine_sha1: &str) -> DerivedCopies {
-        use sha1::{Digest, Sha1};
-
         let instrumented = format!("instrumented-{jar_leaf}");
         let mut out = DerivedCopies {
             incomplete: self.incomplete,
@@ -460,7 +457,9 @@ impl DerivedIndex {
                 out.stale.push(path.clone());
             } else if name == jar_leaf || name == instrumented {
                 match crate::utils::fs::read_regular_to_bytes_sync(path) {
-                    Ok(bytes) if hash_eq(&hex::encode(Sha1::digest(&bytes)), pristine_sha1) => {
+                    Ok(bytes)
+                        if hash_eq(&crate::utils::digest::sha1_hex_of(&bytes), pristine_sha1) =>
+                    {
                         out.stale.push(path.clone())
                     }
                     Ok(_) => out.unknown.push(path.clone()),

crates/socket-patch-core/src/patch/jvm_jar.rs

@@ -25,8 +25,6 @@
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
-use sha1::Digest as _;
-
 use crate::crawlers::gradle_cache;
 use crate::hash::git_sha256::compute_git_sha256_from_bytes;
 use crate::manifest::schema::PatchFileInfo;
@@ -353,12 +351,11 @@ fn unpatched_members(
 }
 
 fn sha256_hex(bytes: &[u8]) -> String {
-    use sha2::Digest as _;
-    hex::encode(sha2::Sha256::digest(bytes))
+    crate::utils::digest::sha256_hex_of(bytes)
 }
 
 fn sha1_hex(bytes: &[u8]) -> String {
-    hex::encode(sha1::Sha1::digest(bytes))
+    crate::utils::digest::sha1_hex_of(bytes)
 }
 
 /// `<socket_dir>/jvm-originals/<sha256>.jar`.

crates/socket-patch-core/src/patch/sidecars/maven.rs

@@ -17,8 +17,6 @@
 
 use std::path::{Path, PathBuf};
 
-use sha1::Digest as _;
-
 use super::{
     SidecarAdvisory, SidecarAdvisoryCode, SidecarError, SidecarFile, SidecarFileAction,
     SidecarPayload, SidecarSeverity,
@@ -44,7 +42,7 @@ impl Algo {
 
     fn digest(self, bytes: &[u8]) -> String {
         match self {
-            Algo::Sha1 => hex::encode(sha1::Sha1::digest(bytes)),
+            Algo::Sha1 => crate::utils::digest::sha1_hex_of(bytes),
             Algo::Md5 => hex::encode(md5(bytes)),
         }
     }

crates/socket-patch-core/src/utils/requirements.rs

@@ -93,6 +93,99 @@ pub(crate) fn strip_comment(text: &str) -> &str {
     split_comment(text).0
 }
 
+/// pip's `expand_env_variables`: each `${NAME}` whose `NAME` is
+/// `[A-Z0-9_]+` is replaced by `lookup(NAME)`; an unset or empty variable
+/// leaves the reference as written. pip expands after stripping comments
+/// and before it splits the options, so a variable can carry quotes or
+/// spaces that the split then reads.
+pub(crate) fn expand_env_vars(code: &str, lookup: impl Fn(&str) -> Option<String>) -> String {
+    let mut out = String::with_capacity(code.len());
+    let mut rest = code;
+    while let Some(start) = rest.find("${") {
+        out.push_str(&rest[..start]);
+        let after = &rest[start + 2..];
+        let name_len = after
+            .find(|c: char| !(c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_'))
+            .unwrap_or(after.len());
+        let value = (name_len > 0 && after[name_len..].starts_with('}'))
+            .then(|| lookup(&after[..name_len]))
+            .flatten()
+            .filter(|v| !v.is_empty());
+        match value {
+            Some(v) => {
+                out.push_str(&v);
+                rest = &after[name_len + 1..];
+            }
+            None => {
+                out.push_str("${");
+                rest = after;
+            }
+        }
+    }
+    out.push_str(rest);
+    out
+}
+
+/// Python's `shlex.split` (POSIX mode), which pip runs over a line's
+/// options: whitespace separates words; `'…'` is literal; inside `"…"` a
+/// backslash escapes only `"` and `\`; elsewhere a backslash escapes any
+/// character; adjacent quoted and bare parts join into one word, and `""`
+/// is an empty word. `None` for an unclosed quote or a trailing lone
+/// backslash, which pip refuses ("Could not split options").
+pub(crate) fn shlex_split(text: &str) -> Option<Vec<String>> {
+    let mut words = Vec::new();
+    let mut word = String::new();
+    let mut in_word = false;
+    let mut chars = text.chars();
+    while let Some(c) = chars.next() {
+        match c {
+            ' ' | '\t' | '\r' | '\n' => {
+                if in_word {
+                    words.push(std::mem::take(&mut word));
+                    in_word = false;
+                }
+            }
+            '\\' => {
+                word.push(chars.next()?);
+                in_word = true;
+            }
+            '\'' => {
+                in_word = true;
+                loop {
+                    match chars.next()? {
+                        '\'' => break,
+                        ch => word.push(ch),
+                    }
+                }
+            }
+            '"' => {
+                in_word = true;
+                loop {
+                    match chars.next()? {
+                        '"' => break,
+                        '\\' => {
+                            let next = chars.next()?;
+                            if next != '"' && next != '\\' {
+                                word.push('\\');
+                            }
+                            word.push(next);
+                        }
+                        ch => word.push(ch),
+                    }
+                }
+            }
+            _ => {
+                word.push(c);
+                in_word = true;
+            }
+        }
+    }
+    if in_word {
+        words.push(word);
+    }
+    Some(words)
+}
+
 /// The `(name as spelled, version)` of an exact `name[extras]==X` registry
 /// requirement (a logical line's code part; an optional `; marker` and
 /// options may follow), `None` for anything else — ranges, `===`, wildcards
@@ -364,4 +457,78 @@ mod tests {
         assert_eq!(archive_filename_coords("six.tar.gz"), None, "no version");
         assert_eq!(archive_filename_coords("six-1.0.egg"), None);
     }
+
+    #[test]
+    fn expand_env_vars_follows_pips_name_grammar() {
+        let lookup = |name: &str| match name {
+            "REQDIR" => Some("sub".to_string()),
+            "SPACED" => Some("\"dev reqs.txt\"".to_string()),
+            "EMPTY" => Some(String::new()),
+            _ => None,
+        };
+        assert_eq!(
+            expand_env_vars("-r ${REQDIR}/dev.txt", lookup),
+            "-r sub/dev.txt"
+        );
+        assert_eq!(
+            expand_env_vars("-r ${SPACED}", lookup),
+            "-r \"dev reqs.txt\""
+        );
+        assert_eq!(
+            expand_env_vars("${REQDIR}/${REQDIR}", lookup),
+            "sub/sub",
+            "every reference is expanded"
+        );
+        for kept in [
+            "-r ${UNSET}/dev.txt",
+            "-r ${EMPTY}/dev.txt",
+            "-r ${reqdir}/dev.txt",
+            "-r $REQDIR/dev.txt",
+            "-r ${}/dev.txt",
+            "-r ${REQDIR",
+            "-r ${REQ-DIR}/dev.txt",
+        ] {
+            assert_eq!(expand_env_vars(kept, lookup), kept, "{kept:?}");
+        }
+    }
+
+    #[test]
+    fn shlex_split_matches_python_posix_mode() {
+        let split = |s: &str| shlex_split(s).map(|w| w.join("|"));
+        assert_eq!(split("-r dev.txt").as_deref(), Some("-r|dev.txt"));
+        assert_eq!(split("-r\t dev.txt ").as_deref(), Some("-r|dev.txt"));
+        assert_eq!(
+            split("-r \"dev reqs.txt\"").as_deref(),
+            Some("-r|dev reqs.txt")
+        );
+        assert_eq!(
+            split("-r 'dev reqs.txt'").as_deref(),
+            Some("-r|dev reqs.txt")
+        );
+        assert_eq!(
+            split("-r dev\\ reqs.txt").as_deref(),
+            Some("-r|dev reqs.txt")
+        );
+        assert_eq!(
+            split("--requirement=\"dev.txt\"").as_deref(),
+            Some("--requirement=dev.txt")
+        );
+        assert_eq!(split("a\"b c\"'d e'f").as_deref(), Some("ab cd ef"));
+        assert_eq!(
+            split("'a\\b'").as_deref(),
+            Some("a\\b"),
+            "no escapes in '…'"
+        );
+        assert_eq!(
+            split("\"a\\\"b\\\\c\\d\"").as_deref(),
+            Some("a\"b\\c\\d"),
+            "in \"…\" only \\\" and \\\\ are escapes"
+        );
+        assert_eq!(split("sub\\dev.txt").as_deref(), Some("subdev.txt"));
+        assert_eq!(shlex_split("-r \"\"").unwrap(), vec!["-r", ""]);
+        assert_eq!(shlex_split("").unwrap(), Vec::<String>::new());
+        for unbalanced in ["-r \"dev.txt", "-r 'dev.txt", "-r dev.txt\\"] {
+            assert_eq!(shlex_split(unbalanced), None, "{unbalanced:?}");
+        }
+    }
 }