From 0334d04104364a134cee5fde8fc11d17ce3dd9c6 Mon Sep 17 00:00:00 2001 From: BrLumen Date: Wed, 16 Sep 2026 01:13:48 +0700 Subject: [PATCH] fix(smtp): tolerate bare reply code without text (RFC 5321 section 4.2) `SmtpResponseLine.parse` called `text.substring(4)` unconditionally, so a reply line consisting of the 3-digit code only (`250`) threw a `RangeError`. Because `SmtpResponse` was constructed outside the try/catch in `SmtpClient.onServerResponse`, the error escaped from the socket callback and the pending command's completer never completed. * parse `Reply-code [ SP textstring ]` per RFC 5321: empty message for a bare code, no throw for lines shorter than a code * construct `SmtpResponse` inside the guard so any parse error fails the pending command instead of hanging it --- lib/src/smtp/smtp_client.dart | 4 +- lib/src/smtp/smtp_response.dart | 13 +++++-- test/smtp/mock_smtp_server.dart | 5 +++ test/smtp/smtp_client_test.dart | 25 +++++++++++++ test/src/smtp/smtp_response_test.dart | 54 +++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 test/src/smtp/smtp_response_test.dart diff --git a/lib/src/smtp/smtp_client.dart b/lib/src/smtp/smtp_client.dart index a28d79ae..452b602d 100644 --- a/lib/src/smtp/smtp_client.dart +++ b/lib/src/smtp/smtp_client.dart @@ -451,10 +451,12 @@ class SmtpClient extends ClientBase { log(responseText, isClient: false); } } - final response = SmtpResponse(responseTexts); final cmd = _currentCommand; if (cmd != null) { try { + // parse inside the guard so that a malformed server reply fails the + // pending command instead of leaving its completer hanging forever + final response = SmtpResponse(responseTexts); final next = cmd.next(response); final text = next?.text; final data = next?.data; diff --git a/lib/src/smtp/smtp_response.dart b/lib/src/smtp/smtp_response.dart index 102a970f..29d89864 100644 --- a/lib/src/smtp/smtp_response.dart +++ b/lib/src/smtp/smtp_response.dart @@ -72,11 +72,18 @@ class SmtpResponseLine { const SmtpResponseLine(this.code, this.message); /// Parses the given response [text]. + /// + /// A reply line is `Reply-code [ SP textstring ]` or + /// `Reply-code "-" [ textstring ]` (RFC 5321, section 4.2), so both the + /// separator and the text are optional: a bare `250` is a valid line. factory SmtpResponseLine.parse(String text) { - final code = int.tryParse(text.substring(0, 3)); - final message = (code == null) ? text : text.substring(4); + final code = text.length < 3 ? null : int.tryParse(text.substring(0, 3)); + if (code == null) { + return SmtpResponseLine(500, text); + } + final message = text.length > 4 ? text.substring(4) : ''; - return SmtpResponseLine(code ?? 500, message); + return SmtpResponseLine(code, message); } /// The code of the response diff --git a/test/smtp/mock_smtp_server.dart b/test/smtp/mock_smtp_server.dart index 2823ed01..e231baf8 100644 --- a/test/smtp/mock_smtp_server.dart +++ b/test/smtp/mock_smtp_server.dart @@ -20,6 +20,9 @@ class MockSmtpServer { } String? nextResponse; + + /// Responses for consecutive requests, consumed before [nextResponse] + final List responses = []; final Socket _socket; _MailSendState _sendState = _MailSendState.notStarted; @@ -35,6 +38,8 @@ class MockSmtpServer { return; } else if (request == 'QUIT\r\n') { writeln('221 2.0.0 Bye'); + } else if (responses.isNotEmpty) { + writeln(responses.removeAt(0)); } else if (nextResponse == null || nextResponse.isEmpty) { // // no supported request found, answer with the pre-defined response: writeln('500 Invalid state - define nextResponse for MockSmtpServer'); diff --git a/test/smtp/smtp_client_test.dart b/test/smtp/smtp_client_test.dart index 0ed2a09e..aa859635 100644 --- a/test/smtp/smtp_client_test.dart +++ b/test/smtp/smtp_client_test.dart @@ -70,6 +70,31 @@ void main() { expect(client.serverInfo.supports('NOTTHERE'), isFalse); }); + test('SmtpClient EHLO with bare reply code', () async { + // RFC 5321 section 4.2: the last reply line may consist of the code only + _mockServer.nextResponse = '250-domain.com Hello\r\n250'; + final response = await client.ehlo(); + expect(response.type, SmtpResponseType.success); + expect(response.code, 250); + expect(response.message, isEmpty); + }); + + test('SmtpClient AUTH XOAUTH2 with bare 334 challenge', () async { + // smtp.yandex.ru answers `AUTH XOAUTH2` (sent without an initial + // response) with an empty challenge written as `334` - no space + _mockServer.responses.addAll([ + '334', + '235 2.7.0 Authentication successful', + ]); + final response = await client.authenticate( + 'user@example.com', + 'access-token', + AuthMechanism.xoauth2, + ); + expect(response.type, SmtpResponseType.success); + expect(response.code, 235); + }); + test('SmtpClient login', () async { _mockServer.nextResponse = '235 2.7.0 Authentication successful'; final response = await client.authenticate(_smtpUser, _smtpPassword); diff --git a/test/src/smtp/smtp_response_test.dart b/test/src/smtp/smtp_response_test.dart new file mode 100644 index 00000000..da9c0795 --- /dev/null +++ b/test/src/smtp/smtp_response_test.dart @@ -0,0 +1,54 @@ +import 'package:enough_mail/enough_mail.dart'; +import 'package:test/test.dart'; + +void main() { + group('SmtpResponseLine.parse', () { + test('code with space and text', () { + final line = SmtpResponseLine.parse('250 OK'); + expect(line.code, 250); + expect(line.message, 'OK'); + expect(line.type, SmtpResponseType.success); + }); + + test('code with hyphen and text (multiline reply)', () { + final line = SmtpResponseLine.parse('250-PIPELINING'); + expect(line.code, 250); + expect(line.message, 'PIPELINING'); + }); + + test('bare code without text (RFC 5321 section 4.2)', () { + final line = SmtpResponseLine.parse('250'); + expect(line.code, 250); + expect(line.message, isEmpty); + expect(line.type, SmtpResponseType.success); + }); + + test('code followed only by a separator', () { + expect(SmtpResponseLine.parse('250 ').message, isEmpty); + expect(SmtpResponseLine.parse('250-').message, isEmpty); + }); + + test('non-numeric prefix is kept as message with code 500', () { + final line = SmtpResponseLine.parse('garbage line'); + expect(line.code, 500); + expect(line.message, 'garbage line'); + expect(line.type, SmtpResponseType.fatalError); + }); + + test('line shorter than a reply code does not throw', () { + final line = SmtpResponseLine.parse('25'); + expect(line.code, 500); + expect(line.message, '25'); + }); + }); + + group('SmtpResponse', () { + test('multiline reply ending with a bare code', () { + final response = SmtpResponse(['250-domain.com Hello', '250']); + expect(response.responseLines.length, 2); + expect(response.code, 250); + expect(response.message, isEmpty); + expect(response.isOkStatus, isTrue); + }); + }); +}