Merge pull request #211 from oleg-nenashev/bug/JENKINS-50339

[JENKINS-50339] - Whitelist SVNErrorMessage and introduce RemotableSVNErrorMessage
jenkinsci · Mar 23, 2018 · 5088b1c · 5088b1c
2 parents 7f392ba + 20630b9
commit 5088b1c
Show file tree

Hide file tree

Showing 12 changed files with 148 additions and 30 deletions.
diff --git a/Jenkinsfile b/Jenkinsfile
@@ -1 +1 @@
-buildPlugin()
+buildPlugin(jenkinsVersions: [null, "2.107.1"])
diff --git a/pom.xml b/pom.xml
@@ -28,7 +28,7 @@ THE SOFTWARE.
   <parent>
     <groupId>org.jenkins-ci.plugins</groupId>
     <artifactId>plugin</artifactId>
-    <version>3.2</version>
+    <version>3.6</version>
     <relativePath />
   </parent>
 

diff --git a/src/main/java/hudson/scm/CredentialsSVNAuthenticationProviderImpl.java b/src/main/java/hudson/scm/CredentialsSVNAuthenticationProviderImpl.java
@@ -20,6 +20,7 @@
 import hudson.security.ACL;
 import hudson.util.Scrambler;
 import hudson.util.Secret;
+import jenkins.scm.impl.subversion.RemotableSVNErrorMessage;
 import org.apache.commons.beanutils.PropertyUtils;
 import org.apache.commons.lang.StringUtils;
 import org.tmatesoft.svn.core.SVNErrorCode;
@@ -470,21 +471,9 @@ public SVNCertificateAuthenticationBuilder(CertificateCredentials c) {
                 ByteArrayOutputStream bos = new ByteArrayOutputStream();
                 dst.store(bos, passwordChars);
                 certificateFile = bos.toByteArray();
-            } catch (KeyStoreException e) {
+            } catch (KeyStoreException | CertificateException | NoSuchAlgorithmException | IOException e) {
                 throw new RuntimeException(
-                        SVNErrorMessage.create(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE, "Unable to save certificate").getFullMessage(),
-                                e);
-            } catch (CertificateException e) {
-                throw new RuntimeException(
-                        SVNErrorMessage.create(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE, "Unable to save certificate").getFullMessage(),
-                                e);
-            } catch (NoSuchAlgorithmException e) {
-                throw new RuntimeException(
-                        SVNErrorMessage.create(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE, "Unable to save certificate").getFullMessage(),
-                                e);
-            } catch (IOException e) {
-                throw new RuntimeException(
-                        SVNErrorMessage.create(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE, "Unable to save certificate").getFullMessage(),
+                        new RemotableSVNErrorMessage(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE, "Unable to save certificate").getFullMessage(),
                                 e);
             } finally {
                 try {

diff --git a/src/main/java/hudson/scm/DirAwareSVNXMLLogHandler.java b/src/main/java/hudson/scm/DirAwareSVNXMLLogHandler.java
@@ -15,6 +15,7 @@
 import java.util.Iterator;
 import java.util.LinkedList;
 
+import jenkins.scm.impl.subversion.RemotableSVNErrorMessage;
 import org.tmatesoft.svn.core.ISVNLogEntryHandler;
 import org.tmatesoft.svn.core.SVNErrorCode;
 import org.tmatesoft.svn.core.SVNErrorMessage;
@@ -83,7 +84,7 @@ public void handleLogEntry(SVNLogEntry logEntry) throws SVNException {
               sendToHandler(logEntry);
           }
       } catch (SAXException e) {
-          SVNErrorMessage err = SVNErrorMessage.create(SVNErrorCode.XML_MALFORMED, e.getLocalizedMessage());
+          RemotableSVNErrorMessage err = new RemotableSVNErrorMessage(SVNErrorCode.XML_MALFORMED, e.getLocalizedMessage());
           SVNErrorManager.error(err, e, SVNLogType.DEFAULT);
       }
   }

diff --git a/src/main/java/hudson/scm/SubversionEventHandlerImpl.java b/src/main/java/hudson/scm/SubversionEventHandlerImpl.java
@@ -11,6 +11,7 @@
  */
 package hudson.scm;
 
+import jenkins.scm.impl.subversion.RemotableSVNErrorMessage;
 import org.tmatesoft.svn.core.SVNErrorCode;
 import org.tmatesoft.svn.core.SVNErrorMessage;
 import org.tmatesoft.svn.core.SVNException;
