Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-970: Add AmqpAppender.verifyHostname option #971

Merged
merged 2 commits into from
Apr 8, 2019
Merged

GH-970: Add AmqpAppender.verifyHostname option #971

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3978e52
GH-970: Add `AmqpAppender.verifyHostname` option
artembilan Apr 5, 2019
abe3a0b
* Fix Checkstyle violations
artembilan Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 33 additions & 16 deletions spring-rabbit/src/main/java/org/springframework/amqp/rabbit/logback/AmqpAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.springframework.amqp.rabbit.core.DeclareExchangeConnectionListener;
import org.springframework.amqp.rabbit.core.RabbitAdmin;
import org.springframework.amqp.rabbit.core.RabbitTemplate;
import org.springframework.amqp.utils.JavaUtils;
import org.springframework.core.io.Resource;
import org.springframework.core.io.support.PathMatchingResourcePatternResolver;
import org.springframework.util.StringUtils;
Expand All @@ -64,7 +65,7 @@
import com.rabbitmq.client.ConnectionFactory;

/**
* A Lockback appender that publishes logging events to an AMQP Exchange.
* A Logback appender that publishes logging events to an AMQP Exchange.
* <p>
* A fully-configured AmqpAppender, with every option set to their defaults, would look like this:
*
Expand Down Expand Up @@ -272,6 +273,8 @@ public class AmqpAppender extends AppenderBase<ILoggingEvent> {
*/
private String trustStoreType = "JKS";

private boolean verifyHostname;

/**
* Default content-type of log messages.
*/
Expand Down Expand Up @@ -381,6 +384,25 @@ public void setUseSsl(boolean ssl) {
this.useSsl = ssl;
}

/**
* Enable server hostname verification for TLS connections.
* @param enable false to disable.
* @since 2.1.6
* @see RabbitConnectionFactoryBean#setEnableHostnameVerification(boolean)
*/
public void setVerifyHostname(boolean enable) {
this.verifyHostname = enable;
}

/**
* Return true (default) if TLS hostname verification is enabled.
* @return true (default) if TLS hostname verification is enabled.
* @since 2.1.6
*/
public boolean isVerifyHostname() {
return verifyHostname;
}

public String getSslAlgorithm() {
return this.sslAlgorithm;
}
Expand Down Expand Up @@ -589,7 +611,6 @@ public void setConnectionName(String connectionName) {
/**
* Set additional client connection properties to be added to the rabbit connection,
* with the form {@code key:value[,key:value]...}.
*
* @param clientConnectionProperties the properties.
* @since 1.5.6
*/
Expand Down Expand Up @@ -620,7 +641,7 @@ public void start() {
if (rabbitConnectionFactory != null) {
super.start();
this.routingKeyLayout.setPattern(this.routingKeyLayout.getPattern()
.replaceAll("%property\\{applicationId\\}", this.applicationId));
.replaceAll("%property\\{applicationId}", this.applicationId));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong, curly brackets must be escaped in RegEx.

Isn't there a test for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what my IDEA shows me:

изображение

And yes: it is covered by our tests.
Look into logback-test.xml and log4j2-amqp-appender.xml and search for {applicationId}

this.routingKeyLayout.setContext(getContext());
this.routingKeyLayout.start();
this.locationLayout.setContext(getContext());
Expand All @@ -645,7 +666,6 @@ public void start() {

/**
* Create the {@link ConnectionFactory}.
*
* @return a {@link ConnectionFactory}.
*/
protected ConnectionFactory createRabbitConnectionFactory() {
Expand All @@ -664,21 +684,21 @@ protected ConnectionFactory createRabbitConnectionFactory() {
/**
* Configure the {@link RabbitConnectionFactoryBean}. Sub-classes may override to
* customize the configuration of the bean.
*
* @param factoryBean the {@link RabbitConnectionFactoryBean}.
*/
protected void configureRabbitConnectionFactory(RabbitConnectionFactoryBean factoryBean) {

Optional.ofNullable(this.host).ifPresent(factoryBean::setHost);
Optional.ofNullable(this.port).ifPresent(factoryBean::setPort);
Optional.ofNullable(this.username).ifPresent(factoryBean::setUsername);
Optional.ofNullable(this.password).ifPresent(factoryBean::setPassword);
Optional.ofNullable(this.virtualHost).ifPresent(factoryBean::setVirtualHost);
// overrides all preceding items when set
Optional.ofNullable(this.uri).ifPresent(factoryBean::setUri);
JavaUtils.INSTANCE
.acceptIfNotNull(this.host, factoryBean::setHost)
.acceptIfNotNull(this.port, factoryBean::setPort)
.acceptIfNotNull(this.username, factoryBean::setUsername)
.acceptIfNotNull(this.password, factoryBean::setPassword)
.acceptIfNotNull(this.virtualHost, factoryBean::setVirtualHost)
// overrides all preceding items when set
.acceptIfNotNull(this.uri, factoryBean::setUri);

if (this.useSsl) {
factoryBean.setUseSSL(true);
factoryBean.setEnableHostnameVerification(this.verifyHostname);
if (this.sslAlgorithm != null) {
factoryBean.setSslAlgorithm(this.sslAlgorithm);
}
Expand Down Expand Up @@ -708,7 +728,6 @@ protected void configureRabbitConnectionFactory(RabbitConnectionFactoryBean fact
/**
* Subclasses can override this method to add properties to the connection client
* properties.
*
* @param clientProperties the client properties.
* @since 1.5.6
*/
Expand All @@ -717,7 +736,6 @@ protected void updateConnectionClientProperties(Map<String, Object> clientProper

/**
* Subclasses can override this method to inject a custom queue implementation.
*
* @return the queue to use for queueing logging events before processing them.
* @since 2.0.1
*/
Expand Down Expand Up @@ -773,7 +791,6 @@ else if ("headers".equals(this.exchangeType)) {

/**
* Subclasses may modify the final message before sending.
*
* @param message The message.
* @param event The event.
* @return The modified message.
Expand Down
7 changes: 5 additions & 2 deletions spring-rabbit/src/test/java/org/springframework/amqp/rabbit/logback/AmqpAppenderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
/**
*
* @author Stephen Oakey
* @author Artem Bilan
*
* @since 2.0
*/
Expand Down Expand Up @@ -99,15 +100,17 @@ public void testDefaultSslConfiguration() {
public void testSslConfigurationWithAlgorithm() {
AmqpAppender appender = new AmqpAppender();
appender.setUseSsl(true);
appender.setVerifyHostname(false);
String sslAlgorithm = "TLSv2";
appender.setSslAlgorithm(sslAlgorithm);

RabbitConnectionFactoryBean bean = mock(RabbitConnectionFactoryBean.class);
appender.configureRabbitConnectionFactory(bean);

verifyDefaultHostProperties(bean);
verify(bean).setUseSSL(eq(true));
verify(bean).setSslAlgorithm(eq(sslAlgorithm));
verify(bean).setUseSSL(true);
verify(bean).setSslAlgorithm(sslAlgorithm);
verify(bean).setEnableHostnameVerification(false);
}

@Test
Expand Down