[
https://issues.apache.org/jira/browse/GROOVY-12461?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12461:
-------------------------------
Affects Version/s: 6.0.0
> @GrabConfig(systemProperties = ...) sets JVM system properties at runtime
> even when Grape is disabled or unavailable
> --------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12461
> URL: https://issues.apache.org/jira/browse/GROOVY-12461
> Project: Groovy
> Issue Type: Improvement
> Affects Versions: 6.0.0
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> h2. Problem
> When a class or script uses {{@Grab}} together with
> {{@GrabConfig(systemProperties = ...)}}, {{GrabAnnotationTransformation}}
> (line 490 in 6.0.0) adds a static initializer that does two things in order:
> # sets each listed system property, guarded by
> {{System.getProperty("groovy.grape.enable", "true") == "true"}};
> # calls {{Grape.grab(...)}}.
> The {{@GrabConfig}} javadoc says the properties "must be set before invoking
> the grab", for example SSL or proxy settings. Their only purpose is to
> support the grab. But the guard looks at the system property alone. It
> ignores the other two ways Grape can be off:
> * *No engine on the classpath.* In Groovy 6, Grape engines live in
> {{groovy-grape-ivy}} and {{groovy-grape-maven}}. With neither present,
> {{Grape.getInstance()}} returns null, {{Grape.grab}} does nothing, and the
> log says "Grapes disabled". The properties are still set.
> * *{{Grape.setEnableGrapes(false)}}.* {{Grape.grab}} checks the static
> {{enableGrapes}} flag and does nothing. The generated guard never looks at
> that flag, so the properties are still set.
> In both cases the class changes process-wide state, possibly trust-store or
> proxy settings, for a grab that never happens. Hosts that turn Grape off,
> such as embedders, tooling and notebook kernels, reasonably expect
> {{@GrabConfig}} to be inert too.
> A smaller inconsistency: {{Grape}} reads the property with
> {{Boolean.parseBoolean}}, which ignores case, while the generated guard
> compares it to the exact string "true". So {{-Dgroovy.grape.enable=TRUE}}
> turns grabbing on but skips the properties.
> h2. Reproducer
> {code:groovy}
> // G2.groovy
> @GrabConfig(systemProperties = 'probe.flag=set-by-grabconfig')
> @Grab('org.apache.commons:commons-lang3:3.14.0')
> import java.util.List
> println "flag=" + System.getProperty('probe.flag')
> {code}
> Results against Groovy 6.0.0 (JDK 17):
> ||Setup||Grab runs?||{{probe.flag}}||
> |core jar only ({{java -cp groovy-6.0.0.jar groovy.ui.GroovyMain
> G2.groovy}})|no ("Grapes disabled")|*set-by-grabconfig*|
> |engines present, {{Grape.enableGrapes = false}} before evaluating with
> {{GroovyShell}}|no|*set-by-grabconfig*|
> |core jar only, {{-Dgroovy.grape.enable=false}}|no|null|
> |engines present, {{GrabAnnotationTransformation}} disabled via
> {{CompilerConfiguration.disabledGlobalASTTransformations}}|no|null|
> |engines present, defaults|yes|set-by-grabconfig (expected)|
> When {{@Grab}} is on a class rather than a script, the properties are set
> when that class initializes, not when the script starts.
> h2. Proposed fix
> Set the properties only when a grab will actually run. The simplest form is
> to add a small public helper to {{Grape}}, for example
> {{Grape.isGrabAvailable()}}, returning {{enableGrapes && getInstance() !=
> null}}. The generated guard then calls the helper instead of reading the
> system property. This also removes the case mismatch, because the helper uses
> the same parsed flag as {{Grape.grab}}.
> An alternative is to pass the properties to {{Grape.grab}} in its arguments
> map, which the compile-time path already does but no engine reads. The facade
> would then apply them just before delegating to the engine. That changes
> compile-time behaviour, because the properties would also be set in the
> compiler's JVM. That should be a separate decision, so the minimal fix above
> is preferred for this ticket.
> h2. Compatibility
> Behaviour changes only when Grape is disabled or no engine is available. That
> is exactly when the properties had no effect on any grab. Code that relied on
> {{@GrabConfig}} as a general way to set system properties, without a working
> grab, would need to call {{System.setProperty}} itself. That seems unlikely,
> and it could be noted in the release notes.
> h2. Tests
> * With no engine on the test classpath, a class using
> {{@GrabConfig(systemProperties)}} leaves the property unset. Run it in a
> forked JVM, or in {{groovy-grape-test}} with a classloader that hides the
> engines.
> * After {{Grape.setEnableGrapes(false)}}, the same class leaves the property
> unset. Restore the flag afterwards.
> * With an engine present and grapes enabled, behaviour is unchanged.
> h2. Context
> Found while drafting GEP-32 (restricted compiler profile). The profile turns
> off the Grab transform for each compilation, which avoids this. Hosts that
> rely on keeping the engines off the classpath, or on
> {{Grape.setEnableGrapes(false)}}, do not get that protection.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