@@ -62,7 +63,7 @@ public void handleEvent(SVNEvent event, double progress) throws SVNException {
             try {
                 path = getRelativePath(file);
             } catch (IOException e) {
-                throw new SVNException(SVNErrorMessage.create(SVNErrorCode.FS_GENERAL, e));
+                throw new SVNException(new RemotableSVNErrorMessage(SVNErrorCode.FS_GENERAL, e));
             }
             path = getLocalPath(path);
         }

diff --git a/src/main/java/hudson/scm/SubversionSCM.java b/src/main/java/hudson/scm/SubversionSCM.java
@@ -134,6 +134,7 @@
 import javax.servlet.ServletException;
 import javax.xml.transform.stream.StreamResult;
 
+import jenkins.scm.impl.subversion.RemotableSVNErrorMessage;
 import net.sf.json.JSONObject;
 
 import org.acegisecurity.context.SecurityContext;
@@ -1907,8 +1908,8 @@ public SshPublicKeyCredential(String userName, String passphrase, File keyFile)
                     setFilePermissions(savedKeyFile, "600");
                 } catch (IOException e) {
                     throw new SVNException(
-                            SVNErrorMessage.create(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE,
-                                    "Unable to save private key") ,e);
+                            new RemotableSVNErrorMessage(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE,
+                                    "Unable to save private key"), e);
                 }
             }
 
@@ -1967,13 +1968,9 @@ public String call() throws IOException {
                             privateKey = FileUtils.readFileToString(getKeyFile(),"iso-8859-1");
                         }
                         return new SVNSSHAuthentication(userName, privateKey.toCharArray(), Scrambler.descramble(Secret.toString(passphrase)),-1,false);
-                    } catch (IOException e) {
-                        throw new SVNException(
-                                SVNErrorMessage.create(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE,
-                                        "Unable to load private key"), e);
-                    } catch (InterruptedException e) {
+                    } catch (IOException | InterruptedException e) {
                         throw new SVNException(
-                                SVNErrorMessage.create(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE,
+                                new RemotableSVNErrorMessage(SVNErrorCode.AUTHN_CREDS_UNAVAILABLE,
                                         "Unable to load private key"), e);
                     }
                 } else

diff --git a/src/main/java/hudson/scm/subversion/SubversionUpdateEventHandler.java b/src/main/java/hudson/scm/subversion/SubversionUpdateEventHandler.java
@@ -27,6 +27,8 @@
 import hudson.scm.SubversionSCM.External;
 import java.util.HashMap;
 import java.util.Map;
+
+import jenkins.scm.impl.subversion.RemotableSVNErrorMessage;
 import org.tmatesoft.svn.core.SVNCancelException;
 import org.tmatesoft.svn.core.SVNErrorCode;
 import org.tmatesoft.svn.core.SVNErrorMessage;
@@ -124,7 +126,7 @@ public void handleEvent(SVNEvent event, double progress) throws SVNException {
                 try {
                     path = getLocalPath(getRelativePath(file));
                 } catch (IOException e) {
-                    throw new SVNException(SVNErrorMessage.create(SVNErrorCode.FS_GENERAL, e));
+                    throw new SVNException(new RemotableSVNErrorMessage(SVNErrorCode.FS_GENERAL, e));
                 }
 
                 out.println(Messages.SubversionUpdateEventHandler_FetchExternal(details.getUrl(), event.getRevision(), file));
@@ -139,7 +141,7 @@ public void handleEvent(SVNEvent event, double progress) throws SVNException {
             }
 
             if (cancelProcessOnExternalsFailed) {
-              throw new SVNException(SVNErrorMessage.create(SVNErrorCode.CL_ERROR_PROCESSING_EXTERNALS,
+              throw new SVNException(new RemotableSVNErrorMessage(SVNErrorCode.CL_ERROR_PROCESSING_EXTERNALS,
                   SVNErrorCode.CL_ERROR_PROCESSING_EXTERNALS.getDescription() + ": <" + file.getName() + ">"));
             }
         }

diff --git a/src/main/java/hudson/scm/subversion/UpdateWithCleanUpdater.java b/src/main/java/hudson/scm/subversion/UpdateWithCleanUpdater.java
@@ -25,6 +25,7 @@
 
 import hudson.Extension;
 import hudson.scm.SubversionSCM.ModuleLocation;
+import jenkins.scm.impl.subversion.RemotableSVNErrorMessage;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.tmatesoft.svn.core.SVNDepth;
 import org.tmatesoft.svn.core.SVNErrorCode;
@@ -75,7 +76,7 @@ public void handleStatus(SVNStatus status) throws SVNException {
                             else
                                 f.delete();
                         } catch (IOException e) {
-                            throw new SVNException(SVNErrorMessage.create(SVNErrorCode.UNKNOWN, e));
+                            throw new SVNException(new RemotableSVNErrorMessage(SVNErrorCode.UNKNOWN, e));
                         }
                     }
                 }

