feat(tests): Implement ERR-02 Invalid Sequence Handling and update test migration documentation
This commit is contained in:
		| @@ -133,6 +133,7 @@ export async function startTestServer(config: ITestServerConfig): Promise<ITestS | |||||||
|         recordAuthenticationFailure: async (_ip: string) => {}, |         recordAuthenticationFailure: async (_ip: string) => {}, | ||||||
|         recordSyntaxError: async (_ip: string) => {}, |         recordSyntaxError: async (_ip: string) => {}, | ||||||
|         recordCommandError: async (_ip: string) => {}, |         recordCommandError: async (_ip: string) => {}, | ||||||
|  |         recordError: (_ip: string) => false, // Returns false = don't block IP in tests | ||||||
|         isBlocked: async (_ip: string) => false, |         isBlocked: async (_ip: string) => false, | ||||||
|         cleanup: async () => {}, |         cleanup: async () => {}, | ||||||
|       }; |       }; | ||||||
|   | |||||||
| @@ -99,7 +99,7 @@ Tests for proper error handling and recovery. | |||||||
| | ID | Test | Priority | Status | | | ID | Test | Priority | Status | | ||||||
| |----|------|----------|--------| | |----|------|----------|--------| | ||||||
| | **ERR-01** | **Syntax Error Handling** | **High** | **✅ PORTED** | | | **ERR-01** | **Syntax Error Handling** | **High** | **✅ PORTED** | | ||||||
| | ERR-02 | Invalid Sequence Handling | High | Planned | | | **ERR-02** | **Invalid Sequence Handling** | **High** | **✅ PORTED** | | ||||||
| | ERR-05 | Resource Exhaustion | High | Planned | | | ERR-05 | Resource Exhaustion | High | Planned | | ||||||
| | ERR-07 | Exception Handling | High | Planned | | | ERR-07 | Exception Handling | High | Planned | | ||||||
|  |  | ||||||
| @@ -275,16 +275,36 @@ Tests for proper error handling and recovery. | |||||||
| - Server lifecycle management | - Server lifecycle management | ||||||
|  |  | ||||||
| **Key validations**: | **Key validations**: | ||||||
| - ✓ Invalid commands rejected with appropriate error codes | - ✓ Invalid commands rejected with 500/502 error codes | ||||||
| - ✓ MAIL FROM requires angle brackets (501 error if missing) | - ✓ MAIL FROM requires angle brackets (501 error if missing) | ||||||
| - ✓ RCPT TO requires angle brackets (501 error if missing) | - ✓ RCPT TO requires angle brackets (501 error if missing) | ||||||
| - ✓ EHLO requires hostname parameter (501 error if missing) | - ✓ EHLO requires hostname parameter (501 error if missing) | ||||||
| - ✓ Extra parameters on QUIT handled (accepted or rejected with 501) | - ✓ Extra parameters on QUIT handled (501 syntax error) | ||||||
| - ✓ Malformed email addresses rejected (501 or 553 error) | - ✓ Malformed email addresses rejected (501 error) | ||||||
| - ✓ Commands in wrong sequence rejected (503 error) | - ✓ Commands in wrong sequence rejected (503 error) | ||||||
| - ✓ Excessively long commands handled gracefully | - ✓ Excessively long commands handled gracefully | ||||||
|  |  | ||||||
| **Note**: Server currently has a bug where `rateLimiter.recordError` is not implemented, causing invalid commands to return 451 (temporary error) instead of 500/502 (syntax error). Tests accept 451 as valid until this is fixed. | ### ✅ ERR-02: Invalid Sequence Handling (`test.err-02.invalid-sequence.test.ts`) | ||||||
|  |  | ||||||
|  | **Tests**: 10 total (10 passing) | ||||||
|  | - Rejects MAIL FROM before EHLO | ||||||
|  | - Rejects RCPT TO before MAIL FROM | ||||||
|  | - Rejects DATA before RCPT TO (RFC 5321 compliance) | ||||||
|  | - Allows multiple EHLO commands | ||||||
|  | - Handles second MAIL FROM without RSET | ||||||
|  | - Rejects DATA without MAIL FROM | ||||||
|  | - Handles commands after QUIT | ||||||
|  | - Recovers from syntax errors in sequence | ||||||
|  | - Server lifecycle management | ||||||
|  |  | ||||||
|  | **Key validations**: | ||||||
|  | - ✓ MAIL FROM requires EHLO first (503 error if missing) | ||||||
|  | - ✓ RCPT TO requires MAIL FROM first (503 error if missing) | ||||||
|  | - ✓ DATA requires RCPT TO with at least one recipient (503 error if missing) | ||||||
|  | - ✓ Multiple EHLO commands allowed (resets session state) | ||||||
|  | - ✓ Commands after QUIT handled correctly (connection closed) | ||||||
|  | - ✓ Session recovers from syntax errors without terminating | ||||||
|  | - ✓ RFC 5321 compliance: strict command sequence enforcement | ||||||
|  |  | ||||||
| ## Running Tests | ## Running Tests | ||||||
|  |  | ||||||
| @@ -383,7 +403,7 @@ import { connectToSmtp, sendSmtpCommand } from '../../helpers/utils.ts'; | |||||||
| - 🔄 SEC-08: Rate Limiting | - 🔄 SEC-08: Rate Limiting | ||||||
| - 🔄 SEC-10: Header Injection Prevention | - 🔄 SEC-10: Header Injection Prevention | ||||||
| - ✅ ERR-01: Syntax Error Handling | - ✅ ERR-01: Syntax Error Handling | ||||||
| - 🔄 ERR-02: Invalid Sequence Handling | - ✅ ERR-02: Invalid Sequence Handling | ||||||
|  |  | ||||||
| ### Phase 3: Advanced Features (Medium Priority) | ### Phase 3: Advanced Features (Medium Priority) | ||||||
| - 🔄 SEC-03: DKIM Processing | - 🔄 SEC-03: DKIM Processing | ||||||
| @@ -408,7 +428,7 @@ import { connectToSmtp, sendSmtpCommand } from '../../helpers/utils.ts'; | |||||||
| - SMTP protocol utilities with readSmtpResponse helper | - SMTP protocol utilities with readSmtpResponse helper | ||||||
| - Test certificates (self-signed RSA) | - Test certificates (self-signed RSA) | ||||||
|  |  | ||||||
| **Tests Ported**: 10/100+ test files (72 total tests passing) | **Tests Ported**: 11/100+ test files (82 total tests passing) | ||||||
| - ✅ CMD-01: EHLO Command (5 tests passing) | - ✅ CMD-01: EHLO Command (5 tests passing) | ||||||
| - ✅ CMD-02: MAIL FROM Command (6 tests passing) | - ✅ CMD-02: MAIL FROM Command (6 tests passing) | ||||||
| - ✅ CMD-03: RCPT TO Command (7 tests passing) | - ✅ CMD-03: RCPT TO Command (7 tests passing) | ||||||
| @@ -419,6 +439,7 @@ import { connectToSmtp, sendSmtpCommand } from '../../helpers/utils.ts'; | |||||||
| - ✅ EP-01: Basic Email Sending (7 tests passing) | - ✅ EP-01: Basic Email Sending (7 tests passing) | ||||||
| - ✅ SEC-06: IP Reputation Checking (7 tests passing) | - ✅ SEC-06: IP Reputation Checking (7 tests passing) | ||||||
| - ✅ ERR-01: Syntax Error Handling (10 tests passing) | - ✅ ERR-01: Syntax Error Handling (10 tests passing) | ||||||
|  | - ✅ ERR-02: Invalid Sequence Handling (10 tests passing) | ||||||
|  |  | ||||||
| **Coverage**: Complete essential SMTP transaction flow | **Coverage**: Complete essential SMTP transaction flow | ||||||
| - EHLO → MAIL FROM → RCPT TO → DATA → QUIT ✅ | - EHLO → MAIL FROM → RCPT TO → DATA → QUIT ✅ | ||||||
| @@ -427,10 +448,19 @@ import { connectToSmtp, sendSmtpCommand } from '../../helpers/utils.ts'; | |||||||
|  |  | ||||||
| **Phase 1 Status**: ✅ **COMPLETE** (7/7 tests, 100%) | **Phase 1 Status**: ✅ **COMPLETE** (7/7 tests, 100%) | ||||||
|  |  | ||||||
|  | **Phase 2 Status**: 🔄 **IN PROGRESS** (3/6 tests, 50%) | ||||||
|  | - ✅ SEC-06: IP Reputation | ||||||
|  | - ✅ ERR-01: Syntax Errors | ||||||
|  | - ✅ ERR-02: Invalid Sequence | ||||||
|  | - 🔄 SEC-01: Authentication | ||||||
|  | - 🔄 SEC-08: Rate Limiting | ||||||
|  | - 🔄 SEC-10: Header Injection | ||||||
|  |  | ||||||
| **Next Steps**: | **Next Steps**: | ||||||
| 1. Port remaining security tests (SEC-01 Authentication, SEC-08 Rate Limiting, SEC-10 Header Injection) | 1. Port SEC-01: Authentication test | ||||||
| 2. Port ERR-02: Invalid Sequence Handling test | 2. Port SEC-08: Rate Limiting test | ||||||
| 3. Continue with remaining high-priority tests | 3. Port SEC-10: Header Injection Prevention test | ||||||
|  | 4. Continue with Phase 3 (Advanced Features) | ||||||
|  |  | ||||||
| ## Production Readiness Criteria | ## Production Readiness Criteria | ||||||
|  |  | ||||||
|   | |||||||
| @@ -92,7 +92,7 @@ Tests for proper error handling and recovery. | |||||||
| | Test ID | Source File | Destination File | Status | Tests | Notes | | | Test ID | Source File | Destination File | Status | Tests | Notes | | ||||||
| |---------|-------------|------------------|--------|-------|-------| | |---------|-------------|------------------|--------|-------|-------| | ||||||
| | **ERR-01** | (dcrouter ERR-01 tests) | `test/suite/smtpserver_error-handling/test.err-01.syntax-errors.test.ts` | **✅ Ported** | 10/10 | Invalid commands, missing brackets, wrong sequences, long commands, malformed addresses | | | **ERR-01** | (dcrouter ERR-01 tests) | `test/suite/smtpserver_error-handling/test.err-01.syntax-errors.test.ts` | **✅ Ported** | 10/10 | Invalid commands, missing brackets, wrong sequences, long commands, malformed addresses | | ||||||
| | ERR-02 | TBD | `test/suite/smtpserver_error-handling/test.err-02.sequence-errors.test.ts` | 📋 Planned | - | Out-of-order commands | | | **ERR-02** | (dcrouter ERR-02 tests) | `test/suite/smtpserver_error-handling/test.err-02.invalid-sequence.test.ts` | **✅ Ported** | 10/10 | MAIL before EHLO, RCPT before MAIL, DATA before RCPT, multiple EHLO, commands after QUIT, sequence recovery | | ||||||
| | ERR-05 | TBD | `test/suite/smtpserver_error-handling/test.err-05.resource-exhaustion.test.ts` | 📋 Planned | - | Memory/connection limits | | | ERR-05 | TBD | `test/suite/smtpserver_error-handling/test.err-05.resource-exhaustion.test.ts` | 📋 Planned | - | Memory/connection limits | | ||||||
| | ERR-07 | TBD | `test/suite/smtpserver_error-handling/test.err-07.exception-handling.test.ts` | 📋 Planned | - | Unexpected errors, crashes | | | ERR-07 | TBD | `test/suite/smtpserver_error-handling/test.err-07.exception-handling.test.ts` | 📋 Planned | - | Unexpected errors, crashes | | ||||||
|  |  | ||||||
| @@ -146,9 +146,9 @@ Tests for RFC 5321/5322 compliance. | |||||||
|  |  | ||||||
| ### Overall Statistics | ### Overall Statistics | ||||||
| - **Total test files identified**: ~100+ | - **Total test files identified**: ~100+ | ||||||
| - **Files ported**: 10/100+ (10%) | - **Files ported**: 11/100+ (11%) | ||||||
| - **Total tests ported**: 72/~500+ (14%) | - **Total tests ported**: 82/~500+ (16%) | ||||||
| - **Tests passing**: 72/72 (100%) | - **Tests passing**: 82/82 (100%) | ||||||
|  |  | ||||||
| ### By Priority | ### By Priority | ||||||
|  |  | ||||||
| @@ -169,9 +169,9 @@ Tests for RFC 5321/5322 compliance. | |||||||
| - 📋 SEC-08: Rate Limiting | - 📋 SEC-08: Rate Limiting | ||||||
| - 📋 SEC-10: Header Injection | - 📋 SEC-10: Header Injection | ||||||
| - ✅ ERR-01: Syntax Errors (10 tests) | - ✅ ERR-01: Syntax Errors (10 tests) | ||||||
| - 📋 ERR-02: Sequence Errors | - ✅ ERR-02: Invalid Sequence (10 tests) | ||||||
|  |  | ||||||
| **Phase 2 Progress**: 2/6 complete (33%) | **Phase 2 Progress**: 3/6 complete (50%) | ||||||
|  |  | ||||||
| #### Medium Priority (Phase 3: Advanced Features) | #### Medium Priority (Phase 3: Advanced Features) | ||||||
| - 📋 SEC-03: DKIM | - 📋 SEC-03: DKIM | ||||||
|   | |||||||
| @@ -719,22 +719,20 @@ export class CommandHandler implements ICommandHandler { | |||||||
|       return; |       return; | ||||||
|     } |     } | ||||||
|      |      | ||||||
|     // For tests, be slightly more permissive - also accept DATA after MAIL FROM |     // RFC 5321: DATA must only be accepted after RCPT TO | ||||||
|     // But ensure we at least have a sender defined |     if (session.state !== SmtpState.RCPT_TO) { | ||||||
|     if (session.state !== SmtpState.RCPT_TO && session.state !== SmtpState.MAIL_FROM) { |  | ||||||
|       this.sendResponse(socket, `${SmtpResponseCode.BAD_SEQUENCE} Bad sequence of commands`); |       this.sendResponse(socket, `${SmtpResponseCode.BAD_SEQUENCE} Bad sequence of commands`); | ||||||
|       return; |       return; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     // Check if we have a sender |     // RFC 5321: Must have a sender | ||||||
|     if (!session.mailFrom) { |     if (!session.mailFrom) { | ||||||
|       this.sendResponse(socket, `${SmtpResponseCode.BAD_SEQUENCE} No sender specified`); |       this.sendResponse(socket, `${SmtpResponseCode.BAD_SEQUENCE} No sender specified`); | ||||||
|       return; |       return; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     // Ideally we should have recipients, but for test compatibility, we'll only |     // RFC 5321: Must have at least one recipient | ||||||
|     // insist on recipients if we're in RCPT_TO state |     if (!session.rcptTo.length) { | ||||||
|     if (session.state === SmtpState.RCPT_TO && !session.rcptTo.length) { |  | ||||||
|       this.sendResponse(socket, `${SmtpResponseCode.BAD_SEQUENCE} No recipients specified`); |       this.sendResponse(socket, `${SmtpResponseCode.BAD_SEQUENCE} No recipients specified`); | ||||||
|       return; |       return; | ||||||
|     } |     } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user