Repository: oozie
Updated Branches:
  refs/heads/master d04b24329 -> 43e3f40b5


OOZIE-2472 Remove infinite socket timeouts in the Oozie email action (harsh)


Project: http://git-wip-us.apache.org/repos/asf/oozie/repo
Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/43e3f40b
Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/43e3f40b
Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/43e3f40b

Branch: refs/heads/master
Commit: 43e3f40b55e0e8fa27289efffcfd5a339fe7bb2a
Parents: d04b243
Author: Harsh J <[email protected]>
Authored: Fri Mar 4 07:17:52 2016 +0530
Committer: Harsh J <[email protected]>
Committed: Fri Mar 4 07:17:52 2016 +0530

----------------------------------------------------------------------
 .../oozie/action/email/EmailActionExecutor.java | 23 ++++---
 core/src/main/resources/oozie-default.xml       | 56 +++++++++++++++++
 .../action/email/TestEmailActionExecutor.java   | 66 +++++++++++++++++---
 .../site/twiki/DG_EmailActionExtension.twiki    |  2 +
 release-log.txt                                 |  1 +
 5 files changed, 130 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/oozie/blob/43e3f40b/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 
b/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
index 2bb4192..902c463 100644
--- a/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
+++ b/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java
@@ -74,6 +74,7 @@ public class EmailActionExecutor extends ActionExecutor {
     public static final String EMAIL_SMTP_USER = CONF_PREFIX + "smtp.username";
     public static final String EMAIL_SMTP_PASS = CONF_PREFIX + "smtp.password";
     public static final String EMAIL_SMTP_FROM = CONF_PREFIX + "from.address";
+    public static final String EMAIL_SMTP_SOCKET_TIMEOUT_MS = CONF_PREFIX + 
"smtp.socket.timeout.ms";
     public static final String EMAIL_ATTACHMENT_ENABLED = CONF_PREFIX + 
"attachment.enabled";
 
     private final static String TO = "to";
@@ -178,22 +179,28 @@ public class EmailActionExecutor extends ActionExecutor {
     public void email(String[] to, String[] cc, String[] bcc, String subject, 
String body, String[] attachments,
                       String contentType, String user) throws 
ActionExecutorException {
         // Get mailing server details.
-        String smtpHost = getOozieConf().get(EMAIL_SMTP_HOST, "localhost");
-        String smtpPort = getOozieConf().get(EMAIL_SMTP_PORT, "25");
-        Boolean smtpAuth = getOozieConf().getBoolean(EMAIL_SMTP_AUTH, false);
-        String smtpUser = getOozieConf().get(EMAIL_SMTP_USER, "");
+        String smtpHost = ConfigurationService.get(EMAIL_SMTP_HOST);
+        Integer smtpPortInt = ConfigurationService.getInt(EMAIL_SMTP_PORT);
+        Boolean smtpAuthBool = 
ConfigurationService.getBoolean(EMAIL_SMTP_AUTH);
+        String smtpUser = ConfigurationService.get(EMAIL_SMTP_USER);
         String smtpPassword = 
ConfigurationService.getPassword(EMAIL_SMTP_PASS, "");
-        String fromAddr = getOozieConf().get(EMAIL_SMTP_FROM, 
"oozie@localhost");
+        String fromAddr = ConfigurationService.get(EMAIL_SMTP_FROM);
+        Integer timeoutMillisInt = 
ConfigurationService.getInt(EMAIL_SMTP_SOCKET_TIMEOUT_MS);
 
         Properties properties = new Properties();
         properties.setProperty("mail.smtp.host", smtpHost);
-        properties.setProperty("mail.smtp.port", smtpPort);
-        properties.setProperty("mail.smtp.auth", smtpAuth.toString());
+        properties.setProperty("mail.smtp.port", smtpPortInt.toString());
+        properties.setProperty("mail.smtp.auth", smtpAuthBool.toString());
+
+        // Apply sensible timeouts, as defaults are infinite. See 
https://s.apache.org/javax-mail-timeouts
+        properties.setProperty("mail.smtp.connectiontimeout", 
timeoutMillisInt.toString());
+        properties.setProperty("mail.smtp.timeout", 
timeoutMillisInt.toString());
+        properties.setProperty("mail.smtp.writetimeout", 
timeoutMillisInt.toString());
 
         Session session;
         // Do not use default instance (i.e. Session.getDefaultInstance)
         // (cause it may lead to issues when used second time).
-        if (!smtpAuth) {
+        if (!smtpAuthBool) {
             session = Session.getInstance(properties);
         } else {
             session = Session.getInstance(properties, new 
JavaMailAuthenticator(smtpUser, smtpPassword));

http://git-wip-us.apache.org/repos/asf/oozie/blob/43e3f40b/core/src/main/resources/oozie-default.xml
----------------------------------------------------------------------
diff --git a/core/src/main/resources/oozie-default.xml 
b/core/src/main/resources/oozie-default.xml
index 3ff7320..d2360fc 100644
--- a/core/src/main/resources/oozie-default.xml
+++ b/core/src/main/resources/oozie-default.xml
@@ -2659,6 +2659,62 @@
     </property>
 
     <property>
+      <name>oozie.email.smtp.host</name>
+      <value>localhost</value>
+      <description>
+          The host where the email action may find the SMTP server.
+      </description>
+    </property>
+
+    <property>
+      <name>oozie.email.smtp.port</name>
+      <value>25</value>
+      <description>
+          The port to connect to for the SMTP server, for email actions.
+      </description>
+    </property>
+
+    <property>
+      <name>oozie.email.smtp.auth</name>
+      <value>false</value>
+      <description>
+          Boolean property that toggles if authentication is to be done or not 
when using email actions.
+      </description>
+    </property>
+
+    <property>
+      <name>oozie.email.smtp.username</name>
+      <value></value>
+      <description>
+          If authentication is enabled for email actions, the username to 
login as (to the SMTP server).
+      </description>
+    </property>
+
+    <property>
+      <name>oozie.email.smtp.password</name>
+      <value></value>
+      <description>
+          If authentication is enabled for email actions, the password to 
login with (to the SMTP server).
+      </description>
+    </property>
+
+    <property>
+      <name>oozie.email.from.address</name>
+      <value>oozie@localhost</value>
+      <description>
+          The from address to be used for mailing all emails done via the 
email action.
+      </description>
+    </property>
+
+    <property>
+      <name>oozie.email.smtp.socket.timeout.ms</name>
+      <value>10000</value>
+      <description>
+          The timeout to apply over all SMTP server socket operations done 
during the email action.
+      </description>
+    </property>
+
+    <property>
         <name>oozie.actions.default.name-node</name>
         <value> </value>
         <description>

http://git-wip-us.apache.org/repos/asf/oozie/blob/43e3f40b/core/src/test/java/org/apache/oozie/action/email/TestEmailActionExecutor.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/org/apache/oozie/action/email/TestEmailActionExecutor.java 
b/core/src/test/java/org/apache/oozie/action/email/TestEmailActionExecutor.java
index cc378fc..354176e 100644
--- 
a/core/src/test/java/org/apache/oozie/action/email/TestEmailActionExecutor.java
+++ 
b/core/src/test/java/org/apache/oozie/action/email/TestEmailActionExecutor.java
@@ -23,6 +23,9 @@ import java.io.File;
 import java.io.FileWriter;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.net.Socket;
+import java.net.SocketTimeoutException;
+import java.net.ServerSocket;
 import java.util.regex.Pattern;
 
 import javax.mail.BodyPart;
@@ -65,18 +68,17 @@ public class TestEmailActionExecutor extends 
ActionExecutorTestCase {
 
     private Context createNormalContext(String actionXml) throws Exception {
         EmailActionExecutor ae = new EmailActionExecutor();
-
-        Services.get().getConf().setInt("oozie.email.smtp.port", 
server.getSmtp().getPort());
+        
Services.get().get(ConfigurationService.class).getConf().setInt("oozie.email.smtp.port",
 server.getSmtp().getPort());
         // Use default host 'localhost'. Hence, do not set the smtp host.
-        // Services.get().getConf().set("oozie.email.smtp.host", "localhost");
+        // 
Services.get().get(ConfigurationService.class).getConf().set("oozie.email.smtp.host",
 "localhost");
         // Use default from address, 'oozie@localhost'.
         // Hence, do not set the from address conf.
-        // Services.get().getConf().set("oozie.email.from.address", 
"oozie@localhost");
+        // 
Services.get().get(ConfigurationService.class).getConf().set("oozie.email.from.address",
 "oozie@localhost");
 
         // Disable auth tests by default.
-        Services.get().getConf().setBoolean("oozie.email.smtp.auth", false);
-        Services.get().getConf().set("oozie.email.smtp.username", "");
-        Services.get().getConf().set("oozie.email.smtp.password", "");
+        
Services.get().get(ConfigurationService.class).getConf().setBoolean("oozie.email.smtp.auth",
 false);
+        
Services.get().get(ConfigurationService.class).getConf().set("oozie.email.smtp.username",
 "");
+        
Services.get().get(ConfigurationService.class).getConf().set("oozie.email.smtp.password",
 "");
 
         XConfiguration protoConf = new XConfiguration();
         protoConf.set(WorkflowAppService.HADOOP_USER, getTestUser());
@@ -94,9 +96,9 @@ public class TestEmailActionExecutor extends 
ActionExecutorTestCase {
         Context ctx = createNormalContext(actionXml);
 
         // Override and enable auth.
-        Services.get().getConf().setBoolean("oozie.email.smtp.auth", true);
-        Services.get().getConf().set("oozie.email.smtp.username", 
"oozie@localhost");
-        Services.get().getConf().set("oozie.email.smtp.password", "oozie");
+        
Services.get().get(ConfigurationService.class).getConf().setBoolean("oozie.email.smtp.auth",
 true);
+        
Services.get().get(ConfigurationService.class).getConf().set("oozie.email.smtp.username",
 "oozie@localhost");
+        
Services.get().get(ConfigurationService.class).getConf().set("oozie.email.smtp.password",
 "oozie");
         return ctx;
     }
 
@@ -172,6 +174,50 @@ public class TestEmailActionExecutor extends 
ActionExecutorTestCase {
         checkEmail(server.getReceivedMessages()[0], true, true);
     }
 
+    public void testServerTimeouts() throws Exception {
+        final ServerSocket srvSocket = new ServerSocket(0);
+        int srvPort = srvSocket.getLocalPort();
+        Thread serverThread = new Thread() {
+            @Override
+            public void run() {
+                try {
+                    Socket clientSocket = srvSocket.accept();
+                    // Sleep 1s (timeout applied on client is 0.1s)
+                    Thread.sleep(1000);
+                    clientSocket.getOutputStream().write(0);
+                    clientSocket.close();
+                } catch (Exception e) {
+                    e.printStackTrace();
+                }
+            }
+        };
+        serverThread.setDaemon(true);
+        try {
+            serverThread.start();
+            EmailActionExecutor email = new EmailActionExecutor();
+            Context ctx = createNormalContext("email-action");
+            
Services.get().get(ConfigurationService.class).getConf().setInt("oozie.email.smtp.port",
 srvPort);
+            // Apply a 0.1s timeout to induce a very quick "Read timed out" 
error
+            
Services.get().get(ConfigurationService.class).getConf().setInt("oozie.email.smtp.socket.timeout.ms",
 100);
+            try {
+              email.validateAndMail(ctx, prepareEmailElement(false, false));
+              fail("Should have failed with a socket timeout error!");
+            } catch (Exception e) {
+              Throwable rootCause = e;
+              while (rootCause.getCause() != null) {
+                rootCause = rootCause.getCause();
+              }
+              assertTrue("Expected exception type to be a 
SocketTimeoutException, but received: " + rootCause,
+                  rootCause instanceof SocketTimeoutException);
+              assertTrue("Expected error to be that of a socket read timeout, 
but got: " + rootCause.getMessage(),
+                  rootCause.getMessage().contains("Read timed out"));
+            }
+        } finally {
+            serverThread.interrupt();
+            srvSocket.close();
+        }
+    }
+
     public void testValidation() throws Exception {
         EmailActionExecutor email = new EmailActionExecutor();
 

http://git-wip-us.apache.org/repos/asf/oozie/blob/43e3f40b/docs/src/site/twiki/DG_EmailActionExtension.twiki
----------------------------------------------------------------------
diff --git a/docs/src/site/twiki/DG_EmailActionExtension.twiki 
b/docs/src/site/twiki/DG_EmailActionExtension.twiki
index 7319ac1..588a413 100644
--- a/docs/src/site/twiki/DG_EmailActionExtension.twiki
+++ b/docs/src/site/twiki/DG_EmailActionExtension.twiki
@@ -63,6 +63,8 @@ it looks for:
 =oozie.email.smtp.auth= - Boolean property that toggles if authentication is 
to be done or not. (false by default).
 =oozie.email.smtp.username= - If authentication is enabled, the username to 
login as (empty by default).
 =oozie.email.smtp.password= - If authentication is enabled, the username's 
password (empty by default).
+=oozie.email.attachment.enabled= - Boolean property that toggles if configured 
attachments are to be placed into the emails. (false by default).
+=oozie.email.smtp.socket.timeout.ms= - The timeout to apply over all SMTP 
server socket operations (10000ms by default).
 
 *Example:*
 

http://git-wip-us.apache.org/repos/asf/oozie/blob/43e3f40b/release-log.txt
----------------------------------------------------------------------
diff --git a/release-log.txt b/release-log.txt
index a48754d..838eafa 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
 -- Oozie 4.3.0 release (trunk - unreleased)
 
+OOZIE-2470 Remove infinite socket timeouts in the Oozie email action (harsh)
 OOZIE-2246 CoordinatorInputCheckCommand does not behave properly when har file 
is one of data dependency and doesn't exist (satishsaley via puru)
 OOZIE-2446 Job does not fail during submission if non existent credential is 
specified (satishsaley via puru)
 OOZIE-2283 Documentation should not say that System.exit is not allowed in 
Java Action (eeeva via rkanter)

Reply via email to