You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/10/27 12:40:42 UTC

[GitHub] [james-project] ottoka opened a new pull request, #1278: JAMES-3843 VacationMailet should use null sender

ottoka opened a new pull request, #1278:
URL: https://github.com/apache/james-project/pull/1278

   When the VacationMailet sends a vacation message, it uses the original recipient as envelope sender. However, RFC 5230 states
   "5.1. SMTP MAIL FROM Address
   The SMTP MAIL FROM address of the message envelope SHOULD be set to <>."
   While we are technically OK because of the SHOULD, I do not see any real requirement to deviate from the recommendation. In fact, using the null sender can help prevent mail loops. So I propose we change the VacationMailet accordingly.


-- 
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


[GitHub] [james-project] ottoka commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1299745013

   Nice work! Let's close this PR and continue the topic with https://github.com/apache/james-project/pull/1287.


-- 
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


[GitHub] [james-project] chibenwa commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1296969049

   My 2 thoughts: Given to choose between `bad` `sad` and `annoying` I would try to have a look at the `annoying` option and if it annoys me too much I would go for the `sad` option. Up to 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


[GitHub] [james-project] ottoka closed pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
ottoka closed pull request #1278: JAMES-3843 VacationMailet should use null sender
URL: https://github.com/apache/james-project/pull/1278


-- 
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


[GitHub] [james-project] chibenwa commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1298748680

   https://github.com/apache/james-project/pull/1287 as promised.


-- 
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


[GitHub] [james-project] chibenwa commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1296969642

   If going for the annoying a comment next to the option would be critical


-- 
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


[GitHub] [james-project] chibenwa commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1298087181

   > FakeSmtp is used in quite a lot of tests, I don't really want to touch it. So I went with the annoying option instead, using a boolean system property "james.vacation.sender.explicit" (default false). Its main purpose is as a workaround for the integration tests, I don't see any real word use case. Should I still add documentation and/or sample configuration for it?
   
   Let me have a try at the mock server today...


-- 
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


[GitHub] [james-project] Arsnael commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1294411397

   ```
   org.apache.james.jmap.memory.MemoryVacationRelayIntegrationTest.forwardingAnEmailShouldWork
   
   Error Message
   
   Assertion condition defined as a lambda expression in org.apache.james.jmap.VacationRelayIntegrationTest that uses java.lang.String null within 1 minutes.
   
   Stacktrace
   
   org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a lambda expression in org.apache.james.jmap.VacationRelayIntegrationTest that uses java.lang.String null within 1 minutes.
   Caused by: java.util.concurrent.TimeoutException
   ```
   and
   ```
   org.apache.james.jmap.rabbitmq.RabbitMQAwsS3VacationRelayIntegrationTest.forwardingAnEmailShouldWork
   
   Error Message
   
   Assertion condition defined as a lambda expression in org.apache.james.jmap.VacationRelayIntegrationTest that uses java.lang.String null within 1 minutes.
   
   Stacktrace
   
   org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a lambda expression in org.apache.james.jmap.VacationRelayIntegrationTest that uses java.lang.String null within 1 minutes.
   Caused by: java.util.concurrent.TimeoutException
   ```
   Possibly related? 


-- 
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


[GitHub] [james-project] chibenwa commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1296970598

   Other options: frop FakeSmtp in this test in favor of https://github.com/apache/james-project/tree/master/server/mailet/mock-smtp-server
   
   CF https://github.com/apache/james-project/tree/master/server/mailet/remote-delivery-integration-testing


-- 
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


[GitHub] [james-project] ottoka commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1298069012

   FakeSmtp is used in quite a lot of tests, I don't really want to touch it. So I went with the annoying option instead, using a boolean system property "james.vacation.sender.explicit" (default false). Its main purpose is as a workaround for the integration tests, I don't see any real word use case. Should I still add documentation and/or sample configuration 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


[GitHub] [james-project] ottoka commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1296871945

   The integration tests fail since the FakeSMTP server does not handle an empty sender correctly:
   `MAIL FROM:<>`
   `250 2.1.0 Ok`
   `RCPT TO:<ra...@yopmail.com>`
   `503 5.5.1 Error: need MAIL command`
   Digging further I find https://github.com/andris9/simplesmtp/blob/master/lib/server.js#L403 , which we hit since since Javascript treats undefined/null/empty string all as false. Not sure if this is a feature or a bug in simplesmtp.
   
   In either case, I don't have the time to fix a third party project. I could
   1. do nothing, drop this PR, close the Jira ticket as won't fix. After all the RFC only has a SHOULD requirement here. - _sad_
   2. disable the vacation integration tests - _bad_
   3. make the sender conditional on a system property or mailet configuration, e.g. "explicitSender=true" would send the address as before, "explicitSender=false" would use the null sender. Default false. - _annoying_
   
   Any suggestions? 


-- 
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


[GitHub] [james-project] ottoka commented on pull request #1278: JAMES-3843 VacationMailet should use null sender

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1278:
URL: https://github.com/apache/james-project/pull/1278#issuecomment-1294593481

   Seems I missed the integration tests, I will take a look.


-- 
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