ramanathan1504 commented on code in PR #4304:
URL: https://github.com/apache/logging-log4j2/pull/4304#discussion_r4065059697
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/SmtpAppenderTest.java:
##########
@@ -108,6 +109,52 @@ void testMessageFactorySetSubject() throws
MessagingException {
assertEquals(subject, builder.build().getSubject());
}
+ @Test
+ @SuppressWarnings("deprecation")
+ void testCreateAppenderForwardsMailAttributes() {
+ final SmtpAppender appender = SmtpAppender.createAppender(
+ new DefaultConfiguration(),
+ "Test",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "Subject Pattern %m",
+ "smtp",
Review Comment:
`"smtp"` is the builder default, so this does not show that `smtpProtocol`
is forwarded.
```suggestion
"smtps",
```
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/SmtpAppenderTest.java:
##########
@@ -108,6 +109,52 @@ void testMessageFactorySetSubject() throws
MessagingException {
assertEquals(subject, builder.build().getSubject());
}
+ @Test
+ @SuppressWarnings("deprecation")
+ void testCreateAppenderForwardsMailAttributes() {
Review Comment:
The password check below needs reflection, which throws checked exceptions.
```suggestion
void testCreateAppenderForwardsMailAttributes() throws Exception {
```
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/SmtpAppenderTest.java:
##########
@@ -108,6 +109,52 @@ void testMessageFactorySetSubject() throws
MessagingException {
assertEquals(subject, builder.build().getSubject());
}
+ @Test
+ @SuppressWarnings("deprecation")
+ void testCreateAppenderForwardsMailAttributes() {
+ final SmtpAppender appender = SmtpAppender.createAppender(
+ new DefaultConfiguration(),
+ "Test",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "Subject Pattern %m",
+ "smtp",
+ HOST,
+ "4711",
+ "username",
+ "password",
+ "false",
+ "3",
+ null,
+ null,
+ null);
+ assertNotNull(appender);
+ assertEquals("Test", appender.getName());
+
+ // `MailManager` names encode the mail attributes, so an equal name
means every attribute was forwarded.
+ final SmtpAppender expected = SmtpAppender.newBuilder()
+ .setName("Test")
+ .setTo("[email protected]")
+ .setCc("[email protected]")
+ .setBcc("[email protected]")
+ .setFrom("[email protected]")
+ .setReplyTo("[email protected]")
+ .setSubject("Subject Pattern %m")
+ .setSmtpProtocol("smtp")
+ .setSmtpHost(HOST)
+ .setSmtpPort(4711)
+ .setSmtpUsername("username")
+ .setSmtpPassword("password")
+ .setSmtpDebug(false)
+ .setBufferSize(3)
+ .build();
+ assertNotNull(expected);
+ assertEquals(expected.getManager().getName(),
appender.getManager().getName());
Review Comment:
This fails if `.setSmtpPassword(smtpPassword)` is dropped from
`createAppender`.
```suggestion
assertEquals(expected.getManager().getName(),
appender.getManager().getName());
final java.lang.reflect.Field field =
SmtpManager.class.getDeclaredField("session");
field.setAccessible(true);
final javax.mail.Session session = (javax.mail.Session)
field.get(appender.getManager());
assertEquals("password", session.requestPasswordAuthentication(null,
0, "smtps", null, null).getPassword());
```
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/SmtpAppenderTest.java:
##########
@@ -108,6 +109,52 @@ void testMessageFactorySetSubject() throws
MessagingException {
assertEquals(subject, builder.build().getSubject());
}
+ @Test
+ @SuppressWarnings("deprecation")
+ void testCreateAppenderForwardsMailAttributes() {
+ final SmtpAppender appender = SmtpAppender.createAppender(
+ new DefaultConfiguration(),
+ "Test",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "Subject Pattern %m",
+ "smtp",
+ HOST,
+ "4711",
+ "username",
+ "password",
+ "false",
+ "3",
+ null,
+ null,
+ null);
+ assertNotNull(appender);
+ assertEquals("Test", appender.getName());
+
+ // `MailManager` names encode the mail attributes, so an equal name
means every attribute was forwarded.
Review Comment:
The manager name has no password in it, so an equal name does not cover
`smtpPassword`.
```suggestion
// `MailManager` names encode every mail attribute except the
password, which is checked on the session below.
```
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/SmtpAppenderTest.java:
##########
@@ -108,6 +109,52 @@ void testMessageFactorySetSubject() throws
MessagingException {
assertEquals(subject, builder.build().getSubject());
}
+ @Test
+ @SuppressWarnings("deprecation")
+ void testCreateAppenderForwardsMailAttributes() {
+ final SmtpAppender appender = SmtpAppender.createAppender(
+ new DefaultConfiguration(),
+ "Test",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "[email protected]",
+ "Subject Pattern %m",
+ "smtp",
+ HOST,
+ "4711",
+ "username",
+ "password",
+ "false",
+ "3",
+ null,
+ null,
+ null);
+ assertNotNull(appender);
+ assertEquals("Test", appender.getName());
+
+ // `MailManager` names encode the mail attributes, so an equal name
means every attribute was forwarded.
+ final SmtpAppender expected = SmtpAppender.newBuilder()
+ .setName("Test")
+ .setTo("[email protected]")
+ .setCc("[email protected]")
+ .setBcc("[email protected]")
+ .setFrom("[email protected]")
+ .setReplyTo("[email protected]")
+ .setSubject("Subject Pattern %m")
+ .setSmtpProtocol("smtp")
Review Comment:
```suggestion
.setSmtpProtocol("smtps")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
