Merge pull request #87 from StevenGBrown/JENKINS-30028

[FIXED JENKINS-30028] Remove logging from password wrapper decorateLogger
jenkinsci · Aug 31, 2016 · 586ec08 · 586ec08
2 parents 80b9665 + f56667c
commit 586ec08
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 8 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectPasswordWrapper.java b/src/main/java/org/jenkinsci/plugins/envinject/EnvInjectPasswordWrapper.java
@@ -79,6 +79,14 @@ public void setPasswordEntries(EnvInjectPasswordEntry[] passwordEntries) {
 
     @Override
     public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException {
+        EnvInjectLogger logger = new EnvInjectLogger(listener);
+        if (isInjectGlobalPasswords()) {
+            logger.info("Inject global passwords.");
+        }
+        if (isMaskPasswordParameters()) {
+            logger.info("Mask passwords passed as build parameters.");
+        }
+
         return new Environment() {
         };
     }
@@ -113,19 +121,11 @@ private List<EnvInjectPasswordEntry> getEnvInjectPasswordEntries() throws EnvInj
     @Override
     public OutputStream decorateLogger(AbstractBuild build, OutputStream outputStream) throws IOException, InterruptedException, Run.RunnerAbortedException {
         try {
-            EnvInjectLogger logger = new EnvInjectLogger(new StreamTaskListener(outputStream));
-
-            if (isInjectGlobalPasswords()) {
-                logger.info("Inject global passwords.");
-            }
-
             // Decorate passwords provided by EnvInject Plugin (globals and locals)
             List<String> passwords2decorate = Lists.newArrayList(Lists.transform(getEnvInjectPasswordEntries(), PASSWORD_ENTRY_TO_VALUE));
 
             // Decorate passwords passed as build parameters
             if (isMaskPasswordParameters()) {
-                logger.info("Mask passwords passed as build parameters.");
-
                 ParametersAction parametersAction = build.getAction(ParametersAction.class);
                 if (parametersAction != null) {
                     List<ParameterValue> parameters = parametersAction.getParameters();

diff --git a/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectPasswordTest.java b/src/test/java/org/jenkinsci/plugins/envinject/EnvInjectPasswordTest.java
@@ -5,10 +5,15 @@
 import hudson.tasks.BuildWrapper;
 import hudson.tasks.BuildWrapperDescriptor;
 import hudson.util.Secret;
+
+import java.io.ByteArrayOutputStream;
 import java.io.FilterOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+
+import org.hamcrest.Matchers;
 import org.jenkinsci.lib.envinject.EnvInjectAction;
+import org.junit.Assert;
 import org.junit.Rule;
 
 import java.util.ArrayList;
@@ -17,10 +22,13 @@
 
 import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.TestExtension;
 
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -85,6 +93,29 @@ public void testFileHandlesLeak() throws Exception {
         assertTrue("Nested output stream has not been closed", fileLeakDetector.getLastOutputStream().isClosed());
     } 
 
+    /**
+     * Test that
+     * {@link EnvInjectPasswordWrapper#decorateLogger(AbstractBuild, OutputStream)}
+     * does not write to the output stream. The written message would bypass
+     * the other build wrappers, which have not yet wrapped the OutputStream.
+     *
+     * @throws Exception
+     */
+    @Test
+    @Issue("JENKINS-30028")
+    public void testDoNotWriteDuringDecorateLogger() throws Exception {
+      FreeStyleProject project = jenkins.createFreeStyleProject();
+      FreeStyleBuild build = new FreeStyleBuild(project);
+
+      EnvInjectPasswordWrapper passwordWrapper = new EnvInjectPasswordWrapper();
+      passwordWrapper.setInjectGlobalPasswords(true);
+      passwordWrapper.setMaskPasswordParameters(true);
+      ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+      passwordWrapper.decorateLogger(build, outputStream);
+
+      assertThat(outputStream.toByteArray(), is(new byte[0]));
+    }
+
     private void checkEnvInjectResult(FreeStyleBuild build) {
         EnvInjectAction action = build.getAction(EnvInjectAction.class);
         Map<String, String> envVars = action.getEnvMap();