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.

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

JAMES-4008 JMAP - Email/set - Should be able to save a draft with invalid email address #2040

Merged
merged 4 commits into from
Feb 26, 2024

Conversation

vttranlina
Copy link
Contributor

Copy link
Contributor

@Arsnael Arsnael left a comment

Choose a reason for hiding this comment

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

Small remarks but looks good otherwise

Copy link
Contributor

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

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

  • Please add a test to ensure that EmailSubmission/set is not able to send a mail with invalid addresses...

@vttranlina
Copy link
Contributor Author

  • Please add a test to ensure that EmailSubmission/set is not able to send a mail with invalid addresses...

updated it in last commit 52ab0e8

the note here is invalid addresses in the "mimeMessage header" (The address in envelop (mailFrom, rcptTo) still correct)

@@ -7507,6 +7507,142 @@ trait EmailSetMethodContract {

}

@Disabled("Bug of james-mime4j when pasing mimeMessages https://github.com/linagora/james-project/issues/5086")
Copy link
Contributor

Choose a reason for hiding this comment

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

Where the problem is? In Email/get?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we handle the exception in Scala code for the time being?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, Email/get
it returns incorrect to property response

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't we handle the exception in Scala code for the time being?

We can handle it by filtering addresses with not value ">"
Is it acceptable?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

As expressed in linagora#5086 (comment) this corner on't happen with Twake mail in practice so it will not be a blocker...

.inPath("methodResponses[1][1].list")
.isEqualTo("""[{
| "to": [{
| "name": "name1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temporarily disable this test:

Right now the response is

"to": [
  {
      "name": "name1",
      "email": "invalid1"
  },
  {
      "email": ">"
  }
]

It is a bug of james-mime4j when parse the message
Detail: linagora#5086

Copy link
Contributor

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

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

Thank you for the very nice work!

@chibenwa
Copy link
Contributor

linagora#5086 (comment)

How about by the time we get this fixed we filter in the JMAP stack the lone > email? Not clean but it would do as a temporary fix...

@chibenwa
Copy link
Contributor

org.apache.james.jmap.rfc8621.memory.MemoryEmailSubmissionSetMethodTest.emailSubmissionSetCanBeChainedAfterEmailSet(GuiceJamesServer)
Failing for the past 1 build (Since
#4 )
Took 10 sec.
Add description
Error Message

Assertion condition defined as a lambda expression in org.apache.james.jmap.rfc8621.contract.EmailSubmissionSetMethodContract that uses java.lang.String [Node "methodResponses[0][1].ids"]
Expected size: 1 but was: 0 in:
[] within 10 seconds.

@chibenwa
Copy link
Contributor

Related?

@chibenwa chibenwa merged commit a2583d4 into apache:master Feb 26, 2024
1 check passed
@chibenwa chibenwa added the bug label Feb 26, 2024
.`then`(SMono.just(mimeMessage))

private def validateMailAddress(headName: String, address: Address): MailAddress =
Try(new MailAddress(address.toString)) match {
Copy link
Contributor

Choose a reason for hiding this comment

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

Try(new MailAddress(address.getAddress)) match {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

class Address do not have .getAddress method

samiulsami pushed a commit to ops-center/james-project that referenced this pull request Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants