SMTP: Consistently validate HELO and scope BAD_EHLO_HELO to connection

[prosgarz35]

Problem

1. Divergent HELO and EHLO validation: HeloCmdHandler used legacy validation logic that rejected valid RFC 1123/5321 identifiers accepted by EhloCmdHandler (such as hostnames with numeric label prefixes like mx-ll-110-164-x-x.2s1n or opaque/MAC identifiers). It also logged Invalid EHLO instead of Invalid HELO.
2. DNS check bypass on transaction reset: ResolvableEhloHeloHandler stored the BAD_EHLO_HELO flag in State.Transaction. When an unresolvable HELO/EHLO was rejected during MAIL, a transaction reset (RSET or subsequent transaction) cleared the flag, allowing subsequent MAIL commands on the same connection to proceed without a valid HELO.

Solution

• Consistent validation (DRY): Delegate HeloCmdHandler argument validation to EhloCmdHandler.isValid() and fix the log message typo.
• Connection-scoped check: Store BAD_EHLO_HELO in State.Connection so the rejection persists across resets until a valid HELO/EHLO command is issued.
• Tests:
  • Added test case for domains with numeric prefixes in SMTPServerTest.
  • Added regression test in ResolvableEhloHeloHandlerTest ensuring bad HELO persists across resets and is cleared upon receiving a valid HELO.
  • Hardened multi-transaction test in SMTPServerTest to assert queue size and message subject.

[chibenwa]

Wonderful, thanks for opening this one.

State is minimal, tests are there. I'll let it open few days for others to have a look.

If some other SMTP bugs are found please favor separate PRs.

[chibenwa]

Consistent validation (DRY): Delegate HeloCmdHandler argument validation to EhloCmdHandler.isValid() and fix the log message typo.

Very good catch: +1

I was aware of this bug, but got faced to it in a context where I lacked the time to handle it then forgot.

That's a nitpick because HELO is pretty much unused now but I fully agree the allignmenet is desirable.

DNS check bypass on transaction reset

Nice catch.

[prosgarz35]

Consistent validation (DRY): Delegate HeloCmdHandler argument validation to EhloCmdHandler.isValid() and fix the log message typo.

Very good catch: +1

I was aware of this bug, but got faced to it in a context where I lacked the time to handle it then forgot.

That's a nitpick because HELO is pretty much unused now but I fully agree the allignmenet is desirable.

DNS check bypass on transaction reset

Nice catch.

you make me blush :) i try to find more not too much big but userful problems, i just dont have unlimited ai resources :)

[felixauringer]

I also like this PR! Maybe squash your commits and let a maintainer restart the pipeline (I do not think the failure is related to your changes).

[prosgarz35]

I also like this PR! Maybe squash your commits and let a maintainer restart the pipeline (I do not think the failure is related to your changes).

i am also read logs and cant understand why it failed checks

[prosgarz35]

find error to failed build in jenkins

pom.xml

3950 line

-javaagent:"${settings.localRepository}"/org/jacoco/org.jacoco.agent/${jacoco-maven-plugin.version}/org.jacoco.agent-${jacoco-maven-plugin.version}-runtime.jar=destfile=${basedir}/target/jacoco.exec:${org.mockito:mockito-core:jar}

THIS :${org.mockito:mockito-core:jar} makes error

[chibenwa]

I also like this PR! Maybe squash your commits and let a maintainer restart the pipeline (I do not think the failure is related to your changes).

You are a maintainer now @felixauringer !

You can use your ASF login => https://id.apache.org/

[Arsnael]

LGTM. i did restart the build, the error was unerelated IMO

[prosgarz35]

Can i ask for help why it not build?

[Arsnael]

Once again, Jenkins is managed by the Apache infra team (not us^^), and like any platforms, sometimes there is issues, or sometimes when they do updates too they for example tell their workers to not take any new job until the ones currently running finish before restarting the platform. So yes it's not always building right off. Patience is appreciated here :)

I saw your build ran in the end. I restarted it though as the error was unrelated to your PR. Hope it helps!

[prosgarz35]

Once again, Jenkins is managed by the Apache infra team (not us^^), and like any platforms, sometimes there is issues, or sometimes when they do updates too they for example tell their workers to not take any new job until the ones currently running finish before restarting the platform. So yes it's not always building right off. Patience is appreciated here :)

I saw your build ran in the end. I restarted it though as the error was unrelated to your PR. Hope it helps!

thank you Rene <3

[chibenwa]

Hello @prosgarz35

Thanks a lot for those cool fixes.
They had been hanging for a while, let's just merge them.

Cheers,

Benoit