You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "quantranhong1999 (via GitHub)" <gi...@apache.org> on 2024/08/27 07:54:36 UTC
[PR] JAMES-4050 Allow `%` and `*` characters in mailbox names [james-project]
quantranhong1999 opened a new pull request, #2391:
URL: https://github.com/apache/james-project/pull/2391
(no comment)
--
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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #2391:
URL: https://github.com/apache/james-project/pull/2391#issuecomment-2320539001
Needs a rebase @quantranhong1999
--
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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #2391:
URL: https://github.com/apache/james-project/pull/2391
--
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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2391:
URL: https://github.com/apache/james-project/pull/2391#discussion_r1732352080
##########
docs/modules/servers/pages/distributed/configure/jvm.adoc:
##########
@@ -103,3 +103,15 @@ Ex in `jvm.properties`
# james.reactor.inputstream.prefetch=4
----
+== Relax mailbox name validation
+
+The property `james.relax.mailbox.name.validation` allows to accept `*` and `%` characters in mailbox name.
+
+Optional. Boolean. Default to true.
+
+Ex in `jvm.properties`
+----
+james.relax.mailbox.name.validation=false
Review Comment:
```suggestion
james.relaxed.mailbox.name.validation=false
```
##########
docs/modules/servers/pages/distributed/configure/jvm.adoc:
##########
@@ -103,3 +103,15 @@ Ex in `jvm.properties`
# james.reactor.inputstream.prefetch=4
----
+== Relax mailbox name validation
+
+The property `james.relax.mailbox.name.validation` allows to accept `*` and `%` characters in mailbox name.
+
+Optional. Boolean. Default to true.
Review Comment:
Default to false. Let's avoid breaking changes?
##########
docs/modules/servers/pages/distributed/configure/jvm.adoc:
##########
@@ -103,3 +103,15 @@ Ex in `jvm.properties`
# james.reactor.inputstream.prefetch=4
----
+== Relax mailbox name validation
+
+The property `james.relax.mailbox.name.validation` allows to accept `*` and `%` characters in mailbox name.
+
+Optional. Boolean. Default to true.
+
+Ex in `jvm.properties`
+----
+james.relax.mailbox.name.validation=false
+----
+To strictly validate `*` and `%` characters in the mailbox name.
Review Comment:
Please explain that % and * are ambiguous for the LIST / LSUB commands that interprete those as wildcard thus returning all mailboxes mating the pattern.
##########
mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxPathTest.java:
##########
@@ -367,17 +367,17 @@ void assertAcceptableShouldNotThrowWhenSharpInTheMiddle() {
}
@Test
- void assertAcceptableShouldThrowOnPercent() {
- assertThatThrownBy(() -> MailboxPath.forUser(USER, "a%b")
+ void assertAcceptableShouldNotThrowOnPercentByDefault() {
+ assertThatCode(() -> MailboxPath.forUser(USER, "a%b")
.assertAcceptable('.'))
- .isInstanceOf(MailboxNameException.class);
+ .doesNotThrowAnyException();
}
@Test
- void assertAcceptableShouldThrowOnWildcard() {
- assertThatThrownBy(() -> MailboxPath.forUser(USER, "a*b")
+ void assertAcceptableShouldNotThrowOnWildcardByDefault() {
+ assertThatCode(() -> MailboxPath.forUser(USER, "a*b")
.assertAcceptable('.'))
- .isInstanceOf(MailboxNameException.class);
+ .doesNotThrowAnyException();
}
Review Comment:
Please check the two modes.
--
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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2391:
URL: https://github.com/apache/james-project/pull/2391#discussion_r1732523877
##########
server/apps/jpa-smtp-app/sample-configuration/jvm.properties:
##########
@@ -53,4 +53,8 @@ jmx.remote.x.mlet.allow.getMBeansFromURL=false
openjpa.Multithreaded=true
# Whether James should unzip JWTs. Default to false
-# james.jwt.zip.allow=false
\ No newline at end of file
+# james.jwt.zip.allow=false
+
+# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
+# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
+#james.relaxed.mailbox.name.validation=true
Review Comment:
This app have no mailbox this option is thus not needed
--
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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2391:
URL: https://github.com/apache/james-project/pull/2391#discussion_r1732524333
##########
server/apps/scaling-pulsar-smtp/sample-configuration/jvm.properties:
##########
@@ -46,3 +46,7 @@
# Whether James should unzip JWTs. Default to false
# james.jwt.zip.allow=false
+
+# Relax validating `*` and `%` characters in the mailbox name. Defaults to false.
+# Be careful turning on this as `%` and `*` are ambiguous for the LIST / LSUB commands that interpret those as wildcard thus returning all mailboxes matching the pattern.
+#james.relaxed.mailbox.name.validation=true
Review Comment:
Idem this app is smtp only
--
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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #2391:
URL: https://github.com/apache/james-project/pull/2391#discussion_r1732525723
##########
mpt/impl/imap-mailbox/inmemory/pom.xml:
##########
@@ -81,7 +81,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
- <reuseForks>true</reuseForks>
+ <reuseForks>false</reuseForks>
Review Comment:
This was needed to retain isolation for `MailboxPath`' static final INVALID_CHARS across test classes.
Acceptable for memory tests 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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2391:
URL: https://github.com/apache/james-project/pull/2391#discussion_r1732680148
##########
mpt/impl/imap-mailbox/inmemory/pom.xml:
##########
@@ -81,7 +81,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
- <reuseForks>true</reuseForks>
+ <reuseForks>false</reuseForks>
Review Comment:
Ok. What's the overall degradation?
--
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-4050 Allow `%` and `*` characters in mailbox names [james-project]
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #2391:
URL: https://github.com/apache/james-project/pull/2391#discussion_r1733913222
##########
mpt/impl/imap-mailbox/inmemory/pom.xml:
##########
@@ -81,7 +81,7 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
- <reuseForks>true</reuseForks>
+ <reuseForks>false</reuseForks>
Review Comment:
On my local PC, for the MPT memory module. Test run time:
Before: 35s
After: 36s
Neglectable. Likely bootstrap new JVMs is too fast on my PC.
--
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