You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "florentos17 (via GitHub)" <gi...@apache.org> on 2024/10/11 08:17:56 UTC
[PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
florentos17 opened a new pull request, #2448:
URL: https://github.com/apache/james-project/pull/2448
i am tackling [this issue](https://github.com/linagora/james-project/issues/5286)
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1799400356
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,108 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.smtpserver.fastfail.MaxRcptHandler;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+import com.github.fge.lambdas.Throwing;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @BeforeEach
+ public void createJamesServer(@TempDir File temporaryFolder) throws Exception {
+ SmtpConfiguration.Builder smtpConfiguration = SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .addHook(MaxRcptHandler.class.getName(),
+ Map.of("maxRcptCount", DEFAULT_MAX_RCPT.toString()));
+
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+ IntStream.range(1, DEFAULT_MAX_RCPT + 2).forEach(Throwing.intConsumer((
Review Comment:
```suggestion
IntStream.range(0, DEFAULT_MAX_RCPT + 1).forEach(Throwing.intConsumer((
```
Zero based index is the standard.
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1798894280
##########
server/mailet/integration-testing/src/main/resources/smtpserver.xml:
##########
@@ -87,7 +87,11 @@
<handler class="org.apache.james.smtpserver.fastfail.ValidRcptHandler"/>
<handler class="org.apache.james.smtpserver.CoreCmdHandlerLoader"/>
{{#hooks}}
- <handler class="{{hookFqcn}}"/>
+ <handler class="{{hookFqcn}}">
+ {{#hookConfigAsXML}}
+ <MaxRcptCount>{{value}}</MaxRcptCount>
Review Comment:
The key is not necessarily maxRcptCount but needs to be taken from the key of the map.
Please write unit tests for this class using an entry the is not MaxRcptCount...
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1798894966
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,113 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.smtpserver.fastfail.MaxRcptHandler;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+import com.github.fge.lambdas.Throwing;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ private void createJamesServer(File temporaryFolder) throws Exception {
+ SmtpConfiguration.Builder smtpConfiguration = SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .addHook(MaxRcptHandler.class.getName(),
+ Map.of("maxRcptCount", DEFAULT_MAX_RCPT.toString()));
+
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+ IntStream.range(1, DEFAULT_MAX_RCPT + 1).forEach(Throwing.intConsumer((
+ i -> dataProbe.addUser("recipient" + i + "@" + DEFAULT_DOMAIN, PASSWORD))));
+ }
+
+ @AfterEach
+ void tearDown() {
+ if (jamesServer != null) {
+ jamesServer.shutdown();
+ }
+ }
+
+ @Disabled("this test wont pass just yet as the tested feature is still in development")
+ @Test
+ void messageShouldNotBeAcceptedWhenMaxRcptHandlerExceeded(@TempDir File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder);
+
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT + 1), "message"))
+ .isEqualTo(new SMTPSendingException(SmtpSendingStep.RCPT, "452 4.5.3 Requested action not taken: max recipients reached\n"));
+ }
+
+ @Disabled("this test wont pass just yet as the tested feature is still in development")
+ @Test
+ void messageShouldBeAcceptedWhenMaxRcptHandlerWithinLimit(@TempDir File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder);
Review Comment:
Move this is a @BeforeEach ?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1796646884
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,106 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.*;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT_HANDLER = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ private void createJamesServer(File temporaryFolder, SmtpConfiguration.Builder smtpConfiguration) throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+
+ for (int i = 1; i <= DEFAULT_MAX_RCPT_HANDLER + 1 ; i++) {
Review Comment:
IntStream.range(0, n).foreach(...)
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #2448:
URL: https://github.com/apache/james-project/pull/2448
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1796645266
##########
server/mailet/integration-testing/src/main/java/org/apache/james/mailets/configuration/SmtpConfiguration.java:
##########
@@ -58,13 +59,15 @@ public static class Builder {
private Optional<SMTPConfiguration.SenderVerificationMode> verifyIndentity;
private Optional<Boolean> bracketEnforcement;
private Optional<String> authorizedAddresses;
+ private Optional<Integer> maxRcptHandler;
Review Comment:
:-1: we shall configure this deirectly through the handle, not as a global server configuration
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "florentos17 (via GitHub)" <gi...@apache.org>.
florentos17 commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1798889692
##########
server/apps/memory-app/src/test/java/org/apache/james/DKIMHookIntegrationTest.java:
##########
@@ -65,7 +65,7 @@ void shouldRejectCheckedDomainWhenNotSigned(GuiceJamesServer guiceJamesServer) t
server.connect(new InetSocketAddress(LOCALHOST_IP, guiceJamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue()));
readBytes(server);
- server.write(ByteBuffer.wrap(("EHLO " + DOMAIN.asString() + "\r\n").getBytes(StandardCharsets.UTF_8)));
+ server.write(ByteBuffer.wrap(("HELO " + DOMAIN.asString() + "\r\n").getBytes(StandardCharsets.UTF_8)));
Review Comment:
oops i did not know enhanced HELO, i thought it was a typo
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1796647523
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,106 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.*;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT_HANDLER = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ private void createJamesServer(File temporaryFolder, SmtpConfiguration.Builder smtpConfiguration) throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+
+ for (int i = 1; i <= DEFAULT_MAX_RCPT_HANDLER + 1 ; i++) {
+ dataProbe.addUser("recipient" + i + "@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+ }
+
+ @AfterEach
+ void tearDown() {
+ if (jamesServer != null) {
+ jamesServer.shutdown();
+ }
+ }
+
+ @Test
+ void messageShouldNotBeAcceptedWhenMaxRcptHandlerExceeded(@TempDir File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .withMaxRcptHandler(DEFAULT_MAX_RCPT_HANDLER));
Review Comment:
Put this common bit in the setup?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1796651088
##########
server/mailet/integration-testing/src/main/java/org/apache/james/mailets/configuration/SmtpConfiguration.java:
##########
@@ -58,13 +59,15 @@ public static class Builder {
private Optional<SMTPConfiguration.SenderVerificationMode> verifyIndentity;
private Optional<Boolean> bracketEnforcement;
private Optional<String> authorizedAddresses;
+ private Optional<Integer> maxRcptHandler;
Review Comment:
I wonna write:
```
<smtpserver enabled="true">
<jmxName>smtpserver-global</jmxName>
<bind>0.0.0.0:0</bind>
<connectionBacklog>200</connectionBacklog>
<tls socketTLS="false" startTLS="false">
<keystore>file://conf/keystore</keystore>
<secret>james72laBalle</secret>
<provider>org.bouncycastle.jce.provider.BouncyCastleProvider</provider>
<algorithm>SunX509</algorithm>
</tls>
<connectiontimeout>360</connectiontimeout>
<connectionLimit>0</connectionLimit>
<connectionLimitPerIP>0</connectionLimitPerIP>
<authRequired>{{authRequired}}</authRequired>
<auth>
<requireSSL>false</requireSSL>
<plainAuthEnabled>true</plainAuthEnabled>
</auth>
<verifyIdentity>{{verifyIdentity}}</verifyIdentity>
<maxmessagesize>{{maxmessagesize}}</maxmessagesize>
<addressBracketsEnforcement>{{bracketEnforcement}}</addressBracketsEnforcement>
<smtpGreeting>Apache JAMES awesome SMTP Server</smtpGreeting>
<handlerchain>
<handler class="org.apache.james.smtpserver.fastfail.ValidRcptHandler"/>
<handler class="org.apache.james.smtpserver.CoreCmdHandlerLoader"/>
<handler class="org.apache.james.smtpserver.MaxRcptHander">
<maxRcptCount>42</maxRcptCount>
</handler>
</handlerchain>
<gracefulShutdown>false</gracefulShutdown>
</smtpserver>
```
CF DKIMHook
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1798891896
##########
server/mailet/integration-testing/src/main/java/org/apache/james/mailets/configuration/SmtpConfiguration.java:
##########
@@ -155,7 +174,17 @@ public String serializeAsXml() throws IOException {
scopes.put("verifyIdentity", verifyIndentity.toString());
scopes.put("maxmessagesize", maxMessageSize);
scopes.put("bracketEnforcement", bracketEnforcement);
- scopes.put("hooks", addittionalHooks);
+
+ List<Map<String, Object>> additionalHooksWithConfig = additionalHooks.stream()
+ .map(hook -> {
+ Map<String, Object> hookScope = new HashMap<>();
+ hookScope.put("hookFqcn", hook.hookFqcn);
+ hookScope.put("hookConfigAsXML", hook.hookConfig.entrySet());
+ return hookScope;
+ })
+ .collect(Collectors.toList());
Review Comment:
```suggestion
.collect(ImmutableList.toImmutableList());
```
(Immutable collections for the <3 )
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1798891169
##########
server/mailet/integration-testing/src/main/java/org/apache/james/mailets/configuration/SmtpConfiguration.java:
##########
@@ -155,7 +174,17 @@ public String serializeAsXml() throws IOException {
scopes.put("verifyIdentity", verifyIndentity.toString());
scopes.put("maxmessagesize", maxMessageSize);
scopes.put("bracketEnforcement", bracketEnforcement);
- scopes.put("hooks", addittionalHooks);
+
+ List<Map<String, Object>> additionalHooksWithConfig = additionalHooks.stream()
+ .map(hook -> {
+ Map<String, Object> hookScope = new HashMap<>();
+ hookScope.put("hookFqcn", hook.hookFqcn);
+ hookScope.put("hookConfigAsXML", hook.hookConfig.entrySet());
+ return hookScope;
+ })
Review Comment:
Could we extract this as a method of `HookConfigurationEntry` so that we could just write `.map(HookConfigurationEntry::asMustacheScopes)` ?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1796646612
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,106 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.*;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT_HANDLER = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ private void createJamesServer(File temporaryFolder, SmtpConfiguration.Builder smtpConfiguration) throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+
+ for (int i = 1; i <= DEFAULT_MAX_RCPT_HANDLER + 1 ; i++) {
+ dataProbe.addUser("recipient" + i + "@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+ }
+
+ @AfterEach
+ void tearDown() {
+ if (jamesServer != null) {
+ jamesServer.shutdown();
+ }
+ }
+
+ @Test
+ void messageShouldNotBeAcceptedWhenMaxRcptHandlerExceeded(@TempDir File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .withMaxRcptHandler(DEFAULT_MAX_RCPT_HANDLER));
+
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT_HANDLER + 1), "message"))
+ .isEqualTo(new SMTPSendingException(SmtpSendingStep.RCPT, "some error message\n"));
+ }
+
+ @Test
+ void messageShouldBeAcceptedWhenMaxRcptHandlerWithinLimit(@TempDir File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .withMaxRcptHandler(DEFAULT_MAX_RCPT_HANDLER));
+
+ assertDoesNotThrow(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT_HANDLER - 1),"message"));
+ }
+
+ private List<String> getRecipients(Integer n) {
+ List<String> recipients = new ArrayList<>();
+ for (int i = 1; i <= n; i++) {
+ recipients.add("recipient" + i + "@" + DEFAULT_DOMAIN);
+ }
+ return recipients;
+ }
Review Comment:
IntStream.range(0, n).foreach(...)
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1796648147
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,106 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.*;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT_HANDLER = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ private void createJamesServer(File temporaryFolder, SmtpConfiguration.Builder smtpConfiguration) throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+
+ for (int i = 1; i <= DEFAULT_MAX_RCPT_HANDLER + 1 ; i++) {
+ dataProbe.addUser("recipient" + i + "@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+ }
+
+ @AfterEach
+ void tearDown() {
+ if (jamesServer != null) {
+ jamesServer.shutdown();
+ }
+ }
+
+ @Test
+ void messageShouldNotBeAcceptedWhenMaxRcptHandlerExceeded(@TempDir File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .withMaxRcptHandler(DEFAULT_MAX_RCPT_HANDLER));
+
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT_HANDLER + 1), "message"))
+ .isEqualTo(new SMTPSendingException(SmtpSendingStep.RCPT, "some error message\n"));
+ }
+
+ @Test
+ void messageShouldBeAcceptedWhenMaxRcptHandlerWithinLimit(@TempDir File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .withMaxRcptHandler(DEFAULT_MAX_RCPT_HANDLER));
+
+ assertDoesNotThrow(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT_HANDLER - 1),"message"));
Review Comment:
```suggestion
.sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT_HANDLER),"message"));
```
Let's test exactly at the limit
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1799401069
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,108 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.smtpserver.fastfail.MaxRcptHandler;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+import com.github.fge.lambdas.Throwing;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @BeforeEach
+ public void createJamesServer(@TempDir File temporaryFolder) throws Exception {
+ SmtpConfiguration.Builder smtpConfiguration = SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .addHook(MaxRcptHandler.class.getName(),
+ Map.of("maxRcptCount", DEFAULT_MAX_RCPT.toString()));
+
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+ IntStream.range(1, DEFAULT_MAX_RCPT + 2).forEach(Throwing.intConsumer((
+ i -> dataProbe.addUser("recipient" + i + "@" + DEFAULT_DOMAIN, PASSWORD))));
+ }
+
+ @AfterEach
+ void tearDown() {
+ if (jamesServer != null) {
+ jamesServer.shutdown();
+ }
+ }
+
+ @Test
+ void messageShouldNotBeAcceptedWhenMaxRcptHandlerExceeded() throws Exception {
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT + 1), "message"))
+ .isEqualTo(new SMTPSendingException(SmtpSendingStep.RCPT, "452 4.5.3 Requested action not taken: max recipients reached\n"));
+ }
+
+ @Test
+ void messageShouldBeAcceptedWhenMaxRcptHandlerWithinLimit() throws Exception {
+ assertDoesNotThrow(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT),"message"));
Review Comment:
```suggestion
.sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT), "message"));
```
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1798888517
##########
server/apps/memory-app/src/test/java/org/apache/james/DKIMHookIntegrationTest.java:
##########
@@ -65,7 +65,7 @@ void shouldRejectCheckedDomainWhenNotSigned(GuiceJamesServer guiceJamesServer) t
server.connect(new InetSocketAddress(LOCALHOST_IP, guiceJamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue()));
readBytes(server);
- server.write(ByteBuffer.wrap(("EHLO " + DOMAIN.asString() + "\r\n").getBytes(StandardCharsets.UTF_8)));
+ server.write(ByteBuffer.wrap(("HELO " + DOMAIN.asString() + "\r\n").getBytes(StandardCharsets.UTF_8)));
Review Comment:
Why this change?
Something against enhanced HELO?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "florentos17 (via GitHub)" <gi...@apache.org>.
florentos17 commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1798911733
##########
server/mailet/integration-testing/src/main/resources/smtpserver.xml:
##########
@@ -87,7 +87,11 @@
<handler class="org.apache.james.smtpserver.fastfail.ValidRcptHandler"/>
<handler class="org.apache.james.smtpserver.CoreCmdHandlerLoader"/>
{{#hooks}}
- <handler class="{{hookFqcn}}"/>
+ <handler class="{{hookFqcn}}">
+ {{#hookConfigAsXML}}
+ <MaxRcptCount>{{value}}</MaxRcptCount>
Review Comment:
yes of course ! i left it like that temporarily because `<{{key}}>{{value}}</{{key}}>` did not seem to work at the time, but i agree with you
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1799401476
##########
server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpMaxRcptHandlerTest.java:
##########
@@ -0,0 +1,108 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.smtp;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+
+import java.io.File;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import org.apache.james.mailets.TemporaryJamesServer;
+import org.apache.james.mailets.configuration.SmtpConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.smtpserver.fastfail.MaxRcptHandler;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SMTPSendingException;
+import org.apache.james.utils.SmtpSendingStep;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+
+import com.github.fge.lambdas.Throwing;
+
+class SmtpMaxRcptHandlerTest {
+ private static final String USER = "user@" + DEFAULT_DOMAIN;
+ private static final Integer DEFAULT_MAX_RCPT = 50;
+
+ @RegisterExtension
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @BeforeEach
+ public void createJamesServer(@TempDir File temporaryFolder) throws Exception {
+ SmtpConfiguration.Builder smtpConfiguration = SmtpConfiguration.builder()
+ .doNotVerifyIdentity()
+ .addHook(MaxRcptHandler.class.getName(),
+ Map.of("maxRcptCount", DEFAULT_MAX_RCPT.toString()));
+
+ jamesServer = TemporaryJamesServer.builder()
+ .withSmtpConfiguration(smtpConfiguration)
+ .build(temporaryFolder);
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser(USER, PASSWORD);
+ IntStream.range(1, DEFAULT_MAX_RCPT + 2).forEach(Throwing.intConsumer((
+ i -> dataProbe.addUser("recipient" + i + "@" + DEFAULT_DOMAIN, PASSWORD))));
+ }
+
+ @AfterEach
+ void tearDown() {
+ if (jamesServer != null) {
+ jamesServer.shutdown();
+ }
+ }
+
+ @Test
+ void messageShouldNotBeAcceptedWhenMaxRcptHandlerExceeded() throws Exception {
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT + 1), "message"))
+ .isEqualTo(new SMTPSendingException(SmtpSendingStep.RCPT, "452 4.5.3 Requested action not taken: max recipients reached\n"));
+ }
+
+ @Test
+ void messageShouldBeAcceptedWhenMaxRcptHandlerWithinLimit() throws Exception {
+ assertDoesNotThrow(() ->
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, getRecipients(DEFAULT_MAX_RCPT),"message"));
+ }
+
+ private List<String> getRecipients(Integer n) {
+ return IntStream.range(1, n + 1)
+ .mapToObj(i -> "recipient" + i + "@" + DEFAULT_DOMAIN)
+ .collect(Collectors.toList());
Review Comment:
```suggestion
.collect(ImmutableList.toImmutableList());
```
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2448:
URL: https://github.com/apache/james-project/pull/2448#discussion_r1799403453
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/fastfail/MaxRcptHandler.java:
##########
@@ -26,7 +26,7 @@
public class MaxRcptHandler extends org.apache.james.protocols.smtp.core.fastfail.MaxRcptHandler implements ProtocolHandler {
@Override
public void init(Configuration config) throws ConfigurationException {
- int maxRcpt = config.getInt("maxRcpt", 0);
- setMaxRcpt(maxRcpt);
+ int maxRcpt = config.getInt("maxRcptCount", 0);
Review Comment:
Can we avoid this breaking change?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] [ISSUE-5286] Integration tests for MaxRcptHandler [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #2448:
URL: https://github.com/apache/james-project/pull/2448#issuecomment-2412702155
@florentos17 squash fixups and clean git history?
--
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: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org