You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "Arsnael (via GitHub)" <gi...@apache.org> on 2024/08/28 07:16:05 UTC
[PR] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Arsnael opened a new pull request, #2393:
URL: https://github.com/apache/james-project/pull/2393
I hesitated adding a test for the jvm property commit as it's a bit tricky for james startup
Maybe in memory-app with some alternative conf files in resources? Or just not necessary?
--
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] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2393:
URL: https://github.com/apache/james-project/pull/2393#discussion_r1734352508
##########
server/container/guice/mailet/src/main/java/org/apache/james/modules/server/MailetContainerModule.java:
##########
@@ -114,7 +115,10 @@ protected void configure() {
initialisationOperations.addBinding().to(MailetModuleInitializationOperation.class);
Multibinder<ProcessorsCheck> transportProcessorChecks = Multibinder.newSetBinder(binder(), ProcessorsCheck.class);
- transportProcessorChecks.addBinding().toInstance(BCC_Check);
+
+ if (MAILET_CONTAINER_CHECK_ENABLED) {
Review Comment:
If disabled we should not perform a single check against the mailet container IMO
--
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] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael merged PR #2393:
URL: https://github.com/apache/james-project/pull/2393
--
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] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #2393:
URL: https://github.com/apache/james-project/pull/2393#discussion_r1735881521
##########
server/container/guice/mailet/src/main/java/org/apache/james/modules/server/MailetContainerModule.java:
##########
@@ -86,7 +86,7 @@ public class MailetContainerModule extends AbstractModule {
"transport",
All.class,
RemoveMimeHeader.class,
- pair -> Arrays.asList(pair.getMailet().getMailetConfig().getInitParameter("name").split(",")).contains("bcc"),
+ pair -> Arrays.asList(pair.getMailet().getMailetConfig().getInitParameter("name").toLowerCase().split(",")).contains("bcc"),
Review Comment:
Actually it does.
RemoveMimeHeader throws this in case of missing name property => https://github.com/apache/james-project/blob/master/mailet/standard/src/main/java/org/apache/james/transport/mailets/RemoveMimeHeader.java#L53
```
throw new MailetException("Invalid config. Please specify atleast one name");
```
And the test => https://github.com/apache/james-project/blob/master/mailet/standard/src/test/java/org/apache/james/transport/mailets/RemoveMimeHeaderTest.java#L212
I think it's ok?
--
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] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2393:
URL: https://github.com/apache/james-project/pull/2393#discussion_r1735910485
##########
server/container/guice/mailet/src/main/java/org/apache/james/modules/server/MailetContainerModule.java:
##########
@@ -86,7 +86,7 @@ public class MailetContainerModule extends AbstractModule {
"transport",
All.class,
RemoveMimeHeader.class,
- pair -> Arrays.asList(pair.getMailet().getMailetConfig().getInitParameter("name").split(",")).contains("bcc"),
+ pair -> Arrays.asList(pair.getMailet().getMailetConfig().getInitParameter("name").toLowerCase().split(",")).contains("bcc"),
Review Comment:
Likely OK
--
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] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2393:
URL: https://github.com/apache/james-project/pull/2393#discussion_r1734353964
##########
server/container/guice/protocols/jmap/src/test/java/org/apache/james/jmap/MailetPreconditionTest.java:
##########
@@ -191,5 +191,19 @@ void bccMailetCheckShouldNotThrowOnValidPair() throws Exception {
assertThatCode(() -> MailetContainerModule.BCC_Check.check(pairs))
.doesNotThrowAnyException();
}
+
+ @Test
+ void bccMailetCheckShouldNotThrowOnValidPairWithOtherNames() throws Exception {
Review Comment:
Edge case: Please add a test ensuring that removing `x-bcc-bug` header is not considered as valid.
--
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] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2393:
URL: https://github.com/apache/james-project/pull/2393#discussion_r1734351194
##########
docs/modules/servers/pages/distributed/configure/jvm.adoc:
##########
@@ -103,3 +103,18 @@ Ex in `jvm.properties`
# james.reactor.inputstream.prefetch=4
----
+== Disable mailet container check at James startup
+
+James is doing a check on startup making sure that the `bcc` header is being removed via
Review Comment:
```suggestion
James is doing checks on startup for validating mailet container configuration against a set of business rules, for instance making sure that the `bcc` header is being removed via
```
##########
docs/modules/servers/pages/distributed/configure/jvm.adoc:
##########
@@ -103,3 +103,18 @@ Ex in `jvm.properties`
# james.reactor.inputstream.prefetch=4
----
+== Disable mailet container check at James startup
+
+James is doing a check on startup making sure that the `bcc` header is being removed via
+`RemoveMimeHeader` mailet in the mail processing pipeline defined in `mailetcontainer.xml` file.
+
+It could be useful for some administrators that know what they are doing to disable such a check
Review Comment:
```suggestion
It could be useful for some administrators that know what they are doing to disable such checks
```
--
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] JAMES-4059 MailetContainerModule should not do a strict check on RemoveMimeHeader with bcc name param [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2393:
URL: https://github.com/apache/james-project/pull/2393#discussion_r1735709020
##########
server/container/guice/mailet/src/main/java/org/apache/james/modules/server/MailetContainerModule.java:
##########
@@ -86,7 +86,7 @@ public class MailetContainerModule extends AbstractModule {
"transport",
All.class,
RemoveMimeHeader.class,
- pair -> Arrays.asList(pair.getMailet().getMailetConfig().getInitParameter("name").split(",")).contains("bcc"),
+ pair -> Arrays.asList(pair.getMailet().getMailetConfig().getInitParameter("name").toLowerCase().split(",")).contains("bcc"),
Review Comment:
Other stupid edge case question: do we handle the case where RemoveMimeHeader do not have a "name" property decently?
IE we likely throw today an uggly non explicit NPE, it would be nice to throw a contextualized with decent error message IllegalStateException, and have a unit test for it?
--
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