ProxySetting FORCED now unsets settings configured outside Gradle #153

node-gradle · Aug 15, 2023 · 0d3f3c9 · 0d3f3c9
1 parent e9e2d96
commit 0d3f3c9
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 36 deletions.
diff --git a/src/main/kotlin/com/github/gradle/node/npm/exec/NpmExecRunner.kt b/src/main/kotlin/com/github/gradle/node/npm/exec/NpmExecRunner.kt
@@ -5,7 +5,6 @@ import com.github.gradle.node.exec.ExecConfiguration
 import com.github.gradle.node.exec.ExecRunner
 import com.github.gradle.node.exec.NodeExecConfiguration
 import com.github.gradle.node.npm.proxy.NpmProxy
-import com.github.gradle.node.npm.proxy.NpmProxy.Companion.computeNpmProxyEnvironmentVariables
 import com.github.gradle.node.util.ProjectApiHelper
 import com.github.gradle.node.util.zip
 import com.github.gradle.node.variant.VariantComputer
@@ -24,24 +23,11 @@ abstract class NpmExecRunner {
     fun executeNpmCommand(project: ProjectApiHelper, extension: NodeExtension, nodeExecConfiguration: NodeExecConfiguration, variants: VariantComputer): ExecResult {
         val npmExecConfiguration = NpmExecConfiguration("npm"
         ) { variantComputer, nodeExtension, npmBinDir -> variantComputer.computeNpmExec(nodeExtension, npmBinDir) }
-        return executeCommand(project, extension, addProxyEnvironmentVariables(extension, nodeExecConfiguration),
+        return executeCommand(project, extension, NpmProxy.addProxyEnvironmentVariables(extension.nodeProxySettings.get(), nodeExecConfiguration),
                 npmExecConfiguration,
             variants)
     }
 
-    private fun addProxyEnvironmentVariables(nodeExtension: NodeExtension,
-                                             nodeExecConfiguration: NodeExecConfiguration): NodeExecConfiguration {
-        if (NpmProxy.shouldConfigureProxy(System.getenv(), nodeExtension.nodeProxySettings.get())) {
-            val npmProxyEnvironmentVariables = computeNpmProxyEnvironmentVariables()
-            if (npmProxyEnvironmentVariables.isNotEmpty()) {
-                val environmentVariables =
-                        nodeExecConfiguration.environment.plus(npmProxyEnvironmentVariables)
-                return nodeExecConfiguration.copy(environment = environmentVariables)
-            }
-        }
-        return nodeExecConfiguration
-    }
-
     fun executeNpxCommand(project: ProjectApiHelper, extension: NodeExtension, nodeExecConfiguration: NodeExecConfiguration, variants: VariantComputer): ExecResult {
         val npxExecConfiguration = NpmExecConfiguration("npx") { variantComputer, nodeExtension, npmBinDir ->
             variantComputer.computeNpxExec(nodeExtension, npmBinDir)

diff --git a/src/main/kotlin/com/github/gradle/node/npm/proxy/NpmProxy.kt b/src/main/kotlin/com/github/gradle/node/npm/proxy/NpmProxy.kt
@@ -1,5 +1,6 @@
 package com.github.gradle.node.npm.proxy
 
+import com.github.gradle.node.exec.NodeExecConfiguration
 import java.net.URLEncoder
 import java.util.stream.Collectors.toList
 import java.util.stream.Stream
@@ -17,7 +18,8 @@ class NpmProxy {
         // And since npm also takes settings in the form of environment variables with the
         // NPM_CONFIG_<setting> format, we should check those. Hopefully nobody does this.
         // Windows will let you set environment variables with hyphens in them, but shells
-        // on Linux will fight you so you'll have to be sneaky, adding both here "just in case".
+        // on Linux will fight you. So you'll have to be pretty sneaky to do this.
+        // I'm adding both here "just in case".
         private val npmProxyVariables = listOf(
                 "NPM_CONFIG_PROXY", "NPM_CONFIG_HTTPS-PROXY", "NPM_CONFIG_HTTPS_PROXY", "NPM_CONFIG_NOPROXY"
         )
@@ -49,7 +51,7 @@ class NpmProxy {
         }
 
         /**
-         * Returns true if the given map of environment variables already has
+         * Returns true if the given map of environment variables has any
          * proxy settings configured.
          *
          * @param env map of environment variables
@@ -60,6 +62,33 @@ class NpmProxy {
             }
         }
 
+        /**
+         * Get a list of all known keys that affect the proxy configuration
+         */
+        fun getKnownProxyConfigurationKeys(): Set<String> {
+            return proxyVariables.plus(npmProxyVariables).toSet()
+        }
+
+        /**
+         * Creates a new NodeExecConfiguration with the proxy environment variables configured
+         */
+        fun addProxyEnvironmentVariables(proxySettings: ProxySettings, nodeExecConfiguration: NodeExecConfiguration,
+                                         environment: Map<String, String> = System.getenv()): NodeExecConfiguration {
+            if (shouldConfigureProxy(environment, proxySettings)) {
+                val npmProxyEnvironmentVariables = computeNpmProxyEnvironmentVariables()
+                val environmentVariablesToUnset = if (proxySettings == ProxySettings.FORCED) getKnownProxyConfigurationKeys()
+                                                  else emptySet()
+                if (npmProxyEnvironmentVariables.isNotEmpty()) {
+                    val environmentVariables =
+                        nodeExecConfiguration.environment
+                            .minus(environmentVariablesToUnset)
+                            .plus(npmProxyEnvironmentVariables)
+                    return nodeExecConfiguration.copy(environment = environmentVariables)
+                }
+            }
+            return nodeExecConfiguration
+        }
+
         private fun computeProxyUrlEnvironmentVariables(): MutableMap<String, String> {
             val proxyArgs = mutableMapOf<String, String>()
             for ((proxyProto, proxyParam) in

diff --git a/src/main/kotlin/com/github/gradle/node/npm/task/NpmTask.kt b/src/main/kotlin/com/github/gradle/node/npm/task/NpmTask.kt
@@ -8,7 +8,6 @@ import com.github.gradle.node.task.BaseTask
 import com.github.gradle.node.util.DefaultProjectApiHelper
 import org.gradle.api.Action
 import org.gradle.api.model.ObjectFactory
-import org.gradle.api.provider.Provider
 import org.gradle.api.provider.ProviderFactory
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.Internal
@@ -19,7 +18,6 @@ import org.gradle.kotlin.dsl.mapProperty
 import org.gradle.kotlin.dsl.newInstance
 import org.gradle.kotlin.dsl.property
 import org.gradle.process.ExecSpec
-import java.io.File
 import javax.inject.Inject
 
 abstract class NpmTask : BaseTask() {

diff --git a/src/main/kotlin/com/github/gradle/node/pnpm/exec/PnpmExecRunner.kt b/src/main/kotlin/com/github/gradle/node/pnpm/exec/PnpmExecRunner.kt
@@ -6,7 +6,6 @@ import com.github.gradle.node.exec.ExecRunner
 import com.github.gradle.node.exec.NodeExecConfiguration
 import com.github.gradle.node.npm.exec.NpmExecConfiguration
 import com.github.gradle.node.npm.proxy.NpmProxy
-import com.github.gradle.node.npm.proxy.NpmProxy.Companion.computeNpmProxyEnvironmentVariables
 import com.github.gradle.node.util.ProjectApiHelper
 import com.github.gradle.node.util.zip
 import com.github.gradle.node.variant.VariantComputer
@@ -24,24 +23,11 @@ abstract class PnpmExecRunner {
         val npmExecConfiguration = NpmExecConfiguration("pnpm"
         ) { variantComputer, nodeExtension, pnpmBinDir -> variantComputer.computePnpmExec(nodeExtension, pnpmBinDir) }
 
-        return executeCommand(project, extension, addProxyEnvironmentVariables(extension, nodeExecConfiguration),
+        return executeCommand(project, extension, NpmProxy.addProxyEnvironmentVariables(extension.nodeProxySettings.get(), nodeExecConfiguration),
             npmExecConfiguration,
             variants)
     }
 
-    private fun addProxyEnvironmentVariables(nodeExtension: NodeExtension,
-                                             nodeExecConfiguration: NodeExecConfiguration): NodeExecConfiguration {
-        if (NpmProxy.shouldConfigureProxy(System.getenv(), nodeExtension.nodeProxySettings.get())) {
-            val npmProxyEnvironmentVariables = computeNpmProxyEnvironmentVariables()
-            if (npmProxyEnvironmentVariables.isNotEmpty()) {
-                val environmentVariables =
-                    nodeExecConfiguration.environment.plus(npmProxyEnvironmentVariables)
-                return nodeExecConfiguration.copy(environment = environmentVariables)
-            }
-        }
-        return nodeExecConfiguration
-    }
-
     private fun executeCommand(project: ProjectApiHelper, extension: NodeExtension, nodeExecConfiguration: NodeExecConfiguration,
                                pnpmExecConfiguration: NpmExecConfiguration,
                                variantComputer: VariantComputer): ExecResult {

diff --git a/src/main/kotlin/com/github/gradle/node/util/ProjectApiHelper.kt b/src/main/kotlin/com/github/gradle/node/util/ProjectApiHelper.kt
@@ -10,7 +10,6 @@ import org.gradle.api.tasks.WorkResult
 import org.gradle.process.ExecOperations
 import org.gradle.process.ExecResult
 import org.gradle.process.ExecSpec
-import java.io.ByteArrayOutputStream
 import java.io.File
 import javax.inject.Inject
 

diff --git a/src/test/groovy/com/github/gradle/node/npm/task/NpmProxy_integTest.groovy b/src/test/groovy/com/github/gradle/node/npm/task/NpmProxy_integTest.groovy
@@ -3,7 +3,6 @@ package com.github.gradle.node.npm.task
 import com.github.gradle.AbstractIntegTest
 import com.github.gradle.node.ProxyTestHelper
 import org.gradle.testkit.runner.TaskOutcome
-import org.gradle.util.GradleVersion
 import org.mockserver.integration.ClientAndServer
 import org.mockserver.socket.PortFactory