gnodet-bot commented on code in PR #13452:
URL: https://github.com/apache/maven/pull/13452#discussion_r4264956540
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileActivationContext.java:
##########
@@ -418,9 +411,15 @@ public String getUserProperty(String key) {
return null;
}
+ /**
+ * Model properties are suppressed: a dependency POM's own {@code
<properties>} must not
+ * activate its own profiles. Both {@link
org.apache.maven.impl.model.profile.PropertyProfileActivator} and
+ * {@link
org.apache.maven.impl.model.profile.ConditionProfileActivator} call
+ * {@code getModelProperty()} directly, so returning {@code null}
here closes the gap.
Review Comment:
💡 **Javadoc inaccuracy:** Same as above — `PropertyProfileActivator` does
not call `getModelProperty()`. Only `ConditionProfileActivator` does.
```suggestion
/**
* Model properties are suppressed: a dependency POM's own
{@code <properties>} must not
* activate its own profiles. {@link
org.apache.maven.impl.model.profile.ConditionProfileActivator}
* calls {@code getModelProperty()} directly, so returning
{@code null} here closes the gap.
*/
```
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileActivationContext.java:
##########
@@ -369,14 +369,16 @@ public DefaultProfileActivationContext setModel(Model
model) {
* <p>
* The sandboxed context:
* <ul>
- * <li><b>Merges model properties into system property lookups</b>:
system properties
- * ({@code java.version}, {@code os.name}, …) are checked first
(preserving JDK/OS
- * activation), then model properties (the POM's own {@code
<properties>}) are used
- * as a fallback. This makes POM-declared properties visible to the
- * {@code PropertyProfileActivator} without modifying the
activator's lookup chain
- * for non-external models.</li>
+ * <li><b>Preserves system properties</b> ({@code java.version}, {@code
os.name}, …)
+ * so JDK/OS activation works.</li>
* <li><b>Suppresses user properties</b> (consumer {@code -D} flags):
they were not
* set for the dependency and must not accidentally activate its
profiles.</li>
+ * <li><b>Suppresses model properties</b>: a dependency POM's own
+ * {@code <properties>} must not activate its own property-based or
+ * condition-based profiles (see #13390). Both {@link
org.apache.maven.impl.model.profile.PropertyProfileActivator}
+ * and {@link
org.apache.maven.impl.model.profile.ConditionProfileActivator}
+ * read model properties directly via {@code getModelProperty()}, so
the
+ * sandbox overrides that method to return {@code null}.</li>
Review Comment:
💡 **Javadoc inaccuracy:** `PropertyProfileActivator` does not call
`getModelProperty()` — it only checks `getUserProperty()` and
`getSystemProperty()`. Only `ConditionProfileActivator` calls
`getModelProperty()` directly (via `doGetProperty()`).
```suggestion
* <li><b>Suppresses model properties</b>: a dependency POM's own
* {@code <properties>} must not activate its own property-based or
* condition-based profiles (see #13390).
* {@link
org.apache.maven.impl.model.profile.ConditionProfileActivator}
* reads model properties directly via {@code getModelProperty()},
so the
* sandbox overrides that method to return {@code null}.</li>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
