diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index fcc0e6443..314658806 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -547,7 +547,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Six cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build), live or inside ``, resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier (a profile's overriding the live one), or no version in the root pom — the pin is omitted. A pre-v5 single-pom vendored repository holds only the main jar, so any such copy omits its reference. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Maven profile declarations** (`vex_maven_profile_unpatched`): a `` declaration of a pinned GA's main jar in the root `pom.xml` at any version other than the pin (the base release, another release, a `${property}` the pom does not resolve) overrides the pin whenever that profile is active, so the reference is omitted. A profile `` entry always counts; a profile `` entry counts only when management governs the pin (no direct `` outside ``), since a direct literal version beats management. A declaration without a `` takes the live pin, and a `pom` (or `java-source` / `javadoc`) typed one carries no code; neither withholds the statement. Activation is not evaluated: every profile counts, as in vendored mode, which rewrites profile declarations too; hosted scan leaves them as written (`redirect_maven_profile_dependency_unpatched`). The same holds for a pre-v5 single-pom vendored repository, which serves only the original version. **Maven user properties** (`vex_maven_config_override`): Maven resolves a `${property}` `` from `.mvn/maven.config`'s `-D` user properties before the pom's `` (`${project.*}` / `${pom.*}` always come from the model). A pin the pom spells through a property that `.mvn/maven.config` redefines resolves that other version, so the reference is omitted. A Socket version that only a `-D` user property supplies is not `pom.xml` wiring (hosted scan never writes one, and `rollback` / `remove` never edit `.mvn/maven.config`): it is no reference at all, only a diagnostic. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Six cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build), live or inside ``, resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier (a profile's overriding the live one), or no version in the root pom — the pin is omitted. A pre-v5 single-pom vendored repository holds only the main jar, so any such copy omits its reference. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Maven profile declarations** (`vex_maven_profile_unpatched`): a `` declaration of a pinned GA's main jar in the root `pom.xml` at any version other than the pin (the base release, another release, a `${property}` the pom does not resolve) overrides the pin whenever that profile is active, so the reference is omitted. A profile `` entry always counts; a profile `` entry counts only when management governs the pin (no direct `` outside ``), since a direct literal version beats management. A declaration without a `` takes the live pin, and a `pom` (or `java-source` / `javadoc`) typed one carries no code; neither withholds the statement. Activation is not evaluated: every profile counts, as in vendored mode, which rewrites profile declarations too; hosted scan leaves them as written (`redirect_maven_profile_dependency_unpatched`). A pin spelled through a property counts the same way: a `${property}` `` is interpolated through every property it names, the project's own `${project.version}` included (itself often spelled through `${revision}`), at most 16 levels deep (a cycle or an undefined name never resolves to a pin), and when any profile's `` — or any combination of profiles' together, each nested reference read with that combination active, as the vendored planner weighs them too — redefine one of those properties so that it resolves anything other than the pin (or nothing), the reference is omitted too; a `.mvn/maven.config` user property wins over every profile's. A profile declaration reads its own profile's `` first. When more than 8 profiles may change such a version, or a profile's `` cannot be parsed, the reference is omitted (fail closed); the rest of the pom is still read. The statement names the profiles that move the pin and the version they move it to. The same holds for a pre-v5 single-pom vendored repository, which serves only the original version. **Maven user properties** (`vex_maven_config_override`): Maven resolves a `${property}` `` from `.mvn/maven.config`'s `-D` user properties before the pom's `` (`${project.*}` / `${pom.*}` always come from the model). A pin the pom spells through a property that `.mvn/maven.config` redefines resolves that other version, so the reference is omitted. A Socket version that only a `-D` user property supplies is not `pom.xml` wiring (hosted scan never writes one, and `rollback` / `remove` never edit `.mvn/maven.config`): it is no reference at all, only a diagnostic. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -2678,7 +2678,7 @@ match it, withholds the statement: | `vex_deno_lock_copy` | run warning and `failed[].reason` | An npm pin is wired, but `deno.lock` locks the same `name@version`, which `deno install` installs without the wiring; no statement while it does. `vendor --check` and `scan` still treat the wiring as live. | | `vex_maven_reactor_root` | run warning and `failed[].reason` | A hosted Maven pin sits in a root `pom.xml` that declares `` / `` (a profile's included), where a module's own literal `` overrides it; only the root pom is read, so no statement (#261). `list`, `rollback` and `remove` still find the pin. | | `vex_maven_classifier_unpatched` | run warning and `failed[].reason` | A hosted (or pre-v5 vendored) Maven pin's GA also has an executable `` copy (not `sources` / `javadoc`; live or inside ``) at a version other than the pin, which the classifier-less pin does not retarget and the grant does not serve; no statement while it does. `list`, `rollback` and `remove` still find the pin. | -| `vex_maven_profile_unpatched` | run warning and `failed[].reason` | A Maven pin's GA main jar is also declared inside `` in the root `pom.xml` at a version other than the pin (a profile `` entry only where management governs the pin), which overrides the pin whenever that profile is active; no statement while it does. `list`, `rollback` and `remove` still find the pin. | +| `vex_maven_profile_unpatched` | run warning and `failed[].reason` | A Maven pin's GA main jar is also declared inside `` in the root `pom.xml` at a version other than the pin (a profile `` entry only where management governs the pin), or a profile's `` redefine a property the pin's `${property}` version names so it resolves another version, which overrides the pin whenever that profile is active; no statement while it does. `list`, `rollback` and `remove` still find the pin. | | `vex_maven_config_override` | run warning and `failed[].reason` | A hosted (or pre-v5 vendored) Maven pin is spelled through a `${property}` that a `-D` user property in `.mvn/maven.config` redefines, so Maven resolves another version; no statement while it does. `list`, `rollback` and `remove` still find the pin. | | `vex_gradle_derived_cache_unchecked` | run warning | The derived-cache walk was cut short (a very large transforms cache); the statement is still emitted, the copies the walk did reach were checked. | diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs index 2040be50c..448b60a39 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -233,7 +233,8 @@ const NOTE_MAVEN_REACTOR_ROOT: &str = "vex_maven_reactor_root"; /// (`vex::Unattested`). const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched"; /// Omission tag and note: a hosted Maven pin's GA is also declared inside -/// `` at another version, which overrides the pin whenever that +/// `` at another version, or a profile's `` move +/// the pin's `${property}` version, which overrides the pin whenever that /// profile is active (`vex::Unattested`). const NOTE_MAVEN_PROFILE_UNPATCHED: &str = "vex_maven_profile_unpatched"; /// Omission tag and note: a hosted (or pre-v5 vendored) Maven pin is spelled @@ -278,9 +279,10 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { ), UnattestedKind::MavenProfileUnpatched => ( NOTE_MAVEN_PROFILE_UNPATCHED, - "not attested while a profile declares it at another version; the wiring itself is \ - intact, so re-running `scan` does not change this (drop that profile or \ - set it to the pin, or re-patch with `scan --mode vendored`)", + "not attested while a profile declares it at, or its move it to, \ + another version; the wiring itself is intact, so re-running `scan` does not change \ + this (drop that profile or property or set it to the pin, or re-patch \ + with `scan --mode vendored`)", ), UnattestedKind::MavenConfigOverride => ( NOTE_MAVEN_CONFIG_OVERRIDE, diff --git a/crates/socket-patch-core/src/formats/maven/mod.rs b/crates/socket-patch-core/src/formats/maven/mod.rs index 6625d3e1e..ff8ccd27c 100644 --- a/crates/socket-patch-core/src/formats/maven/mod.rs +++ b/crates/socket-patch-core/src/formats/maven/mod.rs @@ -2,8 +2,16 @@ //! [`parse_pom`]), the Maven coordinate grammars and the hosted //! `-socket.` version split. Pure: text in, structure out. +mod properties; + use std::collections::BTreeMap; +use properties::{interpolate_lenient, read_properties, Interpolate}; +pub(crate) use properties::{ + interpolate_strict, is_model_reference, lookup_property, profile_combinations, Definitions, + PomLayer, MAX_COMBINED_PROFILES, MAX_INTERPOLATION_DEPTH, +}; + use super::xml::{blank_non_markup, child_text, elements, open_tags, Element}; // ── pure reader ── @@ -41,15 +49,27 @@ pub(crate) struct PomRepo { pub(crate) struct PomDep { pub(crate) group: String, pub(crate) artifact: String, - /// Trimmed literal version, `${prop}` resolved (one level) from the - /// root `` when possible: the version the pom itself - /// spells, which the hosted rewriter writes and rolls back. `None` when - /// the declaration has none. + /// Trimmed version, every `${prop}` interpolated + /// ([`interpolate_strict`]) from the project model (`project.*` / + /// `pom.*`), the `` of the profile holding the declaration + /// (if any), then the root ``: the version the pom itself + /// spells, which the hosted rewriter writes and rolls back. As written + /// when that fails (undefined, cyclic, too deep); `None` when the + /// declaration has none. pub(crate) version: Option, - /// [`Self::version`] as the build resolves it: `${prop}` resolved from - /// the build's user properties first (`.mvn/maven.config`'s `-D`), - /// else the root `` — Maven's own lookup order. + /// [`Self::version`] as the build resolves it: the build's user + /// properties (`.mvn/maven.config`'s `-D`) come before the profile's + /// and the root `` at every level — Maven's own lookup + /// order (the model still comes first). pub(crate) build_version: Option, + /// The trimmed `` text as written. + pub(crate) version_text: Option, + /// The other versions [`Self::build_version`] becomes when some + /// combination of profiles is active whose `` redefine a + /// property it names, directly, through another, or through the + /// project model ([`PomScope::resolve_in_build`]); the smallest + /// combination that moves it first. Empty when no profile can move it. + pub(crate) version_alternatives: Vec, /// The non-empty ``: a sibling artifact of the GA (sources, /// tests, a native build), managed and resolved apart from the main jar. pub(crate) classifier: Option, @@ -60,6 +80,58 @@ pub(crate) struct PomDep { pub(crate) in_profile: bool, } +/// A value some combination of active profiles gives a `${…}` text other +/// than its resolution with none active, and why. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Alternative { + /// The value, or the text as written when that combination leaves it + /// unresolved (undefined, cyclic, too deep) or it could not be worked + /// out. + pub(crate) value: String, + pub(crate) by: MovedBy, +} + +/// What gives an [`Alternative`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum MovedBy { + /// These profiles (their ``s) active together. + Profiles(Vec), + /// More profiles may change it than are combined + /// ([`MAX_COMBINED_PROFILES`]): not worked out, fail closed. + TooManyProfiles, + /// A `` `` this reader cannot parse: fail closed. + Unreadable(String), +} + +impl std::fmt::Display for MovedBy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + MovedBy::Profiles(ids) => { + let s = if ids.len() == 1 { "" } else { "s" }; + write!(f, "profile{s} {}", ids.join(" + ")) + } + MovedBy::TooManyProfiles => write!( + f, + "some combination of the more than {MAX_COMBINED_PROFILES} profiles that redefine \ + what it names" + ), + MovedBy::Unreadable(e) => { + write!(f, "a section this reader cannot parse ({e})") + } + } + } +} + +/// One `` inside ``: the `` element that +/// holds it (when the scan finds one), that profile's ``, and its +/// `name -> trimmed value` pairs. +#[derive(Debug)] +struct ProfileProperties { + holder: Option, + id: String, + props: BTreeMap, +} + /// Where Maven reads a root pom's model from: its text with comments and /// CDATA blanked (offsets kept), and the sections that are not the /// project's own resolution. `` / `` hold plugin @@ -75,16 +147,21 @@ pub(crate) struct PomScope { profiles: Vec, /// The root `` (see [`properties`]). properties: BTreeMap, - /// [`Self::properties`] with the build's user properties laid over - /// them, except `project.*` / `pom.*`, which Maven reads from the model - /// before any user property: what a `${name}` version resolves to in - /// the build. - build_properties: BTreeMap, - /// [`Self::properties`] plus the `${project.*}` model references a - /// coordinate may name (see [`model_refs`]). - coordinate_refs: BTreeMap, + /// The build's user properties (`.mvn/maven.config`'s `-D`). + user: BTreeMap, + /// The project model references, raw (see [`model_refs`]). + model: BTreeMap, + /// Each `` inside ``, once [`Self::read_profiles`] + /// has read them: merged over the root's while that profile is active. + profile_properties: Vec, + /// Why [`Self::read_profiles`] could not read them (every `${…}` then + /// fails closed in [`Self::resolve_in_build`]). + profiles_unreadable: Option, } +/// No user properties: the view of a version as the pom spells it. +static NO_USER_PROPERTIES: BTreeMap = BTreeMap::new(); + impl PomScope { /// Fails (fail-closed) on a pom that is not well-formed where it /// matters: no ``, or an unterminated element or CDATA. @@ -97,7 +174,8 @@ impl PomScope { /// (`.mvn/maven.config`'s `-D`, see [`maven_config_properties`]): they /// override a same-named pom property when a coordinate or a version is /// interpolated, as Maven's model interpolator looks them up before the - /// model's own. + /// pom's own. Profile `` are not read (see + /// [`Self::read_profiles`]). pub(crate) fn with_user_properties( raw: &str, user: &BTreeMap, @@ -115,33 +193,173 @@ impl PomScope { .concat(); let profiles = elements(&masked, "profiles")?; let properties = properties(&masked, &ignored, &profiles)?; - let mut build_properties = properties.clone(); - build_properties.extend( - user.iter() - .filter(|(k, _)| !is_model_reference(k)) - .map(|(k, v)| (k.clone(), v.clone())), - ); - let mut coordinate_refs = properties.clone(); - coordinate_refs.extend(user.iter().map(|(k, v)| (k.clone(), v.clone()))); - // A `project.*` / `pom.*` reference names the model, whatever a - // same-named property says; the deprecated bare form yields to one. - for (name, value) in model_refs(&masked, &coordinate_refs.clone())? { - if is_model_reference(&name) { - coordinate_refs.insert(name, value); - } else { - coordinate_refs.entry(name).or_insert(value); - } - } + let model = model_refs(&masked)?; Ok(PomScope { masked, ignored, profiles, properties, - build_properties, - coordinate_refs, + user: user.clone(), + model, + profile_properties: Vec::new(), + profiles_unreadable: None, }) } + /// Read every `` inside ``, which only + /// [`Self::resolve_in_build`] weighs. One this reader cannot parse + /// does not fail the pom: it is recorded, and every `${…}` resolution + /// then fails closed ([`MovedBy::Unreadable`]). + pub(crate) fn read_profiles(&mut self) { + match profile_properties(&self.masked, &self.ignored, &self.profiles) { + Ok(found) => self.profile_properties = found, + Err(e) => self.profiles_unreadable = Some(e), + } + } + + /// The indices into [`Self::profile_properties`] of the profile that + /// holds `pos` (none outside one). + fn own_profile(&self, pos: usize) -> Vec { + (0..self.profile_properties.len()) + .filter(|&i| { + self.profile_properties[i] + .holder + .is_some_and(|p| pos >= p.start && pos < p.end) + }) + .collect() + } + + /// A model reference's raw text (an empty element binds nothing). + fn model_ref(&self, name: &str) -> Option { + self.model.get(name).filter(|v| !v.is_empty()).cloned() + } + + /// What `${name}` reads ([`lookup_property`]) with the profiles `active` + /// (indices, in pom order) active, with or without the build's user + /// properties. + fn lookup(&self, name: &str, active: &[usize], user: bool) -> Option { + let layer = PomLayer { + own: &self.properties, + profiles: active + .iter() + .map(|&i| &self.profile_properties[i].props) + .collect(), + }; + let user = if user { + &self.user + } else { + &NO_USER_PROPERTIES + }; + lookup_property(name, &|n| self.model_ref(n), user, [layer]) + } + + /// Where `${name}` may be defined in the build, for + /// [`profile_combinations`]: the model or a user property fixes it; + /// otherwise the root's, every profile's (each but `own`, which is + /// always active, may be the one read) and a bare model reference. + fn definitions(&self, name: &str, own: &[usize]) -> Definitions { + if let Some(v) = is_model_reference(name) + .then(|| self.model_ref(name)) + .flatten() + .or_else(|| self.user.get(name).cloned()) + { + return Definitions { + texts: vec![v], + profiles: Vec::new(), + }; + } + let mut d = Definitions::default(); + d.texts.extend(self.properties.get(name).cloned()); + d.texts.extend(self.model_ref(name)); + for (i, p) in self.profile_properties.iter().enumerate() { + if let Some(v) = p.props.get(name) { + d.texts.push(v.clone()); + if !own.contains(&i) { + d.profiles.push(i); + } + } + } + d + } + + /// The `` text of the declaration at `pos`, resolved as + /// [`PomDep::version`], [`PomDep::build_version`] and + /// [`PomDep::version_alternatives`] describe. + fn resolve_version(&self, text: &str, pos: usize) -> (String, String, Vec) { + if !text.contains("${") { + return (text.to_string(), text.to_string(), Vec::new()); + } + let own = self.own_profile(pos); + let spelled = interpolate_strict(text, &|name| self.lookup(name, &own, false)) + .unwrap_or_else(|| text.to_string()); + match self.resolve_in_build(text, pos, interpolate_strict) { + (Some(build), alternatives) => (spelled, build, alternatives), + (None, _) => (spelled, text.to_string(), Vec::new()), + } + } + + /// What `text` (written in the declaration at `pos`) resolves to in the + /// build with no profile active but the one holding it, through + /// `interpolate` ([`interpolate_strict`] for a version), and each other + /// value a combination of active profiles gives it + /// ([`profile_combinations`], the vendored reactor's too), the + /// smallest combination first; a combination `interpolate` cannot + /// resolve gives the text as written. Fails closed: when the + /// combinations cannot be enumerated or a profile's `` is + /// unreadable, the text as written is an alternative. + pub(crate) fn resolve_in_build( + &self, + text: &str, + pos: usize, + interpolate: Interpolate, + ) -> (Option, Vec) { + let own = self.own_profile(pos); + let base = interpolate(text, &|name| self.lookup(name, &own, true)); + if !text.contains("${") { + return (base, Vec::new()); + } + let as_written = |by: MovedBy| Alternative { + value: text.to_string(), + by, + }; + if let Some(e) = &self.profiles_unreadable { + return (base, vec![as_written(MovedBy::Unreadable(e.clone()))]); + } + let Some(combinations) = profile_combinations(text, &|name| self.definitions(name, &own)) + else { + return (base, vec![as_written(MovedBy::TooManyProfiles)]); + }; + let mut alternatives: Vec = Vec::new(); + for combination in combinations { + let mut active: Vec = own.iter().chain(&combination).copied().collect(); + active.sort_unstable(); + active.dedup(); + let value = interpolate(text, &|name| self.lookup(name, &active, true)) + .unwrap_or_else(|| text.to_string()); + if Some(&value) != base.as_ref() && alternatives.iter().all(|a| a.value != value) { + let ids = combination + .iter() + .map(|&i| self.profile_properties[i].id.clone()) + .collect(); + alternatives.push(Alternative { + value, + by: MovedBy::Profiles(ids), + }); + } + } + (base, alternatives) + } + + /// `text` (a coordinate of the declaration at `pos`) with every + /// `${name}` the build defines interpolated, reading the declaration's + /// own profile `` first when it sits in a profile (as its + /// version is), an undefined one kept as written + /// ([`interpolate_lenient`]). + fn interpolate_coordinate(&self, text: &str, pos: usize) -> String { + let own = self.own_profile(pos); + interpolate_lenient(text, &|name| self.lookup(name, &own, true)) + } + /// Inside a plugin / deploy section (never project resolution). pub(crate) fn is_ignored(&self, pos: usize) -> bool { inside(pos, &self.ignored) @@ -185,10 +403,11 @@ fn inside(pos: usize, set: &[Element]) -> bool { #[derive(Debug)] pub(crate) struct ScopedDep { pub(crate) element: Element, - /// `` / `` with every `${prop}` the root - /// `` or the project's own coordinates define - /// interpolated (see [`interpolate`]); an - /// undefined one stays as written, so [`may_name`] can tell it apart. + /// `` / `` with every `${prop}` the build defines + /// with no profile active (user properties, the project model, the + /// root ``) interpolated + /// ([`PomScope::interpolate_coordinate`]); an undefined one stays as + /// written, so [`may_name`] can tell it apart. pub(crate) group: Option, pub(crate) artifact: Option, /// Inner-text byte range of the literal ``, when present. @@ -212,9 +431,9 @@ impl PomScope { out.push(ScopedDep { element: dep, group: child_text(&body, "groupId")? - .map(|g| interpolate(&g, &self.coordinate_refs)), + .map(|g| self.interpolate_coordinate(&g, dep.start)), artifact: child_text(&body, "artifactId")? - .map(|a| interpolate(&a, &self.coordinate_refs)), + .map(|a| self.interpolate_coordinate(&a, dep.start)), version_inner: version.map(|v| { ( dep.inner_start + v.inner_start, @@ -243,7 +462,8 @@ pub(crate) fn parse_pom(raw: &str) -> Result { /// [`parse_pom`] with the build's user properties (see /// [`PomScope::with_user_properties`]). pub(crate) fn parse_pom_with(raw: &str, user: &BTreeMap) -> Result { - let scope = PomScope::with_user_properties(raw, user)?; + let mut scope = PomScope::with_user_properties(raw, user)?; + scope.read_profiles(); let text = &scope.masked; let managed = elements(text, "dependencyManagement")?; let mut pom = Pom::default(); @@ -261,17 +481,20 @@ pub(crate) fn parse_pom_with(raw: &str, user: &BTreeMap) -> Resu let (Some(group), Some(artifact)) = (dep.group, dep.artifact) else { continue; }; + let (version, build_version, version_alternatives) = match &dep.version_text { + Some(text) => { + let (spelled, build, alternatives) = scope.resolve_version(text, dep.element.start); + (Some(spelled), Some(build), alternatives) + } + None => (None, None, Vec::new()), + }; pom.deps.push(PomDep { group, artifact, - version: dep - .version_text - .as_deref() - .map(|v| resolve_property(v, &scope.properties)), - build_version: dep - .version_text - .as_deref() - .map(|v| resolve_property(v, &scope.build_properties)), + version, + build_version, + version_text: dep.version_text, + version_alternatives, classifier: dep.classifier, type_text: dep.type_text, managed: inside(dep.element.start, &managed), @@ -298,100 +521,77 @@ fn properties( { continue; } - let body = props.inner(text); - let mut from = 0; - while let Some(rel) = body[from..].find('<') { - let lt = from + rel; - let Some(gt) = body[lt..].find('>').map(|r| lt + r) else { - return Err("unterminated tag in ".to_string()); - }; - let name = &body[lt + 1..gt]; - if !is_property_name(name) { - from = gt + 1; - continue; - } - let close = format!(""); - let Some(end) = body[gt + 1..].find(&close).map(|r| gt + 1 + r) else { - return Err(format!("unterminated <{name}> in ")); - }; - map.entry(name.to_string()) - .or_insert_with(|| body[gt + 1..end].trim().to_string()); - from = end + close.len(); - } + read_properties(props.inner(text), &mut map)?; } Ok(map) } -/// `${name}` → the property's value (one level, no recursion); anything -/// else, or an undefined property, is returned unchanged. -fn resolve_property(version: &str, properties: &BTreeMap) -> String { - version - .strip_prefix("${") - .and_then(|rest| rest.strip_suffix('}')) - .filter(|name| is_property_name(name)) - .and_then(|name| properties.get(name)) - .cloned() - .unwrap_or_else(|| version.to_string()) -} - -/// Interpolation rounds before a still-unresolved reference is given up on -/// (a property defined through another, or a cycle). -const MAX_INTERPOLATION: usize = 8; - -/// Longest text interpolation builds. A real coordinate is far shorter; a -/// pom whose properties each name the previous one many times would -/// otherwise grow exponentially with every round ("billion laughs"). -const MAX_INTERPOLATED_LEN: usize = 1024; - -/// `text` with every `${name}` the root `` or project model -/// defines replaced by its value, as Maven interpolates a coordinate such as -/// `org.${vendor}` (a value naming another property is resolved too, up to -/// [`MAX_INTERPOLATION`] rounds); an undefined or malformed reference is -/// kept as written, and a round that would exceed [`MAX_INTERPOLATED_LEN`] -/// is not taken, leaving its references unresolved (which fails closed). -fn interpolate(text: &str, properties: &BTreeMap) -> String { - let mut text = text.to_string(); - for _ in 0..MAX_INTERPOLATION { - let Some(next) = interpolate_once(&text, properties) else { - break; - }; - if next == text { - break; +/// The trimmed text of `body`'s own `` child (depth 0), never one +/// nested deeper (a ``'s `` inside a ``). +fn direct_child_text(body: &str, name: &str) -> Option { + let mut depth = 0usize; + let mut from = 0; + while let Some(rel) = body[from..].find('<') { + let lt = from + rel; + let gt = lt + body[lt..].find('>')?; + let tag = &body[lt + 1..gt]; + from = gt + 1; + if tag.starts_with('!') || tag.starts_with('?') || tag.ends_with('/') { + continue; } - text = next; + if tag.starts_with('/') { + depth = depth.saturating_sub(1); + continue; + } + let tag_name = tag.split(|c: char| c.is_ascii_whitespace()).next()?; + if depth == 0 && tag_name == name { + let end = from + body[from..].find(&format!(") -> Option { - let mut out = String::with_capacity(text.len()); - let mut rest = text; - while let Some(open) = rest.find("${") { - if out.len() > MAX_INTERPOLATED_LEN { - return None; - } - out.push_str(&rest[..open]); - let after = &rest[open + 2..]; - match after - .find('}') - .map(|close| (&after[..close], close)) - .filter(|(name, _)| is_property_name(name)) - .and_then(|(name, close)| properties.get(name).map(|v| (v, close))) - { - Some((value, close)) => { - out.push_str(value); - rest = &after[close + 1..]; - } - None => { - out.push_str("${"); - rest = after; - } +/// Each `` inside `` (outside ignored sections) as +/// `name -> trimmed value`, with the `` element that holds it. +fn profile_properties( + text: &str, + ignored: &[Element], + profiles: &[Element], +) -> Result, String> { + let live = |pos: usize| inside(pos, profiles) && !inside(pos, ignored); + // A `` no `` is found around (a scan this simple + // cannot place) belongs to no profile: it is then only ever an + // alternative, never a declaration's own, which reads more versions + // into [`PomDep::version_alternatives`], not fewer. + let holders: Vec = elements(text, "profile") + .unwrap_or_default() + .into_iter() + .filter(|p| live(p.start)) + .collect(); + let mut out = Vec::new(); + for props in elements(text, "properties")? { + if !live(props.start) { + continue; } + let holder = holders + .iter() + .find(|p| props.start >= p.start && props.start < p.end) + .copied(); + let id = holder + .and_then(|p| direct_child_text(p.inner(text), "id")) + .filter(|id| !id.is_empty()) + .unwrap_or_else(|| format!("#{}", out.len() + 1)); + let mut map = BTreeMap::new(); + read_properties(props.inner(text), &mut map)?; + out.push(ProfileProperties { + holder, + id, + props: map, + }); } - out.push_str(rest); - (out.len() <= MAX_INTERPOLATED_LEN).then_some(out) + Ok(out) } /// The model references a sibling declaration commonly uses @@ -399,13 +599,12 @@ fn interpolate_once(text: &str, properties: &BTreeMap) -> Option /// `${groupId}`, `${project.parent.groupId}`), read from the project's own /// `` / `` / `` and its ``'s; groupId /// and version fall back to the parent's, which the project inherits when -/// it omits them. A coordinate written through a property is resolved from -/// `properties` (never inherited instead: Maven interpolates a present -/// element), and one that stays unresolved is not bound. -fn model_refs( - masked: &str, - properties: &BTreeMap, -) -> Result, String> { +/// it omits them. Each is bound raw: one written through a property +/// (`${revision}`) is interpolated wherever it is read, +/// with the user properties and active profiles of that read, as Maven +/// interpolates the model itself (never inherited instead: Maven +/// interpolates a present element). +fn model_refs(masked: &str) -> Result, String> { let Some(project) = elements(masked, "project")?.into_iter().next() else { return Ok(BTreeMap::new()); }; @@ -430,15 +629,11 @@ fn model_refs( ] { own = blank_elements(&own, section)?; } - let usable = |v: Option| { - v.map(|v| interpolate(&v, properties)) - .filter(|v| !v.is_empty() && !v.contains("${")) - }; let mut refs = BTreeMap::new(); for (field, inherited) in [("groupId", true), ("artifactId", false), ("version", true)] { - let parent_value = usable(parent_text(field)?); + let parent_value = parent_text(field)?; let value = match child_text(&own, field)? { - Some(v) => usable(Some(v)), + Some(v) => Some(v), None if inherited => parent_value.clone(), None => None, }; @@ -587,12 +782,6 @@ pub(crate) fn is_executable_classifier(classifier: &str) -> bool { !NON_EXECUTABLE_CLASSIFIERS.contains(&classifier) } -/// A `project.*` / `pom.*` reference: Maven reads it from the model, ahead -/// of any user or pom property of the same name. -fn is_model_reference(name: &str) -> bool { - name.starts_with("project.") || name.starts_with("pom.") -} - fn is_property_name(name: &str) -> bool { !name.is_empty() && name @@ -626,6 +815,8 @@ fn blank_elements(text: &str, name: &str) -> Result { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use super::PomScope; #[test] @@ -689,7 +880,7 @@ mod tests { let deps = PomScope::new(&pom).unwrap().dependencies().unwrap(); let group = deps[0].group.as_deref().unwrap(); assert!( - group.len() <= super::MAX_INTERPOLATED_LEN, + group.len() <= super::properties::MAX_RESOLVED_LEN, "{}", group.len() ); @@ -772,7 +963,12 @@ mod tests { let scope = PomScope::new(pom).unwrap(); let deps = scope.dependencies().unwrap(); assert_eq!(deps[0].group.as_deref(), Some("com.acme")); - assert!(!scope.coordinate_refs.contains_key("project.version")); + let dep = super::parse_pom(pom).unwrap().deps.remove(0); + assert_eq!( + dep.build_version.as_deref(), + Some("${project.version}"), + "an unresolvable project version is not the parent's" + ); } /// Maven's model interpolator reads user properties (`.mvn/maven.config`'s @@ -841,6 +1037,246 @@ mod tests { } } + /// A `${property}` version is interpolated through every property it + /// names, user properties first at each level; a cycle, an undefined + /// name or a chain deeper than [`super::MAX_INTERPOLATION_DEPTH`] + /// leaves it as written. + #[test] + fn version_properties_resolve_recursively_and_fail_closed() { + // (as the pom spells it, as the build resolves it) + let both = |props: &str, user: &[(&str, &str)], text: &str| { + let pom = format!( + "{props}\ + ga\ + {text}" + ); + let user = user + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + let dep = super::parse_pom_with(&pom, &user).unwrap().deps.remove(0); + assert!(dep.version_alternatives.is_empty(), "{props}"); + (dep.version.unwrap(), dep.build_version.unwrap()) + }; + let version = |props: &str, user: &[(&str, &str)], text: &str| { + let (spelled, built) = both(props, user, text); + if user.is_empty() { + assert_eq!(spelled, built, "{props}"); + } + built + }; + assert_eq!( + both("${b}1.0", &[("b", "2.0")], "${a}"), + ("1.0".to_string(), "2.0".to_string()), + "a nested user property moves the build's version, not the pom's" + ); + // `${p1}` ... `1.0`. + let chain = |n: usize| -> String { + (0..n) + .map(|i| match i + 1 == n { + true => format!("1.0"), + false => format!("${{p{}}}", i + 1), + }) + .collect() + }; + assert_eq!( + version("${b}${c}1.0", &[], "${a}"), + "1.0" + ); + assert_eq!( + version("${b}1.0", &[("b", "2.0")], "${a}"), + "2.0" + ); + assert_eq!(version("10", &[], "${a}.${b}-x"), "1.0-x"); + assert_eq!( + version("", &[], "${project.version}"), + "${project.version}", + "no project version here" + ); + assert_eq!(version(&chain(16), &[], "${p0}"), "1.0"); + assert_eq!(version(&chain(17), &[], "${p0}"), "${p0}", "too deep"); + for cyclic in [ + "${a}", + "${b}${a}", + "x${b}${a}y", + ] { + assert_eq!(version(cyclic, &[], "${a}"), "${a}", "{cyclic}"); + } + assert_eq!(version("${b}", &[], "${a}"), "${a}", "undefined"); + assert_eq!(version("1", &[], "${a"), "${a", "unterminated"); + } + + /// The versions a profile's `` may move a declaration to: + /// every profile, and every set of profiles active together, counts; a + /// declaration inside a profile reads that profile's own first; user + /// properties and model references no profile can redefine. + #[test] + fn profile_properties_give_version_alternatives() { + fn values(alternatives: Vec) -> Vec { + alternatives.into_iter().map(|a| a.value).collect() + } + let resolve = |props: &str, profiles: &[&str], user: &[(&str, &str)], deps: &str| { + let profiles: String = profiles + .iter() + .map(|p| format!("{p}")) + .collect(); + let pom = format!( + "9{props}\ + {deps}{profiles}" + ); + let user = user + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + super::parse_pom_with(&pom, &user) + .unwrap() + .deps + .into_iter() + .map(|d| (d.build_version.unwrap(), values(d.version_alternatives))) + .collect::>() + }; + let dep = |v: &str| { + format!( + "ga\ + {v}" + ) + }; + let one = |props, profiles: &[&str], user: &[(&str, &str)]| { + resolve(props, profiles, user, &dep("${v}")).remove(0) + }; + let s = |v: &[&str]| v.iter().map(|v| v.to_string()).collect::>(); + assert_eq!(one("1", &["2"], &[]), ("1".into(), s(&["2"]))); + assert_eq!(one("1", &["1"], &[]), ("1".into(), s(&[]))); + assert_eq!(one("1", &["2"], &[]), ("1".into(), s(&[]))); + assert_eq!( + one("${w}1", &["2", "3"], &[]), + ("1".into(), s(&["2", "3"])) + ); + // Two profiles that move it only together. + assert_eq!( + one("11", &["${w}", "2"], &[]), + ("1".into(), s(&["2"])) + ); + // A profile value this pom cannot resolve, or a cycle. + assert_eq!( + one("1", &["${nope}"], &[]), + ("1".into(), s(&["${v}"])) + ); + assert_eq!( + one("1", &["${v}"], &[]), + ("1".into(), s(&["${v}"])) + ); + // User properties and model references win over every profile's. + assert_eq!( + one("1", &["2"], &[("v", "3")]), + ("3".into(), s(&[])) + ); + assert_eq!( + resolve( + "", + &["2"], + &[], + &dep("${project.version}") + ) + .remove(0), + ("9".into(), s(&[])) + ); + // More alternatives than are enumerated: unresolved. + let many: Vec = (0..17).map(|i| format!("{i}")).collect(); + let many: Vec<&str> = many.iter().map(String::as_str).collect(); + assert_eq!(one("x", &many, &[]), ("x".into(), s(&["${v}"]))); + + // A profile's own declaration reads its own properties first. + let pom = format!( + "1{}\ + a2\ + {}\ + b3", + dep("${v}"), + dep("${v}") + ); + let deps: Vec<_> = super::parse_pom(&pom) + .unwrap() + .deps + .into_iter() + .map(|d| { + ( + d.in_profile, + d.build_version.unwrap(), + d.version_alternatives, + ) + }) + .collect(); + let moved = |value: &str, ids: &[&str]| super::Alternative { + value: value.to_string(), + by: super::MovedBy::Profiles(s(ids)), + }; + assert_eq!( + deps, + [ + ( + false, + "1".to_string(), + vec![moved("2", &["a"]), moved("3", &["b"])] + ), + (true, "2".to_string(), vec![moved("3", &["b"])]), + ] + ); + + // `${project.version}` spelled through a property moves with it. + let pom = format!( + "${{revision}}1\ + {}ci\ + 2", + dep("${project.version}") + ); + let user = [("revision".to_string(), "3".to_string())].into(); + for (user, built, alternatives) in [ + (BTreeMap::new(), "1", vec![moved("2", &["ci"])]), + (user, "3", vec![]), + ] { + let d = super::parse_pom_with(&pom, &user).unwrap().deps.remove(0); + assert_eq!(d.version.as_deref(), Some("1")); + assert_eq!(d.build_version.as_deref(), Some(built)); + assert_eq!(d.version_alternatives, alternatives); + } + + // An unreadable profile `` fails a `${…}` closed, and + // only for the reader that weighs profiles. + let pom = format!( + "1{}{}\ + 2", + dep("${v}"), + dep("1") + ); + assert!(PomScope::new(&pom).unwrap().dependencies().is_ok()); + let deps = super::parse_pom(&pom).unwrap().deps; + assert!(matches!( + deps[0].version_alternatives[..], + [super::Alternative { + by: super::MovedBy::Unreadable(_), + .. + }] + )); + assert!(deps[1].version_alternatives.is_empty()); + } + + /// The shared interpolator's work bounds: a value naming the next + /// property many times over fails instead of growing exponentially. + #[test] + fn interpolation_is_bounded() { + let props: std::collections::BTreeMap = (0..12) + .map(|i| (format!("p{i}"), format!("${{p{}}}", i + 1).repeat(8))) + .chain([("p12".to_string(), "x".to_string())]) + .collect(); + let lookup = |name: &str| props.get(name).cloned(); + assert_eq!(super::interpolate_strict("${p0}", &lookup), None); + assert_eq!( + super::interpolate_strict("${p11}", &lookup).as_deref(), + Some("xxxxxxxx") + ); + } + #[test] fn suffix_grammar_is_exact() { assert_eq!( diff --git a/crates/socket-patch-core/src/formats/maven/properties.rs b/crates/socket-patch-core/src/formats/maven/properties.rs new file mode 100644 index 000000000..1f9cc3343 --- /dev/null +++ b/crates/socket-patch-core/src/formats/maven/properties.rs @@ -0,0 +1,386 @@ +//! Maven property interpolation, shared by VEX discovery ([`super::PomScope`]) +//! and the vendored reactor planner, so both read a `${…}` the same way: +//! Maven's lookup order ([`lookup_property`]), one bounded interpolator +//! ([`interpolate_strict`] / [`interpolate_lenient`]), and the profile +//! combinations that may change a value ([`profile_combinations`]). Pure. + +use std::collections::{BTreeMap, BTreeSet}; + +/// Nesting depth past which interpolation gives up: a property named +/// through more than this many others (Maven itself fails on a cycle, +/// which this bounds too). +pub(crate) const MAX_INTERPOLATION_DEPTH: usize = 16; + +/// Most lookups one interpolation makes before giving up: a value naming +/// the next property many times over would otherwise cost exponentially +/// many with every level ("billion laughs"). +const MAX_LOOKUPS: usize = 1024; + +/// Longest text one interpolation builds before giving up. +pub(crate) const MAX_RESOLVED_LEN: usize = 4096; + +/// Most profiles whose every combination [`profile_combinations`] lists +/// (2^8 - 1 combinations); a value more profiles may change is not +/// enumerated (the caller fails closed). +pub(crate) const MAX_COMBINED_PROFILES: usize = 8; + +/// A `project.*` / `pom.*` reference: Maven reads it from the model, ahead +/// of any user or pom property of the same name. +pub(crate) fn is_model_reference(name: &str) -> bool { + name.starts_with("project.") || name.starts_with("pom.") +} + +/// One pom's `` as Maven reads them with `profiles` active: +/// each active profile's merged over the pom's own, a later one (in pom +/// order) winning. +pub(crate) struct PomLayer<'a> { + pub(crate) own: &'a BTreeMap, + pub(crate) profiles: Vec<&'a BTreeMap>, +} + +impl PomLayer<'_> { + fn get(&self, name: &str) -> Option<&String> { + self.profiles + .iter() + .rev() + .find_map(|p| p.get(name)) + .or_else(|| self.own.get(name)) + } +} + +/// What `${name}` reads, in Maven's model interpolator's order: a +/// `project.*` / `pom.*` reference the model (`model`) answers, then the +/// build's user properties (`.mvn/maven.config`'s `-D`), then each pom's +/// properties with its active profiles merged in (`layers`, nearest pom +/// first — a child's beat its parent's), then a deprecated bare model +/// reference (`${version}`) `model` answers. The value is returned raw: +/// [`interpolate_strict`] resolves what it names in turn. +pub(crate) fn lookup_property<'a>( + name: &str, + model: &dyn Fn(&str) -> Option, + user: &BTreeMap, + layers: impl IntoIterator>, +) -> Option { + if is_model_reference(name) { + if let Some(v) = model(name) { + return Some(v); + } + } + if let Some(v) = user.get(name) { + return Some(v.clone()); + } + for layer in layers { + if let Some(v) = layer.get(name) { + return Some(v.clone()); + } + } + if is_model_reference(name) { + None + } else { + model(name) + } +} + +/// An interpolation of a text through a lookup: [`interpolate_strict`], +/// or a keep-unresolved one built on [`interpolate_lenient`]. +pub(crate) type Interpolate = fn(&str, &dyn Fn(&str) -> Option) -> Option; + +/// `value` with every `${name}` replaced by `lookup(name)`, itself +/// interpolated the same way, as Maven's model interpolator resolves a +/// property defined through another. `None` when a name is undefined, a +/// `${` is unterminated, a name is reached through itself (a cycle) or +/// through more than [`MAX_INTERPOLATION_DEPTH`] others, or the work +/// bounds are hit: an unresolved value never stands for a version. +pub(crate) fn interpolate_strict( + value: &str, + lookup: &dyn Fn(&str) -> Option, +) -> Option { + let lookups = std::cell::Cell::new(MAX_LOOKUPS); + expand(value, lookup, &mut Vec::new(), &lookups, false) +} + +/// [`interpolate_strict`] for a coordinate, where an unresolved reference +/// must stay visible: an undefined, cyclic or too deeply nested `${name}` +/// (or an unterminated `${`) is kept as written, so +/// [`may_name`](super::may_name) can treat it as unknown text; past the +/// work bounds the whole value is returned as written. +pub(crate) fn interpolate_lenient(value: &str, lookup: &dyn Fn(&str) -> Option) -> String { + let lookups = std::cell::Cell::new(MAX_LOOKUPS); + expand(value, lookup, &mut Vec::new(), &lookups, true).unwrap_or_else(|| value.to_string()) +} + +fn expand( + value: &str, + lookup: &dyn Fn(&str) -> Option, + stack: &mut Vec, + lookups: &std::cell::Cell, + keep: bool, +) -> Option { + let mut out = String::new(); + let mut rest = value; + while let Some(at) = rest.find("${") { + out.push_str(&rest[..at]); + let after = &rest[at + 2..]; + let Some(close) = after.find('}') else { + if !keep { + return None; + } + out.push_str(&rest[at..]); + rest = ""; + break; + }; + let name = &after[..close]; + let reference = &rest[at..at + close + 3]; + rest = &after[close + 1..]; + let resolved = if stack.len() >= MAX_INTERPOLATION_DEPTH || stack.iter().any(|n| n == name) + { + None + } else { + lookups.set(lookups.get().checked_sub(1)?); + match lookup(name) { + Some(raw) => { + stack.push(name.to_string()); + let v = expand(&raw, lookup, stack, lookups, keep); + stack.pop(); + Some(v?) + } + None => None, + } + }; + match resolved { + Some(v) => out.push_str(&v), + None if keep => out.push_str(reference), + None => return None, + } + if out.len() > MAX_RESOLVED_LEN { + return None; + } + } + out.push_str(rest); + (out.len() <= MAX_RESOLVED_LEN).then_some(out) +} + +/// The names `text` references (`${name}`), in order. +fn references(text: &str) -> impl Iterator { + let mut rest = text; + std::iter::from_fn(move || { + let at = rest.find("${")?; + let after = &rest[at + 2..]; + let close = after.find('}')?; + rest = &after[close + 1..]; + Some(&after[..close]) + }) +} + +/// Where `${name}` may be defined, for [`profile_combinations`]: every raw +/// text it may read under some set of active profiles (the model's, a +/// user property, a pom's, a profile's), and the profiles whose own +/// definition may be the one read (none when something Maven reads first, +/// a model reference or a user property, fixes it). +#[derive(Default)] +pub(crate) struct Definitions { + pub(crate) texts: Vec, + pub(crate) profiles: Vec, +} + +/// Every nonempty combination of the profiles that may change what +/// `value` interpolates to — those defining a name reachable from it +/// through any definition ([`Definitions`]) — smallest first, each sorted: +/// Maven activates any number of profiles together, so a value two +/// profiles move only jointly is caught. Empty when no profile can change +/// it. `None` when more than [`MAX_COMBINED_PROFILES`] may, or the walk +/// exceeds its bound: the caller fails closed. +pub(crate) fn profile_combinations( + value: &str, + defs: &dyn Fn(&str) -> Definitions, +) -> Option>> { + let mut seen: BTreeSet = BTreeSet::new(); + let mut queue: Vec = references(value).map(str::to_string).collect(); + let mut relevant: BTreeSet = BTreeSet::new(); + while let Some(name) = queue.pop() { + if !seen.insert(name.clone()) { + continue; + } + if seen.len() > MAX_LOOKUPS { + return None; + } + let d = defs(&name); + relevant.extend(d.profiles); + for text in &d.texts { + queue.extend(references(text).map(str::to_string)); + } + } + if relevant.len() > MAX_COMBINED_PROFILES { + return None; + } + let relevant: Vec = relevant.into_iter().collect(); + let mut masks: Vec = (1..(1u32 << relevant.len())).collect(); + masks.sort_by_key(|m| (m.count_ones(), *m)); + Some( + masks + .into_iter() + .map(|m| { + (0..relevant.len()) + .filter(|i| m & (1 << i) != 0) + .map(|i| relevant[i]) + .collect() + }) + .collect(), + ) +} + +/// The `name -> trimmed value` pairs of a `` body, into `map` +/// (a name's first definition wins). An element's name is its tag up to +/// whitespace or `/`, so `` and `` define +/// `v`, and `` defines it empty; a comment-blanked or malformed child +/// is an error (fail closed). +pub(crate) fn read_properties( + body: &str, + map: &mut BTreeMap, +) -> Result<(), String> { + let mut from = 0; + while let Some(rel) = body[from..].find('<') { + let lt = from + rel; + let Some(gt) = body[lt..].find('>').map(|r| lt + r) else { + return Err("unterminated tag in ".to_string()); + }; + let tag = &body[lt + 1..gt]; + let name_end = tag + .find(|c: char| c.is_ascii_whitespace() || c == '/') + .unwrap_or(tag.len()); + let name = &tag[..name_end]; + if !super::is_property_name(name) { + from = gt + 1; + continue; + } + // A later definition replaces an earlier one, as Maven's model + // reader's `Properties.put` does. + if tag.ends_with('/') { + map.insert(name.to_string(), String::new()); + from = gt + 1; + continue; + } + // ``, with optional whitespace before the `>`. + let close = format!(" in ")); + }; + let tail = &body[at + close.len()..]; + let ws = tail.len() - tail.trim_start().len(); + if tail[ws..].starts_with('>') { + break (at, at + close.len() + ws + 1); + } + search = at + close.len(); + }; + map.insert(name.to_string(), body[gt + 1..end].trim().to_string()); + from = after; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn map(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + + #[test] + fn strict_and_lenient_share_one_expansion() { + let props = map(&[("a", "${b}"), ("b", "1"), ("c", "${c}"), ("d", "x${e}")]); + let lookup = |n: &str| props.get(n).cloned(); + assert_eq!(interpolate_strict("v${a}", &lookup).as_deref(), Some("v1")); + assert_eq!(interpolate_strict("${c}", &lookup), None, "cycle"); + assert_eq!(interpolate_strict("${d}", &lookup), None, "undefined"); + assert_eq!(interpolate_strict("${a", &lookup), None, "unterminated"); + assert_eq!(interpolate_lenient("v${a}", &lookup), "v1"); + assert_eq!(interpolate_lenient("${c}-${a}", &lookup), "${c}-1"); + assert_eq!(interpolate_lenient("${d}", &lookup), "x${e}"); + assert_eq!(interpolate_lenient("${a", &lookup), "${a"); + } + + #[test] + fn lookup_follows_mavens_order() { + let model = |n: &str| match n { + "project.version" => Some("9".to_string()), + "version" => Some("bare".to_string()), + _ => None, + }; + let user = map(&[("u", "user"), ("project.version", "no"), ("p", "user")]); + let own = map(&[("p", "pom"), ("q", "pom"), ("version", "prop")]); + let first = map(&[("q", "first"), ("r", "first")]); + let second = map(&[("r", "second")]); + let layer = || PomLayer { + own: &own, + profiles: vec![&first, &second], + }; + let get = |n: &str| lookup_property(n, &model, &user, [layer()]); + assert_eq!(get("project.version").as_deref(), Some("9")); + assert_eq!(get("p").as_deref(), Some("user")); + assert_eq!(get("q").as_deref(), Some("first")); + assert_eq!(get("r").as_deref(), Some("second"), "a later profile wins"); + assert_eq!(get("version").as_deref(), Some("prop")); + let empty = BTreeMap::new(); + let bare = lookup_property("version", &model, &empty, []); + assert_eq!(bare.as_deref(), Some("bare")); + assert_eq!(get("project.other"), None); + } + + #[test] + fn combinations_cover_the_relevant_profiles_only() { + // v -> w; profile 0 defines v, profile 1 w, profile 2 only x. + let defs = |n: &str| match n { + "v" => Definitions { + texts: vec!["${w}".into(), "1".into()], + profiles: vec![0], + }, + "w" => Definitions { + texts: vec!["1".into()], + profiles: vec![1], + }, + _ => Definitions { + texts: vec![], + profiles: vec![2], + }, + }; + assert_eq!( + profile_combinations("${v}", &defs), + Some(vec![vec![0], vec![1], vec![0, 1]]) + ); + assert_eq!(profile_combinations("1.0", &defs), Some(vec![])); + let many = |_: &str| Definitions { + texts: vec![], + profiles: (0..=MAX_COMBINED_PROFILES).collect(), + }; + assert_eq!(profile_combinations("${v}", &many), None); + } + + #[test] + fn properties_read_attributed_and_spaced_tags() { + let mut m = BTreeMap::new(); + read_properties( + "123\ + 45", + &mut m, + ) + .unwrap(); + assert_eq!( + m, + map(&[ + ("a", "1"), + ("b", "2"), + ("c", "3"), + ("d", ""), + ("e", ""), + ("f", "5") + ]) + ); + assert!(read_properties("1", &mut BTreeMap::new()).is_err()); + } +} diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index 42a853fff..0b9239a69 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -11,7 +11,11 @@ use std::ops::Range; use serde_json::{json, Value}; -use crate::formats::maven::maven_config_properties as cli_properties; +use crate::formats::maven::{ + interpolate_strict, is_model_reference, lookup_property, + maven_config_properties as cli_properties, profile_combinations, Definitions, PomLayer, + MAX_COMBINED_PROFILES, MAX_INTERPOLATION_DEPTH, +}; use super::super::state::{WiringAction, WiringRecord}; use super::layout::{self, safe_coordinates}; @@ -51,8 +55,6 @@ const NESTED_MVN_FILES: &[&str] = &[ const SHAPE_UNSUPPORTED: &str = "vendor_jvm_shape_unsupported"; const UPSTREAM_UNAVAILABLE: &str = "vendor_jvm_upstream_unavailable"; const DEGRADED: &str = "vendor_jvm_degraded"; -/// Property interpolation depth cap (Maven itself fails on cycles). -const MAX_INTERPOLATION_DEPTH: usize = 16; /// Whether the pom declares a `` or `` of the project /// or of a profile (plugin configuration, comments and CDATA do not count). @@ -737,7 +739,7 @@ fn restore_versions( let resolve = |v: &str| { reactor .filter(|r| r.poms.contains_key(rel)) - .and_then(|r| r.interpolate(rel, v, 0)) + .and_then(|r| r.interpolate(rel, v)) }; let mut edits = Vec::new(); for (dep, key) in doc.keyed_declarations(c.group_id, c.artifact_id, &resolve) { @@ -1055,30 +1057,6 @@ struct ExternalPom { pom: Pom, } -/// `${name}` interpolation through `lookup`; `None` when a name is -/// undefined or the nesting is too deep. -fn interpolate_by( - value: &str, - lookup: &dyn Fn(&str) -> Option, - depth: usize, -) -> Option { - if depth > MAX_INTERPOLATION_DEPTH { - return None; - } - let mut out = String::new(); - let mut rest = value; - while let Some(at) = rest.find("${") { - out.push_str(&rest[..at]); - let after = &rest[at + 2..]; - let close = after.find('}')?; - let raw = lookup(&after[..close])?; - out.push_str(&interpolate_by(&raw, lookup, depth + 1)?); - rest = &after[close + 1..]; - } - out.push_str(rest); - Some(out) -} - impl Reactor { /// Management of `patch`'s g:a that a pin in local root `root` would /// override and that does not manage it at the patch's base version: @@ -1120,7 +1098,7 @@ impl Reactor { }) }; let resolve = |value: &str, what: &str| -> Result { - interpolate_by(value, &props, 0).ok_or_else(|| { + interpolate_strict(value, &props).ok_or_else(|| { Missing::Unresolved(format!("{rel}: {what} {value} is not defined")) }) }; @@ -1207,8 +1185,7 @@ impl Reactor { ) -> Result, Missing> { let mut chain: Vec = Vec::new(); let mut parent = self.poms[root].parent.as_ref().map(|p| { - let local = - |v: &Option| v.as_deref().and_then(|v| self.interpolate(root, v, 0)); + let local = |v: &Option| v.as_deref().and_then(|v| self.interpolate(root, v)); (local(&p.group), local(&p.artifact), local(&p.version)) }); while let Some((pg, pa, pv)) = parent { @@ -1374,7 +1351,7 @@ fn bom_manages<'b>( chain.iter().find_map(|e| e.pom.props.get(name).cloned()) }; let resolve = |value: &str| -> Result { - interpolate_by(value, &props, 0).ok_or_else(|| { + interpolate_strict(value, &props).ok_or_else(|| { Missing::Unresolved(format!( "the BOM {}:{}:{} manages {g}:{a} at {value}, which it does not define", bom.0, bom.1, bom.2 @@ -1729,89 +1706,70 @@ impl Reactor { fn keyed_declarations(&self, rel: &str, g: &str, a: &str) -> Vec<(usize, String)> { self.poms[rel] .doc - .keyed_declarations(g, a, &|v| self.interpolate(rel, v, 0)) + .keyed_declarations(g, a, &|v| self.interpolate(rel, v)) } /// Interpolate every `${name}` in `value` in the context of `rel`; /// `None` when any name is undefined in the checkout. - fn interpolate(&self, rel: &str, value: &str, depth: usize) -> Option { - self.interpolate_with(rel, value, depth, None) + fn interpolate(&self, rel: &str, value: &str) -> Option { + self.interpolate_with(rel, value, &[]) } - /// [`Self::interpolate`] with `overlay`'s properties (a profile's) - /// layered over its pom's own, as when that profile is active. - fn interpolate_with( - &self, - rel: &str, - value: &str, - depth: usize, - overlay: Option>, - ) -> Option { - if depth > MAX_INTERPOLATION_DEPTH { - return None; - } - let mut out = String::new(); - let mut rest = value; - while let Some(at) = rest.find("${") { - out.push_str(&rest[..at]); - let after = &rest[at + 2..]; - let close = after.find('}')?; - let raw = self.lookup_with(rel, &after[..close], overlay)?; - out.push_str(&self.interpolate_with(rel, &raw, depth + 1, overlay)?); - rest = &after[close + 1..]; - } - out.push_str(rest); - Some(out) + /// [`Self::interpolate`] with the `active` profiles' properties layered + /// over their poms' own, as when those profiles are active. + fn interpolate_with(&self, rel: &str, value: &str, active: &[Overlay<'_>]) -> Option { + interpolate_strict(value, &|name| self.lookup_with(rel, name, active)) } - /// User properties win over model properties, as in Maven; then the - /// model's own version expressions, then `` up the local - /// chain. + /// Maven's lookup order ([`lookup_property`], shared with VEX + /// discovery): the model's own coordinates, user properties, then + /// `` up the local chain with the `active` profiles merged + /// over their poms', then the deprecated bare `${version}`. fn lookup(&self, rel: &str, name: &str) -> Option { - self.lookup_with(rel, name, None) + self.lookup_with(rel, name, &[]) } - fn lookup_with(&self, rel: &str, name: &str, overlay: Option>) -> Option { + fn lookup_with(&self, rel: &str, name: &str, active: &[Overlay<'_>]) -> Option { + let layers = self.chain(rel).map(|p| PomLayer { + own: &self.poms[p].props, + profiles: active + .iter() + .filter(|(at, _)| *at == p) + .map(|(_, props)| *props) + .collect(), + }); + lookup_property(name, &|n| self.model_ref(rel, n), &self.cli_props, layers) + } + + /// The project model reference `name` of `rel` (`project.version`, + /// `project.groupId`, ...; the deprecated bare `version`), raw. + fn model_ref(&self, rel: &str, name: &str) -> Option { let pom = &self.poms[rel]; // A Maven 4 child may omit its parent's coordinates too: they come // from the inferred local parent. - let version = || { - self.chain(rel) - .find_map(|p| self.poms[p].effective_version()) - .map(str::to_string) - }; match name { - "project.version" | "pom.version" => return version(), - "project.groupId" | "pom.groupId" => { - return self - .chain(rel) - .find_map(|p| self.poms[p].effective_group()) - .map(str::to_string); - } - "project.artifactId" | "pom.artifactId" => return pom.artifact.clone(), - "project.parent.version" => return pom.parent.as_ref()?.version.clone(), - "project.parent.groupId" => return pom.parent.as_ref()?.group.clone(), - _ => {} - } - if let Some(v) = self.cli_props.get(name) { - return Some(v.clone()); - } - if let Some(v) = self.chain(rel).find_map(|p| { - overlay - .filter(|(at, _)| *at == p) - .and_then(|(_, props)| props.get(name)) - .or_else(|| self.poms[p].props.get(name)) - }) { - return Some(v.clone()); + "project.version" | "pom.version" | "version" => self + .chain(rel) + .find_map(|p| self.poms[p].effective_version()) + .map(str::to_string), + "project.groupId" | "pom.groupId" => self + .chain(rel) + .find_map(|p| self.poms[p].effective_group()) + .map(str::to_string), + "project.artifactId" | "pom.artifactId" => pom.artifact.clone(), + "project.parent.version" => pom.parent.as_ref()?.version.clone(), + "project.parent.groupId" => pom.parent.as_ref()?.group.clone(), + _ => None, } - (name == "version").then(version).flatten() } /// A version `declared` (with `${...}`) reaches in a pom that reads - /// it, an inheritor of `rel` or `rel` itself with a profile on its - /// chain active, that is neither the base nor `sv`: the version and - /// where. Profile activation is not evaluated, so every profile counts - /// (one at a time). + /// it, an inheritor of `rel` or `rel` itself with some combination of + /// the profiles on its chain active, that is neither the base nor `sv`: + /// the version and where. Profile activation is not evaluated, so every + /// combination of profiles that may change it counts + /// ([`profile_combinations`], shared with VEX discovery); more of them + /// than are combined fails closed. fn version_override( &self, rel: &str, @@ -1827,25 +1785,85 @@ impl Reactor { // has redefined something it names: that moves it too. let undefined = || "an undefined value".to_string(); let plain = (reader != rel) - .then(|| { - self.interpolate(reader, declared, 0) - .unwrap_or_else(undefined) - }) + .then(|| self.interpolate(reader, declared).unwrap_or_else(undefined)) .filter(|v| moved(v)) .map(|v| (v, reader.to_string())); - plain.or_else(|| { - self.chain(reader).find_map(|at| { - self.poms[at].profile_props.iter().find_map(|(id, props)| { - let v = self - .interpolate_with(reader, declared, 0, Some((at, props))) - .unwrap_or_else(undefined); - moved(&v).then(|| (v, format!("{reader} under profile {id} of {at}"))) - }) - }) - }) + plain.or_else(|| self.profile_override(reader, declared, &moved)) }) } + /// The first combination of the profiles on `reader`'s chain under + /// which `declared` resolves a version `moved` accepts, with the + /// version and where; fails closed (an undetermined version) when more + /// profiles may change it than are combined. + fn profile_override( + &self, + reader: &str, + declared: &str, + moved: &dyn Fn(&str) -> bool, + ) -> Option<(String, String)> { + let profiles: Vec<(&str, &str, &BTreeMap)> = self + .chain(reader) + .flat_map(|at| { + self.poms[at] + .profile_props + .iter() + .map(move |(id, props)| (at, id.as_str(), props)) + }) + .collect(); + let definitions = |name: &str| { + if let Some(v) = is_model_reference(name) + .then(|| self.model_ref(reader, name)) + .flatten() + .or_else(|| self.cli_props.get(name).cloned()) + { + return Definitions { + texts: vec![v], + profiles: Vec::new(), + }; + } + let mut d = Definitions::default(); + d.texts.extend( + self.chain(reader) + .filter_map(|p| self.poms[p].props.get(name).cloned()), + ); + d.texts.extend(self.model_ref(reader, name)); + for (i, (_, _, props)) in profiles.iter().enumerate() { + if let Some(v) = props.get(name) { + d.texts.push(v.clone()); + d.profiles.push(i); + } + } + d + }; + let Some(combinations) = profile_combinations(declared, &definitions) else { + return Some(( + "an undetermined value".to_string(), + format!( + "{reader} under some combination of the more than {MAX_COMBINED_PROFILES} \ + profiles on its chain that redefine what it names" + ), + )); + }; + combinations.into_iter().find_map(|combination| { + let active: Vec> = combination + .iter() + .map(|&i| (profiles[i].0, profiles[i].2)) + .collect(); + let v = self + .interpolate_with(reader, declared, &active) + .unwrap_or_else(|| "an undefined value".to_string()); + moved(&v).then(|| { + let under = combination + .iter() + .map(|&i| format!("profile {} of {}", profiles[i].1, profiles[i].0)) + .collect::>() + .join(" + "); + (v, format!("{reader} under {under}")) + }) + }) + } + /// Every pom in scope that inherits from `rel` (has it on its chain). fn inheritors<'s>(&'s self, rel: &'s str) -> impl Iterator + 's { self.scope @@ -1897,7 +1915,7 @@ impl Reactor { // literal SV must not reach. if doc.has_property_coordinate(dep) { let other = self.inheritors(rel).find(|p| { - let resolve = |v: &str| self.interpolate(p, v, 0); + let resolve = |v: &str| self.interpolate(p, v); doc.coordinate(dep, "groupId", &resolve).as_deref() != Some(patch.group_id) || doc.coordinate(dep, "artifactId", &resolve).as_deref() != Some(patch.artifact_id) @@ -1924,7 +1942,7 @@ impl Reactor { }; let declared = doc.text_of(version); let resolved = if declared.contains("${") { - match self.interpolate(rel, &declared, 0) { + match self.interpolate(rel, &declared) { Some(v) => v, None => { warnings.push(degraded( @@ -2012,7 +2030,7 @@ impl Reactor { uncertain: &mut BTreeSet, ) { let doc = &self.poms[rel].doc; - let resolve = |v: &str| self.interpolate(rel, v, 0); + let resolve = |v: &str| self.interpolate(rel, v); for dep in doc.declarations() { let (Some(g), Some(a)) = ( doc.child_text(dep, "groupId"), @@ -5833,6 +5851,42 @@ mod tests { } } + /// Profiles that move the version only when active together (one + /// points the property at another, the other redefines that one) are + /// weighed like a single one: VEX discovery and this planner share the + /// combinations (`formats::maven::profile_combinations`). + #[test] + fn profiles_that_move_the_version_only_together_are_weighed() { + let root = with_profile( + &ROOT.replace( + " ", + " 1.10.01.10.0\n \ + ", + ), + "indirect", + "${w}", + ); + let a = with_profile( + &module("a", &dep("${ct.version}")), + "raise", + "1.11.0", + ); + let files = fs(&[ + ("pom.xml", &root), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + let plan = run(&files).unwrap(); + assert_eq!(reasons(&plan), ["conflicting_literal_version"]); + let detail = &plan.warnings[0].detail; + assert!( + detail.contains("profile indirect") && detail.contains("profile raise"), + "{detail}" + ); + let after = applied(&files, &plan); + assert_eq!(text(&after, "a/pom.xml"), a); + } + /// #459: a module's profile overriding the property an inherited /// management declaration reads keeps that declaration. #[test] diff --git a/crates/socket-patch-core/src/vex/discover/maven.rs b/crates/socket-patch-core/src/vex/discover/maven.rs index eae4a6b6e..c94398d20 100644 --- a/crates/socket-patch-core/src/vex/discover/maven.rs +++ b/crates/socket-patch-core/src/vex/discover/maven.rs @@ -54,14 +54,18 @@ //! pin (a profile's `` only where management governs //! the pin): the rewriter leaves it as written, and whenever that profile //! is active it overrides the pin (vendored mode treats every profile as -//! live and rewrites it too). A `${property}` version is resolved one -//! level from the root `` — the pin as the pom spells it — and -//! again with `.mvn/maven.config`'s `-D` user properties first, as the -//! build resolves it (Maven's order; `${project.*}` stays the model's): a -//! pin the build config moves keeps the ref but marks it unattested, and a -//! Socket version only the build config supplies is diagnosed, never a -//! ref (hosted mode never writes one; rollback / remove never edit -//! `.mvn/maven.config`). +//! live and rewrites it too). A `${property}` version is interpolated +//! through every property it names (the vendored reactor's interpolator: +//! at most 16 deep, a cycle never resolves) from the root `` — +//! the pin as the pom spells it — and again with `.mvn/maven.config`'s +//! `-D` user properties first at every level, as the build resolves it +//! (Maven's order; `${project.*}` stays the model's): a pin the build +//! config moves keeps the ref but marks it unattested, and a Socket +//! version only the build config supplies is diagnosed, never a ref +//! (hosted mode never writes one; rollback / remove never edit +//! `.mvn/maven.config`). A pin the build resolves but that any profile's +//! (or set of profiles') `` make resolve anything else keeps +//! the ref unattested too, as a profile declaration does. //! //! Integrity: when the pom sha256 was known the rewriter also writes Maven //! Trusted Checksums — `.mvn/checksums/checksums.sha256` lines @@ -104,7 +108,8 @@ //! (plugin classpaths) are not project resolution and are skipped silently; //! `` are only read when activated, so a Socket repository or pin //! there is diagnosed, never a ref, and a profile declaration of a pinned -//! GA at another version withholds the pin's attestation. Submodule poms +//! GA at another version, or a profile's `` that move the +//! pin's `${property}`, withholds the pin's attestation. Submodule poms //! and parent poms are not read (root-only). Gradle builds are discovered //! by `gradle.rs` (the hosted index) and the vendored JVM backend's own //! probes. @@ -715,6 +720,18 @@ fn build_version(d: &PomDep) -> Option<&str> { d.build_version.as_deref() } +/// Every version the build may resolve for `d` (`None` when it has no +/// ``): [`build_version`], then each version a combination of +/// active profiles' `` moves it to +/// ([`PomDep::version_alternatives`]). +fn resolutions(d: &PomDep) -> impl Iterator> { + std::iter::once(build_version(d)).chain( + d.version_alternatives + .iter() + .map(|a| Some(a.value.as_str())), + ) +} + /// Every GA with a versioned, classifier-less declaration Maven always /// reads: the candidates for a pin. fn live_gas(deps: &[PomDep]) -> BTreeSet<(String, String)> { @@ -868,23 +885,34 @@ fn unpatched_classifier_copy( Wiring::Hosted => Some(pinned), Wiring::Vendored => None, }; - // Each entry carries whether it names `group:artifact` literally: one - // reached only through an undefined property ([`may_name`]) may manage - // another GA, so it never stands in for this copy's version alone. + // Each version a declaration may resolve, flagged when a profile gives + // it: the declaration's own profile, or one whose `` move + // it (every resolution after the first). + fn flagged(d: &PomDep, in_profile: bool) -> Vec<(Option<&str>, bool)> { + resolutions(d) + .enumerate() + .map(|(i, v)| (v, in_profile || i > 0)) + .collect() + } + // Each entry also carries whether it names `group:artifact` literally: + // one reached only through an undefined property ([`may_name`]) may + // manage another GA, so it never stands in for this copy's version alone. let managed = |classifier: &str, in_profile: bool| -> Vec<(Option<&str>, bool, bool)> { classifier_declarations(deps, group, artifact, move |d| { d.managed && d.in_profile == in_profile }) .filter(|(_, c)| *c == classifier) - .map(|(d, _)| { + .flat_map(|(d, _)| { let literal = d.group == group && d.artifact == artifact; - (build_version(d), in_profile, literal) + flagged(d, in_profile) + .into_iter() + .map(move |(v, p)| (v, p, literal)) }) .collect() }; classifier_declarations(deps, group, artifact, |d| !d.managed).find_map(|(d, classifier)| { let resolved: Vec<(Option<&str>, bool)> = match build_version(d) { - Some(version) => vec![(Some(version), false)], + Some(_) => flagged(d, false), None => { let mut live = managed(classifier, false); // Without a live entry that surely manages this GA, the @@ -939,7 +967,14 @@ const NON_CODE_TYPES: &[&str] = &["pom", "java-source", "javadoc"]; /// says. Activation is not evaluated, so every profile counts, as in /// vendored mode. A declaration without a version takes the live one and /// moves nothing, and a pom (or sources / javadoc) typed one carries no -/// code. +/// code. Every version a profile's `` may give the +/// declaration counts too ([`PomDep::version_alternatives`]). +/// +/// The same holds for a live declaration that resolves `pinned` through a +/// property some profile's `` redefine (or one it names, or +/// several profiles together) to another version: Maven merges an active +/// profile's properties over the pom's own before it interpolates. +/// Vendored mode refuses to pin over such a property (`version_override`). fn profile_override( deps: &[PomDep], group: &str, @@ -947,7 +982,7 @@ fn profile_override( pinned: &str, pin_managed: bool, ) -> Option { - main_jar_declarations(deps, group, artifact, |d| d.in_profile) + main_jar_declarations(deps, group, artifact, |_| true) .filter(|d| { !d.type_text .as_deref() @@ -955,7 +990,6 @@ fn profile_override( }) .filter(|d| !d.managed || pin_managed) .find_map(|d| { - let version = build_version(d).filter(|v| *v != pinned)?; let section = if d.managed { " " } else { @@ -970,6 +1004,29 @@ fn profile_override( d.group, d.artifact ) }; + if !d.in_profile { + // Off the pin as built, it is no pin (or another bypass). + if build_version(d) != Some(pinned) { + return None; + } + // The smallest combination of profiles that moves it. + let moved = d.version_alternatives.first()?; + let declared = d.version_text.as_deref().unwrap_or_default(); + let value = if !matches!(moved.by, crate::formats::maven::MovedBy::Profiles(_)) { + "resolve a version this reader cannot work out".to_string() + } else if moved.value.contains("${") { + format!("nothing it can resolve ({})", moved.value) + } else { + moved.value.clone() + }; + return Some(format!( + "{POM}: {section}{named} {declared} resolves the \ + patched {pinned}, but the of {} make it {value}; whenever that \ + applies Maven resolves it instead of the pin", + moved.by + )); + } + let version = resolutions(d).find_map(|v| v.filter(|v| *v != pinned))?; Some(format!( "{POM}: a {section}declaration of {named} carries \ {version}, not the patched {pinned}; whenever that profile \ @@ -1980,6 +2037,225 @@ mod tests { } } + /// Whether `out` attests `purl`: a ref no unattested entry withholds. + fn attests(out: &Discovery, purl: &str) -> bool { + out.refs.iter().any(|r| { + r.purl == purl + && !out + .unattested + .iter() + .any(|u| u.purl == r.purl && u.uuid == r.uuid) + }) + } + + /// A hosted project pinning slf4j-api through `decl` (a + /// `` / `` block), with the root + /// `` `props` and one `` per entry of `profiles` + /// (each a profile body). + fn property_project(props: &str, decl: &str, profiles: &[&str]) -> Project { + let repo = hosted_repo(&format!("socket-patch-{UUID_A}"), ®istry_url(UUID_A)); + let profiles: String = profiles + .iter() + .enumerate() + .map(|(i, body)| format!("p{i}\n{body}\n")) + .collect(); + let p = Project::new(); + p.write( + "pom.xml", + pom(&format!( + "{props}\n{decl}\n{repo}\n\ + \n{profiles}\n" + )), + ); + p + } + + fn direct_decl(version: &str) -> String { + format!( + "\n{}\n", + dep("org.slf4j", "slf4j-api", Some(version)) + ) + } + + /// A pin spelled through a property is what Maven resolves only while + /// no active profile's `` redefines that property (or one + /// it names) to anything else: Maven merges an active profile's + /// properties over the pom's own before it interpolates the + /// ``. Vendored mode refuses to pin over such a property + /// (`version_override`), so the ref stays (rollback / remove / list + /// find it) but is never attested. User properties + /// (`.mvn/maven.config`) win over every profile's. + #[tokio::test] + async fn profile_properties_redefining_a_pin_property_are_not_attested() { + let pin = suffixed(UUID_A); + let v = |value: &str| format!("{value}"); + let w = |value: &str| format!("{value}"); + let in_props = |s: String| format!("{s}"); + let direct = direct_decl("${slf4j.version}"); + let managed = format!( + "\n{}\n\ + \n{}\n", + dep("org.slf4j", "slf4j-api", Some("${slf4j.version}")), + dep("org.slf4j", "slf4j-api", None), + ); + let withheld = [ + // The base release. + (v(&pin), &direct, vec![in_props(v("1.7.36"))]), + (v(&pin), &managed, vec![in_props(v("1.7.36"))]), + // Through a property the pin's property names. + (v("${w}") + &w(&pin), &direct, vec![in_props(w("1.7.30"))]), + // A value this pom cannot resolve, or a cycle. + (v(&pin), &direct, vec![in_props(v("${undefined}"))]), + (v(&pin), &direct, vec![in_props(v("${slf4j.version}"))]), + // Two profiles that move the pin only together. + ( + v(&pin) + &w(&pin), + &direct, + vec![in_props(v("${w}")), in_props(w("1.7.36"))], + ), + ]; + for (root, decl, profiles) in &withheld { + let profiles: Vec<&str> = profiles.iter().map(String::as_str).collect(); + let out = run(&property_project(root, decl, &profiles)).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert_eq!( + out.unattested.len(), + 1, + "{root} {profiles:?}: {:?}", + out.unattested + ); + let u = &out.unattested[0]; + assert_eq!( + u.kind, + UnattestedKind::MavenProfileUnpatched, + "{profiles:?}" + ); + assert!( + u.detail.contains("org.slf4j:slf4j-api") && u.detail.contains(""), + "{}", + u.detail + ); + } + + for (root, profile, config) in [ + // The pin itself, or an unrelated property. + (v(&pin), in_props(v(&pin)), None), + (v(&pin), in_props("1".to_string()), None), + (v("${w}") + &w(&pin), in_props(w(&pin)), None), + // A user property wins over every profile's. + ( + v(&pin), + in_props(v("1.7.36")), + Some(format!("-Dslf4j.version={pin}\n")), + ), + ] { + let p = property_project(&root, &direct, &[&profile]); + if let Some(config) = &config { + p.write(".mvn/maven.config", config); + } + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert!( + attests(&out, FX_PURL), + "{root} {profile}: {:?}", + out.unattested + ); + } + } + + /// A `` declaration's `${property}` version reads its own + /// profile's `` first: that profile is active whenever the + /// declaration applies. + #[tokio::test] + async fn profile_declaration_reads_its_own_profile_properties() { + let pin = suffixed(UUID_A); + let profile = |value: &str| { + format!( + "{value}\n\ + \n{}\n", + dep("org.slf4j", "slf4j-api", Some("${slf4j.version}")) + ) + }; + let direct = direct_decl(&pin); + for (root, own, attested) in [ + // The root resolves the pin, the profile the base release. + (pin.as_str(), "1.7.36", false), + // The root resolves the base release, the profile the pin. + ("1.7.36", pin.as_str(), true), + ] { + let p = property_project( + &format!("{root}"), + &direct, + &[&profile(own)], + ); + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert_eq!( + attests(&out, FX_PURL), + attested, + "{own}: {:?}", + out.unattested + ); + } + } + + /// A `${property}` version resolves through every property it names, + /// with user properties first at each level, as Maven interpolates; a + /// cycle, or a chain deeper than the vendored reactor follows, is + /// never a pin. + #[tokio::test] + async fn version_property_resolves_through_nested_properties() { + let pin = suffixed(UUID_A); + // `${p1}` ... `{end}`. + let chain = |links: usize, end: &str| -> String { + (0..links) + .map(|i| { + let value = if i + 1 == links { + end.to_string() + } else { + format!("${{p{}}}", i + 1) + }; + format!("{value}") + }) + .collect() + }; + let split = format!( + "${{p1}}-socket.${{p2}}1.7.36{}", + &UUID_A[..8] + ); + let user_pin = format!("-Dp1={pin}\n"); + for (props, config, attested) in [ + (chain(2, &pin), None, true), + (chain(3, &pin), None, true), + (split, None, true), + // The deepest chain the vendored reactor resolves, and one more. + (chain(16, &pin), None, true), + (chain(17, &pin), None, false), + // A user property redefines a nested one: off the pin, or onto + // a Socket version only the build config supplies (never a + // pom.xml ref). + (chain(2, &pin), Some("-Dp1=1.7.36\n"), false), + (chain(2, "1.7.36"), Some(user_pin.as_str()), false), + // A cycle. + ("${p1}${p0}".to_string(), None, false), + ("${p0}".to_string(), None, false), + ] { + let p = property_project(&props, &direct_decl("${p0}"), &[]); + if let Some(config) = config { + p.write(".mvn/maven.config", config); + } + let out = run(&p).await; + assert_eq!( + attests(&out, FX_PURL), + attested, + "{props} {config:?}: {:?} {:?} {:?}", + out.refs, + out.unattested, + out.diagnostics + ); + } + } + /// A versioned declaration whose coordinate is a property this pom /// does not define may be the pinned GA, so the pin is not attested /// past it (#683). @@ -2249,6 +2525,254 @@ mod tests { } } + /// The same for a profile's `` that move a `${property}` + /// version of the vendored GA off the vendored version. + #[tokio::test] + async fn vendored_ref_under_profile_properties_is_not_attested() { + for (version, attested) in [("1.10.1", false), ("1.10.0", true)] { + let p = vendored_project(UUID_A); + let wired = std::fs::read_to_string(p.root().join("pom.xml")) + .expect("read pom") + .replace("1.10.0", "${ct}"); + let at = wired.rfind("").expect("project close"); + p.write( + "pom.xml", + format!( + "{}1.10.0\np\ + {version}\n{}", + &wired[..at], + &wired[at..] + ), + ); + let out = run(&p).await; + assert_refs(&out, &[(V_PURL, UUID_A, WiringMode::Vendored)]); + assert_eq!( + attests(&out, V_PURL), + attested, + "{version}: {:?}", + out.unattested + ); + } + } + + /// A `${project.version}` is the project's own ``, which may + /// itself be spelled through a property (`${revision}`, Maven's + /// CI-friendly versions): a profile or a `-D` user property that + /// redefines that property moves the model reference too. + #[tokio::test] + async fn model_references_move_with_the_properties_they_are_spelled_through() { + let pin = suffixed(UUID_A); + let repo = hosted_repo(&format!("socket-patch-{UUID_A}"), ®istry_url(UUID_A)); + let project = |revision: &str, profile: &str| { + let p = Project::new(); + p.write( + "pom.xml", + format!( + "\n\n4.0.0\n\ + com.exampleapp\ + ${{revision}}\n\ + {revision}\n\ + \n{}\n\n{repo}\n\ + ci{profile}\n\n", + dep("org.slf4j", "slf4j-api", Some("${project.version}")), + ), + ); + p + }; + // A profile that redefines what the project version is spelled + // through moves the pin. + let out = run(&project( + &pin, + "1.7.36", + )) + .await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert_eq!(out.unattested.len(), 1, "{:?}", out.unattested); + assert_eq!( + out.unattested[0].kind, + UnattestedKind::MavenProfileUnpatched + ); + // One that keeps it does not. + let out = run(&project( + &pin, + &format!("{pin}"), + )) + .await; + assert!(attests(&out, FX_PURL), "{:?}", out.unattested); + // A Socket version only `-Drevision=` supplies is no pom.xml ref. + let p = project("1.7.36", ""); + p.write(".mvn/maven.config", format!("-Drevision={pin}\n")); + let out = run(&p).await; + assert!(!attests(&out, FX_PURL), "{:?}", out.refs); + assert!( + out.diagnostics.iter().any( + |d| d.code == DIAG_REF_UNATTRIBUTABLE && d.detail.contains(".mvn/maven.config") + ), + "{:?}", + out.diagnostics + ); + } + + /// Review on #2020 (round 2): a property defined twice is Maven's LAST + /// definition; a profile declaration's coordinates read its own + /// profile's `` as its version does; a profile is named by + /// its own ``, not one nested in it. + #[tokio::test] + async fn duplicate_properties_profile_coordinates_and_profile_ids() { + let pin = suffixed(UUID_A); + let p = property_project( + &format!("{pin}1.7.36"), + &direct_decl("${slf4j.version}"), + &[], + ); + let out = run(&p).await; + assert!(!attests(&out, FX_PURL), "{:?}", out.refs); + + let p = property_project( + "org.other", + &direct_decl(&pin), + &[ + "org.slf4j\n\n\ + ${g}slf4j-api\ + 1.7.30\n\n", + ], + ); + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert!(!attests(&out, FX_PURL), "{:?}", out.unattested); + + let repo = hosted_repo(&format!("socket-patch-{UUID_A}"), ®istry_url(UUID_A)); + let p = Project::new(); + p.write( + "pom.xml", + pom(&format!( + "{pin}\n{}\n{repo}\n\ + mirror\ + https://example.test/m2\ + ci1.7.30\n", + direct_decl("${v}"), + )), + ); + let out = run(&p).await; + assert_eq!(out.unattested.len(), 1, "{:?}", out.unattested); + let detail = &out.unattested[0].detail; + assert!( + detail.contains("profile ci") && !detail.contains("mirror"), + "{detail}" + ); + } + + /// A profile property written with attributes (`combine.self`) or + /// whitespace in its tag is still a definition Maven reads. + #[tokio::test] + async fn profile_properties_with_attributes_are_read() { + let pin = suffixed(UUID_A); + for profile in [ + "1.7.36", + "1.7.36", + "\n1.7.36", + ] { + let p = property_project( + &format!("{pin}"), + &direct_decl("${slf4j.version}"), + &[profile], + ); + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert!(!attests(&out, FX_PURL), "{profile}: {:?}", out.unattested); + } + } + + /// A nested `${…}` in a profile's value reads that profile's own + /// properties first, as Maven merges the whole profile before it + /// interpolates: a profile that keeps the pin through its own + /// property does not withhold it. + #[tokio::test] + async fn profile_values_resolve_through_their_own_profile() { + let pin = suffixed(UUID_A); + let p = property_project( + &format!("{pin}1.7.36"), + &direct_decl("${slf4j.version}"), + &[&format!( + "${{w}}{pin}" + )], + ); + let out = run(&p).await; + assert!(attests(&out, FX_PURL), "{:?}", out.unattested); + } + + /// A profile `` this reader cannot parse withholds a + /// `${property}` pin (fail closed) instead of failing the whole pom; a + /// literal pin, which no property moves, is still attested. + #[tokio::test] + async fn unreadable_profile_properties_withhold_only_property_pins() { + let pin = suffixed(UUID_A); + let broken = "1.7.36"; + let p = property_project( + &format!("{pin}"), + &direct_decl("${slf4j.version}"), + &[broken], + ); + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert_eq!(out.unattested.len(), 1, "{:?}", out.unattested); + assert_eq!( + out.unattested[0].kind, + UnattestedKind::MavenProfileUnpatched + ); + let p = property_project("", &direct_decl(&pin), &[broken]); + let out = run(&p).await; + assert!( + attests(&out, FX_PURL), + "{:?} {:?}", + out.unattested, + out.diagnostics + ); + } + + /// Profiles that each keep the pin, however many and however deep the + /// chain they spell it through, never withhold it: every combination + /// is checked, not a capped cross product. + #[tokio::test] + async fn many_profiles_that_keep_the_pin_are_attested() { + let pin = suffixed(UUID_A); + // `${p1}` ... `{pin}`. + let chain: String = (0..5) + .map(|i| match i { + 4 => format!("{pin}"), + i => format!("${{p{}}}", i + 1), + }) + .collect(); + let profile = format!("{chain}"); + let profiles = [profile.as_str(); 5]; + let p = property_project(&chain, &direct_decl("${p0}"), &profiles); + let out = run(&p).await; + assert_refs(&out, &[(FX_PURL, UUID_A, WiringMode::Hosted)]); + assert!(attests(&out, FX_PURL), "{:?}", out.unattested); + } + + /// The withheld statement names the profiles that move the pin and + /// the version they move it to. + #[tokio::test] + async fn profile_property_detail_names_the_profile_and_version() { + let pin = suffixed(UUID_A); + let p = property_project( + &format!("{pin}"), + &direct_decl("${slf4j.version}"), + &[ + "1", + "1.7.30", + ], + ); + let out = run(&p).await; + assert_eq!(out.unattested.len(), 1, "{:?}", out.unattested); + let detail = &out.unattested[0].detail; + assert!( + detail.contains("profile p1") && detail.contains("1.7.30") && !detail.contains("p0"), + "{detail}" + ); + } + /// Review on #2005 (round 2): a `-D` property `.mvn/maven.config` alone /// defines AT the vendored version is the pin, not an override. #[tokio::test] diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 2f931a87e..cb8b3179f 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -609,8 +609,11 @@ pub enum UnattestedKind { /// A Maven pin (hosted, or the pre-v5 single-pom vendored repository) /// whose GA a `` declaration of the root pom gives another /// version (the base release, another release, an - /// unresolved `${property}`): whenever that profile is active it - /// overrides the pin, so the build resolves the unpatched jar. + /// unresolved `${property}`), or whose `${property}` version a + /// profile's `` redefine (directly or through a property + /// it names) to another or an unresolvable version: whenever that + /// profile is active it overrides the pin, so the build resolves the + /// unpatched jar. MavenProfileUnpatched, /// A hosted (or pre-v5 vendored) Maven pin the root pom spells through a /// `${property}` that