diff --git a/src/main/java/jenkins/scm/impl/subversion/RemotableSVNErrorMessage.java b/src/main/java/jenkins/scm/impl/subversion/RemotableSVNErrorMessage.java
@@ -0,0 +1,53 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2018 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package jenkins.scm.impl.subversion;
+
+import hudson.remoting.ProxyException;
+import org.tmatesoft.svn.core.SVNErrorCode;
+import org.tmatesoft.svn.core.SVNErrorMessage;
+
+/**
+ * Version of {@link SVNErrorMessage}, which can be serialized over the channel.
+ * This version does not serialize random {@link Object} instances.
+ * @author Oleg Nenashev
+ * @since TODO
+ */
+public class RemotableSVNErrorMessage extends SVNErrorMessage {
+
+    public RemotableSVNErrorMessage(SVNErrorCode code) {
+        super(code, null, null, null, 0);
+    }
+
+    public RemotableSVNErrorMessage(SVNErrorCode code, String message) {
+        super(code, message, null, null, 0);
+    }
+
+    public RemotableSVNErrorMessage(SVNErrorCode code, Throwable cause) {
+        super(code, null, null, new ProxyException(cause), 0);
+    }
+
+    public RemotableSVNErrorMessage(SVNErrorCode code, String message, Throwable cause) {
+        super(code, message, null, new ProxyException(cause), 0);
+    }
+}
diff --git a/src/main/resources/META-INF/hudson.remoting.ClassFilter b/src/main/resources/META-INF/hudson.remoting.ClassFilter
@@ -0,0 +1,5 @@
+# Whitelist for error messaging (JENKINS-50339).
+# SVNErrorMessage supports "Object" fields in API in 'myObjects',
+# so it is not guaranteed to work with JEP-200 in all cases.
+org.tmatesoft.svn.core.SVNErrorMessage
+org.tmatesoft.svn.core.SVNErrorCode
diff --git a/src/test/java/hudson/scm/SubversionSCMTest.java b/src/test/java/hudson/scm/SubversionSCMTest.java
@@ -47,6 +47,7 @@
 import hudson.triggers.SCMTrigger;
 import hudson.util.FormValidation;
 import hudson.util.StreamTaskListener;
+import jenkins.scm.impl.subversion.RemotableSVNErrorMessage;
 import org.dom4j.Document;
 import org.dom4j.io.DOMReader;
 import org.junit.Test;
@@ -1242,7 +1243,7 @@ public void infiniteLoop() throws Exception {
         assertNotNull(a);
         attempted.add(a);
         for (int i=0; i<10; i++) {
-            m.acknowledgeAuthentication(false,kind,realm,SVNErrorMessage.create(SVNErrorCode.RA_NOT_AUTHORIZED),a);
+            m.acknowledgeAuthentication(false,kind,realm,new RemotableSVNErrorMessage(SVNErrorCode.RA_NOT_AUTHORIZED),a);
             try {
                 a = m.getNextAuthentication(kind,realm,repo);
                 assertNotNull(a);

diff --git a/src/test/java/jenkins/scm/impl/subversion/RemotableSVNErrorMessageStepTest.java b/src/test/java/jenkins/scm/impl/subversion/RemotableSVNErrorMessageStepTest.java
@@ -0,0 +1,68 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2018 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package jenkins.scm.impl.subversion;
+
+import hudson.remoting.Callable;
+import hudson.slaves.DumbSlave;
+import jenkins.security.MasterToSlaveCallable;
+import org.jenkinsci.remoting.RoleChecker;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.For;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.tmatesoft.svn.core.SVNErrorCode;
+import org.tmatesoft.svn.core.SVNErrorMessage;
+import org.tmatesoft.svn.core.SVNException;
+
+/**
+ * @author Oleg Nenashev
+ */
+@For(SVNErrorMessage.class)
+public class RemotableSVNErrorMessageStepTest {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Test
+    @Issue("JENKINS-50339")
+    public void shouldSerializeException() throws Exception {
+        DumbSlave agent = j.createOnlineSlave();
+        try {
+            agent.getChannel().call(new ErrorCallable());
+        } catch (SVNException e) {
+            return; // fine
+        }
+    }
+
+    private static class ErrorCallable extends MasterToSlaveCallable<Void, SVNException> {
+
+        @Override
+        public Void call() throws SVNException {
+            SVNErrorMessage err = new RemotableSVNErrorMessage(SVNErrorCode.UNKNOWN, "Just a test exception",
+                    new IllegalStateException("foo"));
+            throw new SVNException(err);
+        }
+    }
+}