From 959834c2181c58646283e4311cdb41c641846392 Mon Sep 17 00:00:00 2001 From: shubh586 Date: Tue, 6 Jan 2026 02:31:51 +0530 Subject: [PATCH] JENKINS-68961: Fail gracefully on empty email address input --- src/main/java/hudson/tasks/Mailer.java | 369 ++++++++++++--------- src/test/java/hudson/tasks/MailerTest.java | 218 ++++++------ 2 files changed, 333 insertions(+), 254 deletions(-) diff --git a/src/main/java/hudson/tasks/Mailer.java b/src/main/java/hudson/tasks/Mailer.java index 632ee739..2e9795a1 100644 --- a/src/main/java/hudson/tasks/Mailer.java +++ b/src/main/java/hudson/tasks/Mailer.java @@ -24,10 +24,34 @@ */ package hudson.tasks; +import java.io.File; +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.Date; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.tools.ant.types.selectors.SelectorUtils; +import org.jenkinsci.Symbol; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.DoNotUse; +import org.kohsuke.accmod.restrictions.NoExternalUse; +import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.StaplerRequest2; +import org.kohsuke.stapler.export.Exported; +import org.kohsuke.stapler.interceptor.RequirePOST; + import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - import hudson.BulkChange; import hudson.EnvVars; import hudson.Extension; @@ -36,30 +60,17 @@ import hudson.Launcher; import hudson.RestrictedSince; import hudson.Util; -import hudson.model.*; -import jenkins.plugins.mailer.tasks.i18n.Messages; -import jenkins.security.FIPS140; +import hudson.model.AbstractBuild; +import hudson.model.AbstractProject; +import hudson.model.BuildListener; +import hudson.model.Run; +import hudson.model.TaskListener; +import hudson.model.User; +import hudson.model.UserPropertyDescriptor; import hudson.security.Permission; import hudson.util.FormValidation; import hudson.util.Secret; import hudson.util.XStream2; - -import jenkins.model.JenkinsLocationConfiguration; -import org.jenkinsci.Symbol; -import org.kohsuke.stapler.DataBoundConstructor; - -import java.io.File; -import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.util.Date; -import java.util.Properties; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.regex.Pattern; -import java.util.regex.Matcher; import jakarta.mail.Address; import jakarta.mail.Authenticator; import jakarta.mail.Message; @@ -70,20 +81,12 @@ import jakarta.mail.internet.AddressException; import jakarta.mail.internet.InternetAddress; import jakarta.mail.internet.MimeMessage; - -import org.apache.tools.ant.types.selectors.SelectorUtils; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; -import org.kohsuke.stapler.DataBoundSetter; -import org.kohsuke.stapler.QueryParameter; -import org.kohsuke.stapler.StaplerRequest2; -import org.kohsuke.stapler.export.Exported; - import jenkins.model.Jenkins; +import jenkins.model.JenkinsLocationConfiguration; +import jenkins.plugins.mailer.tasks.i18n.Messages; +import jenkins.security.FIPS140; import jenkins.tasks.SimpleBuildStep; import net.sf.json.JSONObject; -import org.kohsuke.accmod.restrictions.DoNotUse; -import org.kohsuke.stapler.interceptor.RequirePOST; /** * {@link Publisher} that sends the build result in e-mail. @@ -118,12 +121,14 @@ public boolean isNotifyEveryUnstableBuild() { * This is left for backward compatibility. */ @Deprecated - public Mailer() {} + public Mailer() { + } /** - * @param recipients one or more recipients with separators + * @param recipients one or more recipients with separators * @param notifyEveryUnstableBuild inverted for historical reasons. - * @param sendToIndividuals if {@code true} mails are sent to individual committers + * @param sendToIndividuals if {@code true} mails are sent to individual + * committers */ @DataBoundConstructor public Mailer(String recipients, boolean notifyEveryUnstableBuild, boolean sendToIndividuals) { @@ -134,14 +139,16 @@ public Mailer(String recipients, boolean notifyEveryUnstableBuild, boolean sendT @Override @SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification = "build cannnot be null and the workspace is not used in case it was null") - public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { + public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) + throws InterruptedException, IOException { perform(build, build.getWorkspace(), launcher, listener); return true; } @Override - public void perform(Run build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { - if(debug) + public void perform(Run build, FilePath workspace, Launcher launcher, TaskListener listener) + throws IOException, InterruptedException { + if (debug) listener.getLogger().println("Running mailer"); // substitute build parameters EnvVars env = build.getEnvironment(listener); @@ -150,9 +157,11 @@ public void perform(Run build, FilePath workspace, Launcher launcher, TaskL new MailSender(recip, dontNotifyEveryUnstableBuild, sendToIndividuals, descriptor().getCharset()) { /** Check whether a path (/-separated) will be archived. */ @Override - public boolean artifactMatches(String path, AbstractBuild build) { - // TODO a Notifier runs after a Recorder so it would make more sense to just check actual artifacts, not configuration - // (Anyway currently this code would only be called for an AbstractBuild, since otherwise we cannot know what hyperlink to use for a random workspace.) + public boolean artifactMatches(String path, AbstractBuild build) { + // TODO a Notifier runs after a Recorder so it would make more sense to just + // check actual artifacts, not configuration + // (Anyway currently this code would only be called for an AbstractBuild, since + // otherwise we cannot know what hyperlink to use for a random workspace.) ArtifactArchiver aa = build.getProject().getPublishersList().get(ArtifactArchiver.class); if (aa == null) { LOGGER.finer("No ArtifactArchiver found"); @@ -165,14 +174,16 @@ public boolean artifactMatches(String path, AbstractBuild build) { pattern += "**"; } if (SelectorUtils.matchPath(pattern, path)) { - LOGGER.log(Level.FINER, "DescriptorImpl.artifactMatches true for {0} against {1}", new Object[] {path, pattern}); + LOGGER.log(Level.FINER, "DescriptorImpl.artifactMatches true for {0} against {1}", + new Object[] { path, pattern }); return true; } } - LOGGER.log(Level.FINER, "DescriptorImpl.artifactMatches for {0} matched none of {1}", new Object[] {path, artifacts}); + LOGGER.log(Level.FINER, "DescriptorImpl.artifactMatches for {0} matched none of {1}", + new Object[] { path, artifacts }); return false; } - }.run(build,listener); + }.run(build, listener); } /** @@ -183,13 +194,14 @@ public BuildStepMonitor getRequiredMonitorService() { } private static Pattern ADDRESS_PATTERN = Pattern.compile("\\s*([^<]*)<([^>]+)>\\s*"); - + /** * Deprecated! Converts a string to {@link InternetAddress}. + * * @param strAddress Address string - * @param charset Charset (encoding) to be used + * @param charset Charset (encoding) to be used * @return {@link InternetAddress} for the specified string - * @throws AddressException Malformed address + * @throws AddressException Malformed address * @throws UnsupportedEncodingException Unsupported encoding * * @deprecated Use {@link #stringToAddress(java.lang.String, java.lang.String)}. @@ -198,23 +210,29 @@ public BuildStepMonitor getRequiredMonitorService() { @Restricted(DoNotUse.class) @RestrictedSince("1.16") @SuppressFBWarnings(value = "NM_METHOD_NAMING_CONVENTION", justification = "It's deprecated and required for API compatibility") - public static InternetAddress StringToAddress(String strAddress, String charset) throws AddressException, UnsupportedEncodingException { + public static InternetAddress StringToAddress(String strAddress, String charset) + throws AddressException, UnsupportedEncodingException { return stringToAddress(strAddress, charset); } - + /** * Converts a string to {@link InternetAddress}. + * * @param strAddress Address string - * @param charset Charset (encoding) to be used + * @param charset Charset (encoding) to be used * @return {@link InternetAddress} for the specified string - * @throws AddressException Malformed address + * @throws AddressException Malformed address * @throws UnsupportedEncodingException Unsupported encoding * @since TODO */ - public static @NonNull InternetAddress stringToAddress(@NonNull String strAddress, + public static @NonNull InternetAddress stringToAddress(@NonNull String strAddress, @NonNull String charset) throws AddressException, UnsupportedEncodingException { + if (strAddress.isBlank()) { + throw new AddressException( + "Email address must not be empty. Please configure a valid email address."); + } Matcher m = ADDRESS_PATTERN.matcher(strAddress); - if(!m.matches()) { + if (!m.matches()) { return new InternetAddress(strAddress); } @@ -225,7 +243,7 @@ public static InternetAddress StringToAddress(String strAddress, String charset) /** * @deprecated as of 1.286 - * Use {@link #descriptor()} to obtain the current instance. + * Use {@link #descriptor()} to obtain the current instance. */ @Deprecated @Restricted(NoExternalUse.class) @@ -240,7 +258,8 @@ public static DescriptorImpl descriptor() { @Extension public static final class DescriptorImpl extends BuildStepDescriptor { /** - * The default e-mail address suffix appended to the user name found from changelog, + * The default e-mail address suffix appended to the user name found from + * changelog, * to send e-mails. Null if not configured. */ private String defaultSuffix; @@ -249,10 +268,12 @@ public static final class DescriptorImpl extends BuildStepDescriptor * Hudson's own URL, to put into the e-mail. * * @deprecated as of 1.4 - * Maintained in {@link JenkinsLocationConfiguration} but left here - * for compatibility just in case, so as not to lose this information. - * This is loaded to {@link JenkinsLocationConfiguration} via the XML file - * marshaled with {@link XStream2}. + * Maintained in {@link JenkinsLocationConfiguration} but left here + * for compatibility just in case, so as not to lose this + * information. + * This is loaded to {@link JenkinsLocationConfiguration} via the + * XML file + * marshaled with {@link XStream2}. */ @Deprecated private String hudsonUrl; @@ -261,7 +282,6 @@ public static final class DescriptorImpl extends BuildStepDescriptor @Deprecated private transient String smtpAuthUsername; - /** @deprecated as of 1.23, use {@link #authentication} */ @Deprecated private transient Secret smtpAuthPassword; @@ -273,31 +293,36 @@ public static final class DescriptorImpl extends BuildStepDescriptor * Null if not configured. * * @deprecated as of 1.4 - * Maintained in {@link JenkinsLocationConfiguration} but left here - * for compatibility just in case, so as not to lose this information. + * Maintained in {@link JenkinsLocationConfiguration} but left here + * for compatibility just in case, so as not to lose this + * information. */ @Deprecated private String adminAddress; /** - * The e-mail address that Jenkins puts to "Reply-To" header in outgoing e-mails. + * The e-mail address that Jenkins puts to "Reply-To" header in outgoing + * e-mails. * Null if not configured. */ private String replyToAddress; /** - * The SMTP server to use for sending e-mail. Null for default to the environment, + * The SMTP server to use for sending e-mail. Null for default to the + * environment, * which is usually localhost. */ private String smtpHost; - + /** - * If true use SSL on port 465 (standard SMTPS) unless smtpPort is set. + * If true use SSL on port 465 (standard SMTPS) unless smtpPort is + * set. */ private boolean useSsl; /** - * If true use TLS on port 587 (standard STARTTLS) unless smtpPort is set. + * If true use TLS on port 587 (standard STARTTLS) unless smtpPort + * is set. */ private boolean useTls; @@ -311,14 +336,13 @@ public static final class DescriptorImpl extends BuildStepDescriptor * The charset to use for the text and subject. */ private String charset; - + /** * Used to keep track of number test e-mails. */ private static transient AtomicInteger testEmailCount = new AtomicInteger(0); - @SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD", - justification = "Writing to a deprecated field") + @SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD", justification = "Writing to a deprecated field") public DescriptorImpl() { load(); DESCRIPTOR = this; @@ -350,12 +374,16 @@ public void setReplyToAddress(String address) { /** * Creates a JavaMail session. + * * @return mail session based on the underlying session parameters. */ public Session createSession() { - return createSession(smtpHost,smtpPort,useSsl,useTls,getSmtpAuthUserName(),getSmtpAuthPasswordSecret()); + return createSession(smtpHost, smtpPort, useSsl, useTls, getSmtpAuthUserName(), + getSmtpAuthPasswordSecret()); } - private static Session createSession(String smtpHost, String smtpPort, boolean useSsl, boolean useTls, String smtpAuthUserName, Secret smtpAuthPassword) { + + private static Session createSession(String smtpHost, String smtpPort, boolean useSsl, boolean useTls, + String smtpAuthUserName, Secret smtpAuthPassword) { final String SMTP_PORT_PROPERTY = "mail.smtp.port"; final String SMTP_SOCKETFACTORY_PORT_PROPERTY = "mail.smtp.socketFactory.port"; final String SMTP_SSL_ENABLE_PROPERTY = "mail.smtp.ssl.enable"; @@ -365,74 +393,81 @@ private static Session createSession(String smtpHost, String smtpPort, boolean u smtpAuthUserName = Util.fixEmptyAndTrim(smtpAuthUserName); Properties props = new Properties(System.getProperties()); - if(smtpHost!=null) { + if (smtpHost != null) { props.put("mail.smtp.host", smtpHost); } - if (smtpPort!=null) { + if (smtpPort != null) { props.put(SMTP_PORT_PROPERTY, smtpPort); } if (useSsl) { - /* This allows the user to override settings by setting system properties but - * also allows us to use the default SMTPs port of 465 if no port is already set. - * It would be cleaner to use smtps, but that's done by calling session.getTransport()... - * and thats done in mail sender, and it would be a bit of a hack to get it all to - * coordinate, and we can make it work through setting mail.smtp properties. - */ - if (props.getProperty(SMTP_SOCKETFACTORY_PORT_PROPERTY) == null) { - String port = smtpPort==null?"465":smtpPort; + /* + * This allows the user to override settings by setting system properties but + * also allows us to use the default SMTPs port of 465 if no port is already + * set. + * It would be cleaner to use smtps, but that's done by calling + * session.getTransport()... + * and thats done in mail sender, and it would be a bit of a hack to get it all + * to + * coordinate, and we can make it work through setting mail.smtp properties. + */ + if (props.getProperty(SMTP_SOCKETFACTORY_PORT_PROPERTY) == null) { + String port = smtpPort == null ? "465" : smtpPort; props.put(SMTP_PORT_PROPERTY, port); props.put(SMTP_SOCKETFACTORY_PORT_PROPERTY, port); - } - if (props.getProperty(SMTP_SSL_ENABLE_PROPERTY) == null) { + } + if (props.getProperty(SMTP_SSL_ENABLE_PROPERTY) == null) { props.put(SMTP_SSL_ENABLE_PROPERTY, "true"); props.put("mail.smtp.ssl.checkserveridentity", true); - } - props.put("mail.smtp.socketFactory.fallback", "false"); - if (props.getProperty("mail.smtp.ssl.checkserveridentity") == null) { + } + props.put("mail.smtp.socketFactory.fallback", "false"); + if (props.getProperty("mail.smtp.ssl.checkserveridentity") == null) { props.put("mail.smtp.ssl.checkserveridentity", "true"); } - } - if(useTls){ - /* This allows the user to override settings by setting system properties and - * also allows us to use the default STARTTLS port, 587, if no port is already set. - * Only the properties included below are required to use STARTTLS and they are - * not expected to be enabled simultaneously with SSL (it will actually throw a - * "javax.net.ssl.SSLException: Unrecognized SSL message, plaintext connection?" - * if SMTP server expects only TLS). - */ + } + if (useTls) { + /* + * This allows the user to override settings by setting system properties and + * also allows us to use the default STARTTLS port, 587, if no port is already + * set. + * Only the properties included below are required to use STARTTLS and they are + * not expected to be enabled simultaneously with SSL (it will actually throw a + * "javax.net.ssl.SSLException: Unrecognized SSL message, plaintext connection?" + * if SMTP server expects only TLS). + */ if (props.getProperty(SMTP_SOCKETFACTORY_PORT_PROPERTY) == null) { - String port = smtpPort==null?"587":smtpPort; + String port = smtpPort == null ? "587" : smtpPort; props.put(SMTP_PORT_PROPERTY, port); props.put(SMTP_SOCKETFACTORY_PORT_PROPERTY, port); } props.put("mail.smtp.starttls.enable", "true"); props.put("mail.smtp.starttls.required", "true"); } - if(smtpAuthUserName!=null) { - props.put("mail.smtp.auth","true"); + if (smtpAuthUserName != null) { + props.put("mail.smtp.auth", "true"); if (FIPS140.useCompliantAlgorithms()) { // the authentication mechanisms can only be performed when protected by TLS/SSL if (!(useSsl || useTls)) { - throw new IllegalStateException("SMTP Authentication can only be used when either TLS or SSL is used"); + throw new IllegalStateException( + "SMTP Authentication can only be used when either TLS or SSL is used"); } } } - // avoid hang by setting some timeout. - props.put("mail.smtp.timeout","60000"); - props.put("mail.smtp.connectiontimeout","60000"); + // avoid hang by setting some timeout. + props.put("mail.smtp.timeout", "60000"); + props.put("mail.smtp.connectiontimeout", "60000"); - return Session.getInstance(props,getAuthenticator(smtpAuthUserName,Secret.toString(smtpAuthPassword))); + return Session.getInstance(props, getAuthenticator(smtpAuthUserName, Secret.toString(smtpAuthPassword))); } private static Authenticator getAuthenticator(final String smtpAuthUserName, final String smtpAuthPassword) { - if(smtpAuthUserName == null) { - return null; + if (smtpAuthUserName == null) { + return null; } return new Authenticator() { @Override protected PasswordAuthentication getPasswordAuthentication() { - return new PasswordAuthentication(smtpAuthUserName,smtpAuthPassword); + return new PasswordAuthentication(smtpAuthUserName, smtpAuthPassword); } }; } @@ -440,12 +475,14 @@ protected PasswordAuthentication getPasswordAuthentication() { @Override public boolean configure(StaplerRequest2 req, JSONObject json) throws FormException { - // Nested Describable (SMTPAuthentication) is not set to null in case it is not configured. - // To mitigate that, it is being set to null before (so it gets set to sent value or null correctly) and, in + // Nested Describable (SMTPAuthentication) is not set to null in case it is not + // configured. + // To mitigate that, it is being set to null before (so it gets set to sent + // value or null correctly) and, in // case of failure to databind, it gets reverted to previous value. // Would not be necessary by https://github.com/jenkinsci/jenkins/pull/3669 SMTPAuthentication current = this.authentication; - + try (BulkChange b = new BulkChange(this)) { this.authentication = null; req.bindJSON(this, json); @@ -454,12 +491,13 @@ public boolean configure(StaplerRequest2 req, JSONObject json) throws FormExcept this.authentication = current; throw new FormException("Failed to apply configuration", e, null); } - + return true; } private String nullify(String v) { - if(v!=null && v.length()==0) v=null; + if (v != null && v.length() == 0) + v = null; return v; } @@ -467,7 +505,6 @@ public String getSmtpHost() { return smtpHost; } - /** @deprecated as of 1.23, use {@link #getSmtpHost()} */ @Deprecated public String getSmtpServer() { @@ -476,7 +513,7 @@ public String getSmtpServer() { /** * @deprecated as of 1.4 - * Use {@link JenkinsLocationConfiguration} instead + * Use {@link JenkinsLocationConfiguration} instead * @return administrator mail address */ @Deprecated @@ -486,7 +523,7 @@ public String getAdminAddress() { /** * @deprecated as of 1.4 - * Use {@link JenkinsLocationConfiguration} instead + * Use {@link JenkinsLocationConfiguration} instead * @return Jenkins base URL */ @Deprecated @@ -496,31 +533,34 @@ public String getUrl() { /** * @deprecated as of 1.21 - * Use {@link #authentication} + * Use {@link #authentication} */ @Deprecated public String getSmtpAuthUserName() { - if (authentication == null) return null; + if (authentication == null) + return null; return authentication.getUsername(); } /** * @deprecated as of 1.21 - * Use {@link #authentication} + * Use {@link #authentication} */ @Deprecated public String getSmtpAuthPassword() { - if (authentication == null) return null; + if (authentication == null) + return null; return Secret.toString(authentication.getPassword()); } public Secret getSmtpAuthPasswordSecret() { - if (authentication == null) return null; + if (authentication == null) + return null; return authentication.getPassword(); } public boolean getUseSsl() { - return useSsl; + return useSsl; } public boolean getUseTls() { @@ -528,13 +568,14 @@ public boolean getUseTls() { } public String getSmtpPort() { - return smtpPort; + return smtpPort; } public String getCharset() { - String c = charset; - if (c == null || c.length() == 0) c = "UTF-8"; - return c; + String c = charset; + if (c == null || c.length() == 0) + c = "UTF-8"; + return c; } @DataBoundSetter @@ -545,7 +586,7 @@ public void setDefaultSuffix(String defaultSuffix) { /** * @deprecated as of 1.4 - * Use {@link JenkinsLocationConfiguration} instead + * Use {@link JenkinsLocationConfiguration} instead * @param hudsonUrl Jenkins base URL to set */ @Deprecated @@ -555,7 +596,7 @@ public void setHudsonUrl(String hudsonUrl) { /** * @deprecated as of 1.4 - * Use {@link JenkinsLocationConfiguration} instead + * Use {@link JenkinsLocationConfiguration} instead * @param adminAddress Jenkins administrator mail address to set */ @Deprecated @@ -609,7 +650,7 @@ public SMTPAuthentication getAuthentication() { /** * @deprecated as of 1.21 - * Use {@link #authentication} + * Use {@link #authentication} */ @Deprecated public void setSmtpAuth(String userName, String password) { @@ -622,9 +663,9 @@ public void setSmtpAuth(String userName, String password) { @Override public Publisher newInstance(StaplerRequest2 req, JSONObject formData) throws FormException { - Mailer m = (Mailer)super.newInstance(req, formData); + Mailer m = (Mailer) super.newInstance(req, formData); - if(hudsonUrl==null) { + if (hudsonUrl == null) { // if Hudson URL is not configured yet, infer some default hudsonUrl = Functions.inferHudsonURL(req); save(); @@ -653,16 +694,16 @@ public FormValidation doAddressCheck(@QueryParameter String value) { public FormValidation doCheckSmtpHost(@QueryParameter String value) { Jenkins.get().checkPermission(Jenkins.MANAGE); try { - if (Util.fixEmptyAndTrim(value)!=null) + if (Util.fixEmptyAndTrim(value) != null) InetAddress.getByName(value); return FormValidation.ok(); } catch (UnknownHostException e) { - return FormValidation.error(Messages.Mailer_Unknown_Host_Name()+value); + return FormValidation.error(Messages.Mailer_Unknown_Host_Name() + value); } } public FormValidation doCheckDefaultSuffix(@QueryParameter String value) { - if (value.matches("@[A-Za-z0-9.\\-]+") || Util.fixEmptyAndTrim(value)==null) + if (value.matches("@[A-Za-z0-9.\\-]+") || Util.fixEmptyAndTrim(value) == null) return FormValidation.ok(); else return FormValidation.error(Messages.Mailer_Suffix_Error()); @@ -670,24 +711,29 @@ public FormValidation doCheckDefaultSuffix(@QueryParameter String value) { /** * Send an email to the admin address + * * @throws IOException in case the active jenkins instance cannot be retrieved - * @param smtpHost name of the SMTP server to use for mail sending - * @param adminAddress Jenkins administrator mail address - * @param authentication if set to {@code true} SMTP is used without authentication (username and password) - * @param username plaintext username for SMTP authentication - * @param password secret password for SMTP authentication - * @param useSsl if set to {@code true} SSL is used - * @param useTls if set to {@code true} TLS is used - * @param smtpPort port to use for SMTP transfer - * @param charset charset of the underlying MIME-mail message + * @param smtpHost name of the SMTP server to use for mail sending + * @param adminAddress Jenkins administrator mail address + * @param authentication if set to {@code true} SMTP is used without + * authentication (username and password) + * @param username plaintext username for SMTP authentication + * @param password secret password for SMTP authentication + * @param useSsl if set to {@code true} SSL is used + * @param useTls if set to {@code true} TLS is used + * @param smtpPort port to use for SMTP transfer + * @param charset charset of the underlying MIME-mail message * @param sendTestMailTo mail address to send test mail to - * @return response with http status code depending on the result of the mail sending + * @return response with http status code depending on the result of the mail + * sending */ @RequirePOST public FormValidation doSendTestMail( - @QueryParameter String smtpHost, @QueryParameter String adminAddress, @QueryParameter boolean authentication, + @QueryParameter String smtpHost, @QueryParameter String adminAddress, + @QueryParameter boolean authentication, @QueryParameter String username, @QueryParameter Secret password, - @QueryParameter boolean useSsl, @QueryParameter boolean useTls, @QueryParameter String smtpPort, @QueryParameter String charset, + @QueryParameter boolean useSsl, @QueryParameter boolean useTls, @QueryParameter String smtpPort, + @QueryParameter String charset, @QueryParameter String sendTestMailTo) throws IOException { try { Jenkins.get().checkPermission(Jenkins.MANAGE); @@ -699,25 +745,29 @@ public FormValidation doSendTestMail( return FormValidation.error(Messages.Mailer_InsecureAuthError()); } - MimeMessage msg = new MimeMessage(createSession(smtpHost, smtpPort, useSsl, useTls, username, password)); + MimeMessage msg = new MimeMessage( + createSession(smtpHost, smtpPort, useSsl, useTls, username, password)); msg.setSubject(Messages.Mailer_TestMail_Subject(testEmailCount.incrementAndGet()), charset); - msg.setText(Messages.Mailer_TestMail_Content(testEmailCount.get(), Jenkins.get().getDisplayName()), charset); + msg.setText(Messages.Mailer_TestMail_Content(testEmailCount.get(), Jenkins.get().getDisplayName()), + charset); msg.setFrom(stringToAddress(adminAddress, charset)); if (replyToAddress != null && !replyToAddress.isBlank()) { - msg.setReplyTo(new Address[]{stringToAddress(replyToAddress, charset)}); + msg.setReplyTo(new Address[] { stringToAddress(replyToAddress, charset) }); } msg.setSentDate(new Date()); msg.setRecipient(Message.RecipientType.TO, stringToAddress(sendTestMailTo, charset)); - Transport.send(msg); + Transport.send(msg); return FormValidation.ok(Messages.Mailer_EmailSentSuccessfully()); } catch (MessagingException e) { - return FormValidation.errorWithMarkup("

"+Messages.Mailer_FailedToSendEmail()+"

"+Util.escape(Functions.printThrowable(e))+"
"); + return FormValidation.errorWithMarkup("

" + Messages.Mailer_FailedToSendEmail() + "

"
+                        + Util.escape(Functions.printThrowable(e)) + "
"); } } @RequirePOST - public FormValidation doCheckAuthentication(@QueryParameter boolean authentication, @QueryParameter boolean useSsl, @QueryParameter boolean useTls) { + public FormValidation doCheckAuthentication(@QueryParameter boolean authentication, + @QueryParameter boolean useSsl, @QueryParameter boolean useTls) { Jenkins.get().checkPermission(Jenkins.MANAGE); if (!authentication || useSsl || useTls) { return FormValidation.ok(); @@ -751,16 +801,16 @@ public UserProperty(String emailAddress) { @Exported public String getAddress() { - if(hasExplicitlyConfiguredAddress()) { + if (hasExplicitlyConfiguredAddress()) { return emailAddress; - } + } // try the inference logic return MailAddressResolver.resolve(user); } public String getConfiguredAddress() { - if(hasExplicitlyConfiguredAddress()) { + if (hasExplicitlyConfiguredAddress()) { return emailAddress; } @@ -779,6 +829,7 @@ public String getEmailAddress() { * This method also truncates spaces. It is highly recommended to * use {@link #hasExplicitlyConfiguredAddress()} method to check the * option's existence. + * * @return A trimmed email address. It can be null * @since TODO */ @@ -789,10 +840,11 @@ public String getExplicitlyConfiguredAddress() { /** * Has the user configured a value explicitly (true), or is it inferred (false)? + * * @return {@code true} if there is an email address available. */ public boolean hasExplicitlyConfiguredAddress() { - return Util.fixEmptyAndTrim(emailAddress)!=null; + return Util.fixEmptyAndTrim(emailAddress) != null; } @Extension @@ -807,7 +859,8 @@ public UserProperty newInstance(User user) { } @Override - public UserProperty newInstance(@CheckForNull StaplerRequest2 req, JSONObject formData) throws FormException { + public UserProperty newInstance(@CheckForNull StaplerRequest2 req, JSONObject formData) + throws FormException { return new UserProperty(req != null ? req.getParameter("email.address") : null); } } @@ -815,10 +868,10 @@ public UserProperty newInstance(@CheckForNull StaplerRequest2 req, JSONObject fo /** * Debug probe point to be activated by the scripting console. + * * @deprecated This hack may be removed in future versions */ - @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", - justification = "It may used for debugging purposes. We have to keep it for the sake of the binary copatibility") + @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "It may used for debugging purposes. We have to keep it for the sake of the binary copatibility") @Deprecated public static boolean debug = false; } diff --git a/src/test/java/hudson/tasks/MailerTest.java b/src/test/java/hudson/tasks/MailerTest.java index 10062a98..1f4d79df 100644 --- a/src/test/java/hudson/tasks/MailerTest.java +++ b/src/test/java/hudson/tasks/MailerTest.java @@ -23,26 +23,26 @@ */ package hudson.tasks; -import hudson.Functions; -import hudson.Launcher; -import hudson.model.AbstractBuild; -import hudson.model.AbstractProject; -import hudson.model.BuildListener; -import hudson.model.Descriptor; -import hudson.model.FreeStyleBuild; -import hudson.model.FreeStyleProject; -import hudson.model.Result; -import hudson.model.User; -import hudson.security.ACL; -import hudson.security.ACLContext; -import hudson.slaves.DumbSlave; -import hudson.tasks.Mailer.DescriptorImpl; -import hudson.util.FormValidation; -import hudson.util.Secret; -import jakarta.mail.Address; -import jakarta.mail.internet.InternetAddress; -import jenkins.model.Jenkins; -import jenkins.model.JenkinsLocationConfiguration; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Collection; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicLong; + import org.htmlunit.html.HtmlForm; import org.htmlunit.html.HtmlPage; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; @@ -63,25 +63,27 @@ import org.jvnet.hudson.test.recipes.LocalData; import org.jvnet.mock_javamail.Mailbox; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.util.Collection; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicLong; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNotSame; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assumptions.assumeTrue; +import hudson.Functions; +import hudson.Launcher; +import hudson.model.AbstractBuild; +import hudson.model.AbstractProject; +import hudson.model.BuildListener; +import hudson.model.Descriptor; +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; +import hudson.model.Result; +import hudson.model.User; +import hudson.security.ACL; +import hudson.security.ACLContext; +import hudson.slaves.DumbSlave; +import hudson.tasks.Mailer.DescriptorImpl; +import hudson.util.FormValidation; +import hudson.util.Secret; +import jakarta.mail.Address; +import jakarta.mail.internet.AddressException; +import jakarta.mail.internet.InternetAddress; +import jenkins.model.Jenkins; +import jenkins.model.JenkinsLocationConfiguration; /** * @author Kohsuke Kawaguchi @@ -107,7 +109,8 @@ private static Mailbox getMailbox(String recipient) throws Exception { @BeforeAll static void enableManagePermission() { - // TODO remove when baseline contains https://github.com/jenkinsci/jenkins/pull/23873 + // TODO remove when baseline contains + // https://github.com/jenkinsci/jenkins/pull/23873 Jenkins.MANAGE.setEnabled(true); } @@ -117,7 +120,8 @@ private static Mailbox getEmptyMailbox(String recipient) throws Exception { return inbox; } - private TestProject create(JenkinsRule rule, boolean notifyEveryUnstableBuild, boolean sendToIndividuals) throws Exception { + private TestProject create(JenkinsRule rule, boolean notifyEveryUnstableBuild, boolean sendToIndividuals) + throws Exception { Mailer m = new Mailer(RECIPIENT, notifyEveryUnstableBuild, sendToIndividuals); return new TestProject(rule, m); } @@ -267,7 +271,8 @@ void testGlobalConfigRoundtrip(JenkinsRule rule) throws Exception { @Test void globalConfig(JenkinsRule rule) throws Exception { - assumeTrue(rule.getPluginManager().getPlugin("email-ext") == null, "TODO the form elements for email-ext have the same names"); + assumeTrue(rule.getPluginManager().getPlugin("email-ext") == null, + "TODO the form elements for email-ext have the same names"); WebClient webClient = rule.createWebClient(); HtmlPage cp = webClient.goTo("configure"); @@ -359,37 +364,37 @@ void testHudsonUrlCompatibility(JenkinsRule rule) throws Exception { @Test void testNotifyEveryUnstableBuild(JenkinsRule rule) throws Exception { create(rule, true, false).failure() - .buildAndCheck(1) - .buildAndCheck(1) - .unstable() - .buildAndCheck(1) - .buildAndCheck(1) - .success() - .buildAndCheck(1) /* back to normal mail. */ - .buildAndCheck(0) - .unstable() - .buildAndCheck(1) - .failure() - .buildAndCheck(1); + .buildAndCheck(1) + .buildAndCheck(1) + .unstable() + .buildAndCheck(1) + .buildAndCheck(1) + .success() + .buildAndCheck(1) /* back to normal mail. */ + .buildAndCheck(0) + .unstable() + .buildAndCheck(1) + .failure() + .buildAndCheck(1); } @Test void testNotNotifyEveryUnstableBuild(JenkinsRule rule) throws Exception { create(rule, false, false) - .buildAndCheck(0) - .buildAndCheck(0) - .unstable() - .buildAndCheck(1) - .buildAndCheck(0) - .success() - .buildAndCheck(1) /* back to normal mail. */ - .buildAndCheck(0) - .unstable() - .buildAndCheck(1) - .buildAndCheck(0) - .failure() - .buildAndCheck(1) - .buildAndCheck(1); + .buildAndCheck(0) + .buildAndCheck(0) + .unstable() + .buildAndCheck(1) + .buildAndCheck(0) + .success() + .buildAndCheck(1) /* back to normal mail. */ + .buildAndCheck(0) + .unstable() + .buildAndCheck(1) + .buildAndCheck(0) + .failure() + .buildAndCheck(1) + .buildAndCheck(1); } @SuppressWarnings("deprecation") @@ -420,12 +425,12 @@ void testNotifyCulprits(JenkinsRule rule) throws Exception { @Test void testMessageText(JenkinsRule rule) throws Exception { create(rule, true, false) - .failure() - .buildAndCheckContent() - .unstable() - .buildAndCheckContent() - .success() - .buildAndCheckContent(); + .failure() + .buildAndCheckContent() + .unstable() + .buildAndCheckContent() + .success() + .buildAndCheckContent(); } @Issue("JENKINS-37812") @@ -448,7 +453,8 @@ void testPipelineCompatibility(JenkinsRule rule) throws Exception { "node {\n" + " catchError {error 'oops'}\n" + " step([$class: 'Mailer', recipients: '" + RECIPIENT + "'])\n" - + "}", true)); + + "}", + true)); Mailbox inbox = getMailbox(RECIPIENT); inbox.clear(); @@ -464,12 +470,22 @@ void testPipelineCompatibility(JenkinsRule rule) throws Exception { void testMigrateOldData(JenkinsRule rule) { Mailer.DescriptorImpl descriptor = Mailer.descriptor(); assertNotNull(descriptor, "Mailer can not be found"); - assertEquals("olduser", descriptor.getAuthentication().getUsername(), String.format("Authentication did not migrate properly. Username expected %s but received %s", "olduser", descriptor.getAuthentication().getUsername())); - assertEquals("UTF-8", descriptor.getCharset(), String.format("Charset did not migrate properly. Expected %s but received %s", "UTF-8", descriptor.getCharset())); - assertEquals("@mydomain.com", descriptor.getDefaultSuffix(), String.format("Default suffix did not migrate properly. Expected %s but received %s", "@mydomain.com", descriptor.getDefaultSuffix())); - assertEquals("noreplay@mydomain.com", descriptor.getReplyToAddress(), String.format("ReplayTo address did not migrate properly. Expected %s but received %s", "noreplay@mydomain.com", descriptor.getReplyToAddress())); - assertEquals("old.data.smtp.host", descriptor.getSmtpHost(), String.format("SMTP host did not migrate properly. Expected %s but received %s", "old.data.smtp.host", descriptor.getSmtpHost())); - assertEquals("808080", descriptor.getSmtpPort(), String.format("SMTP port did not migrate properly. Expected %s but received %s", "808080", descriptor.getSmtpPort())); + assertEquals("olduser", descriptor.getAuthentication().getUsername(), + String.format("Authentication did not migrate properly. Username expected %s but received %s", + "olduser", descriptor.getAuthentication().getUsername())); + assertEquals("UTF-8", descriptor.getCharset(), String.format( + "Charset did not migrate properly. Expected %s but received %s", "UTF-8", descriptor.getCharset())); + assertEquals("@mydomain.com", descriptor.getDefaultSuffix(), + String.format("Default suffix did not migrate properly. Expected %s but received %s", "@mydomain.com", + descriptor.getDefaultSuffix())); + assertEquals("noreplay@mydomain.com", descriptor.getReplyToAddress(), + String.format("ReplayTo address did not migrate properly. Expected %s but received %s", + "noreplay@mydomain.com", descriptor.getReplyToAddress())); + assertEquals("old.data.smtp.host", descriptor.getSmtpHost(), + String.format("SMTP host did not migrate properly. Expected %s but received %s", "old.data.smtp.host", + descriptor.getSmtpHost())); + assertEquals("808080", descriptor.getSmtpPort(), String.format( + "SMTP port did not migrate properly. Expected %s but received %s", "808080", descriptor.getSmtpPort())); assertTrue(descriptor.getUseSsl(), "SSL should be used"); } @@ -479,13 +495,12 @@ void managePermissionShouldAccessGlobalConfig(JenkinsRule rule) { final String MANAGER = "manager"; rule.jenkins.setSecurityRealm(rule.createDummySecurityRealm()); rule.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() - // Read access - .grant(Jenkins.READ).everywhere().to(USER) + // Read access + .grant(Jenkins.READ).everywhere().to(USER) - // Read and Manage - .grant(Jenkins.READ).everywhere().to(MANAGER) - .grant(Jenkins.MANAGE).everywhere().to(MANAGER) - ); + // Read and Manage + .grant(Jenkins.READ).everywhere().to(MANAGER) + .grant(Jenkins.MANAGE).everywhere().to(MANAGER)); try (ACLContext c = ACL.as(User.getById(USER, true))) { Collection descriptors = Functions.getSortedDescriptorsForGlobalConfigUnclassified(); @@ -494,8 +509,8 @@ void managePermissionShouldAccessGlobalConfig(JenkinsRule rule) { try (ACLContext c = ACL.as(User.getById(MANAGER, true))) { Collection descriptors = Functions.getSortedDescriptorsForGlobalConfigUnclassified(); - Optional found = - descriptors.stream().filter(descriptor -> descriptor instanceof Mailer.DescriptorImpl).findFirst(); + Optional found = descriptors.stream() + .filter(descriptor -> descriptor instanceof Mailer.DescriptorImpl).findFirst(); assertTrue(found.isPresent(), "Global configuration should be accessible to MANAGE users"); } } @@ -506,8 +521,7 @@ void doCheckSmtpServerShouldThrowExceptionForUserWithoutManagePermissions(Jenkin final String USER = "user"; rule.jenkins.setSecurityRealm(rule.createDummySecurityRealm()); rule.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() - .grant(Jenkins.READ).everywhere().to(USER) - ); + .grant(Jenkins.READ).everywhere().to(USER)); final String expectedErrorMessage = "user is missing the Overall/Manage permission"; try (ACLContext ignored = ACL.as(User.getById(USER, true))) { @@ -524,14 +538,23 @@ void doCheckSmtpServerShouldNotThrowForUserWithManagePermissions(JenkinsRule rul final String MANAGER = "manage"; rule.jenkins.setSecurityRealm(rule.createDummySecurityRealm()); rule.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() - .grant(Jenkins.MANAGE).everywhere().to(MANAGER) - ); + .grant(Jenkins.MANAGE).everywhere().to(MANAGER)); try (ACLContext ignored = ACL.as(User.getById(MANAGER, true))) { Mailer.descriptor().doCheckSmtpHost("domain.com"); } } + @Test + @Issue("JENKINS-68961") + public void emptyAddressShouldFailGracefully() { + AddressException ex = assertThrows( + AddressException.class, + () -> Mailer.stringToAddress("", "UTF-8")); + + assertTrue(ex.getMessage().toLowerCase().contains("empty")); + } + private static final class TestProject { private final JenkinsRule rule; private final FakeChangeLogSCM scm = new FakeChangeLogSCM(); @@ -629,7 +652,8 @@ TestBuild check(int expectedRecipient, int expectedAuthor1, int expectedAuthor2) } void checkContent() throws Exception { - String expectedInMessage = String.format("<%sjob/%s/%d/display/redirect>\n\n", rule.getURL(), this.build.getProject().getName(), this.build.getNumber()); + String expectedInMessage = String.format("<%sjob/%s/%d/display/redirect>\n\n", rule.getURL(), + this.build.getProject().getName(), this.build.getNumber()); String emailContent = getMailbox(RECIPIENT).get(0).getContent().toString(); assertThat(emailContent, containsString(expectedInMessage)); } @@ -645,14 +669,16 @@ private static final class MailerDisconnecting extends Mailer { private final transient JenkinsRule rule; private final transient DumbSlave node; - private MailerDisconnecting(JenkinsRule rule, DumbSlave node, String recipients, boolean notifyEveryUnstableBuild, boolean sendToIndividuals) { + private MailerDisconnecting(JenkinsRule rule, DumbSlave node, String recipients, + boolean notifyEveryUnstableBuild, boolean sendToIndividuals) { super(recipients, notifyEveryUnstableBuild, sendToIndividuals); this.rule = rule; this.node = node; } @Override - public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { + public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) + throws InterruptedException, IOException { try { rule.disconnectSlave(node); } catch (Exception e) {