From 78e7c412e99bee12019c231e004fdb47074d9728 Mon Sep 17 00:00:00 2001 From: Stephen Beitzel Date: Wed, 22 Dec 2021 14:06:27 -0800 Subject: [PATCH 1/6] Fix #121 by wrapping headers that we might have assembled as too long. --- Sources/SwiftSMTP/Mail.swift | 8 ++-- Sources/SwiftSMTP/String-Wrap.swift | 46 +++++++++++++++++++++++ Tests/SwiftSMTPTests/TestWrapUnwrap.swift | 36 ++++++++++++++++++ 3 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 Sources/SwiftSMTP/String-Wrap.swift create mode 100644 Tests/SwiftSMTPTests/TestWrapUnwrap.swift diff --git a/Sources/SwiftSMTP/Mail.swift b/Sources/SwiftSMTP/Mail.swift index 3f34df3..c4971bc 100644 --- a/Sources/SwiftSMTP/Mail.swift +++ b/Sources/SwiftSMTP/Mail.swift @@ -122,13 +122,13 @@ public struct Mail { dictionary["MESSAGE-ID"] = id dictionary["DATE"] = Date().smtpFormatted dictionary["FROM"] = from.mime - dictionary["TO"] = to.map { $0.mime }.joined(separator: ", ") + dictionary["TO"] = (to.map { $0.mime }.joined(separator: ", ")).wrap() if !cc.isEmpty { - dictionary["CC"] = cc.map { $0.mime }.joined(separator: ", ") + dictionary["CC"] = (cc.map { $0.mime }.joined(separator: ", ")).wrap() } - dictionary["SUBJECT"] = subject.mimeEncoded ?? "" + dictionary["SUBJECT"] = (subject.mimeEncoded ?? "").wrap() dictionary["MIME-VERSION"] = "1.0 (Swift-SMTP)" for (key, value) in additionalHeaders { @@ -136,7 +136,7 @@ public struct Mail { if keyUppercased != "CONTENT-TYPE" && keyUppercased != "CONTENT-DISPOSITION" && keyUppercased != "CONTENT-TRANSFER-ENCODING" { - dictionary[keyUppercased] = value + dictionary[keyUppercased] = value.wrap() } } diff --git a/Sources/SwiftSMTP/String-Wrap.swift b/Sources/SwiftSMTP/String-Wrap.swift new file mode 100644 index 0000000..a255625 --- /dev/null +++ b/Sources/SwiftSMTP/String-Wrap.swift @@ -0,0 +1,46 @@ +// +// String-Wrap.swift +// SwiftSMTP +// +// Created by Stephen Beitzel on 12/22/21. +// + +import Foundation + +/// This extension is intended to encapsulate the wrap/unwrap +/// functionality described at https://datatracker.ietf.org/doc/html/rfc5322#section-2.2.3 + +extension String { + + /// Given the desired line length, split this string into + /// a sequence of lines of nearly that length, then join them + /// back together with the sequence `\r\n ` + public func wrap(_ to: Int = 78) -> String { + /* + The RFC says that we should try to wrap lines at logical separator + tokens, such as the comma-space separating email addresses or the + whitespace separating words, but that it is not actually required. + This implementation just splits at the suggested length, without + trying to be smart about tokens, since we don't know what the tokens + might be in a given string. + */ + var lines: [String] = [] + var workingString = self + while workingString.count > to { + lines.append(String(workingString.prefix(to))) + let sliceIndex = workingString.index(startIndex, offsetBy: to) + workingString = String(workingString.suffix(from: sliceIndex)) + } + lines.append(workingString) + return lines.joined(separator: "\r\n ") + } + + /// Find all occurrences of CRLF followed by a whitespace and + /// replace them with an empty string. + public func unwrap() -> String { + let regex = try! NSRegularExpression(pattern: "\r\n\\s", options: []) + let range = NSMakeRange(0, count) + let modString = regex.stringByReplacingMatches(in: self, options: [], range: range, withTemplate: "") + return modString + } +} diff --git a/Tests/SwiftSMTPTests/TestWrapUnwrap.swift b/Tests/SwiftSMTPTests/TestWrapUnwrap.swift new file mode 100644 index 0000000..1e045d8 --- /dev/null +++ b/Tests/SwiftSMTPTests/TestWrapUnwrap.swift @@ -0,0 +1,36 @@ +// +// TestWrapUnwrap.swift +// SwiftSMTPTests +// +// Created by Stephen Beitzel on 12/22/21. +// + +@testable import SwiftSMTP +import XCTest + +class TestWrapUnwrap: XCTestCase { + var reallyLongLine: String = "" + + override func setUpWithError() throws { + let address = "test_address@dumbster.local" + reallyLongLine = "To: " + for _ in 1...34 { + reallyLongLine.append(address) + reallyLongLine.append(", ") + } + reallyLongLine.append(address) + } + + func testWrap() throws { + let wrapped = reallyLongLine.wrap(78) + let lines = wrapped.split(separator: "\r\n") + XCTAssertEqual(lines.count, 14) + } + + func testUnwrap() throws { + let wrapped = "Subject: This\r\n is a test" + let unwrapped = wrapped.unwrap() + + XCTAssertEqual(unwrapped, "Subject: This is a test") + } +} From ac30e260ccbdf1cf27863c9e47fe1bf9ae1d0668 Mon Sep 17 00:00:00 2001 From: Stephen Beitzel Date: Wed, 22 Dec 2021 15:01:34 -0800 Subject: [PATCH 2/6] Update file headers with Apache License, per the contributor guidelines --- Sources/SwiftSMTP/String-Wrap.swift | 21 +++++++++++++++------ Tests/SwiftSMTPTests/TestWrapUnwrap.swift | 21 +++++++++++++++------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/Sources/SwiftSMTP/String-Wrap.swift b/Sources/SwiftSMTP/String-Wrap.swift index a255625..4309f7d 100644 --- a/Sources/SwiftSMTP/String-Wrap.swift +++ b/Sources/SwiftSMTP/String-Wrap.swift @@ -1,9 +1,18 @@ -// -// String-Wrap.swift -// SwiftSMTP -// -// Created by Stephen Beitzel on 12/22/21. -// +/** + * Copyright Stephen Beitzel 2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ import Foundation diff --git a/Tests/SwiftSMTPTests/TestWrapUnwrap.swift b/Tests/SwiftSMTPTests/TestWrapUnwrap.swift index 1e045d8..e261e30 100644 --- a/Tests/SwiftSMTPTests/TestWrapUnwrap.swift +++ b/Tests/SwiftSMTPTests/TestWrapUnwrap.swift @@ -1,9 +1,18 @@ -// -// TestWrapUnwrap.swift -// SwiftSMTPTests -// -// Created by Stephen Beitzel on 12/22/21. -// +/** + * Copyright Stephen Beitzel 2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ @testable import SwiftSMTP import XCTest From 49faad1d1f5229d754b5660fe1a80c01bfb7ae2a Mon Sep 17 00:00:00 2001 From: Stephen Beitzel Date: Sun, 2 Jan 2022 15:19:59 -0800 Subject: [PATCH 3/6] Renamed the files with fold/unfold in mind. Modified unit tests to work against Mail, since that's actually what we care about. --- .../{String-Wrap.swift => FoldHeader.swift} | 0 Tests/SwiftSMTPTests/TestFoldUnfold.swift | 91 +++++++++++++++++++ Tests/SwiftSMTPTests/TestWrapUnwrap.swift | 45 --------- 3 files changed, 91 insertions(+), 45 deletions(-) rename Sources/SwiftSMTP/{String-Wrap.swift => FoldHeader.swift} (100%) create mode 100644 Tests/SwiftSMTPTests/TestFoldUnfold.swift delete mode 100644 Tests/SwiftSMTPTests/TestWrapUnwrap.swift diff --git a/Sources/SwiftSMTP/String-Wrap.swift b/Sources/SwiftSMTP/FoldHeader.swift similarity index 100% rename from Sources/SwiftSMTP/String-Wrap.swift rename to Sources/SwiftSMTP/FoldHeader.swift diff --git a/Tests/SwiftSMTPTests/TestFoldUnfold.swift b/Tests/SwiftSMTPTests/TestFoldUnfold.swift new file mode 100644 index 0000000..3d2a479 --- /dev/null +++ b/Tests/SwiftSMTPTests/TestFoldUnfold.swift @@ -0,0 +1,91 @@ +/** + * Copyright Stephen Beitzel 2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +@testable import SwiftSMTP +import XCTest + +class TestFoldUnfold: XCTestCase { + let spaciousSubject = "Sample subject with extra whitespace inside" + var mailMessage: Mail? + + // this is a base64 encoding of Package.swift + let egregiousLine = "Ly8gc3dpZnQtdG9vbHMtdmVyc2lvbjo1LjAKCmltcG9ydCBQYWNrYWdlRGVzY3JpcHRpb24KCmxldCBwYWNrYWdlID0gUGFja2FnZSgKICAgIG5hbWU6ICJTd2lmdFNNVFAiLAogICAgcHJvZHVjdHM6IFsKICAgICAgICAubGlicmFyeSgKICAgICAgICAgICAgbmFtZTogIlN3aWZ0U01UUCIsCiAgICAgICAgICAgIHRhcmdldHM6IFsiU3dpZnRTTVRQIl0pLAogICAgICAgIF0sCiAgICBkZXBlbmRlbmNpZXM6IFsKICAgICAgICAucGFja2FnZSh1cmw6ICJodHRwczovL2dpdGh1Yi5jb20vS2l0dXJhL0JsdWVTb2NrZXQuZ2l0IiwgZnJvbTogIjIuMC4yIiksCiAgICAgICAgLnBhY2thZ2UodXJsOiAiaHR0cHM6Ly9naXRodWIuY29tL0tpdHVyYS9CbHVlU1NMU2VydmljZS5naXQiLCBmcm9tOiAiMi4wLjEiKSwKICAgICAgICAucGFja2FnZSh1cmw6ICJodHRwczovL2dpdGh1Yi5jb20vS2l0dXJhL0JsdWVDcnlwdG9yLmdpdCIsIGZyb206ICIyLjAuMSIpLAogICAgICAgIC5wYWNrYWdlKHVybDogImh0dHBzOi8vZ2l0aHViLmNvbS9LaXR1cmEvTG9nZ2VyQVBJLmdpdCIsIGZyb206ICIxLjkuMjAwIiksCiAgICAgICAgXSwKICAgIHRhcmdldHM6IFsKICAgICAgICAudGFyZ2V0KAogICAgICAgICAgICBuYW1lOiAiU3dpZnRTTVRQIiwKICAgICAgICAgICAgZGVwZW5kZW5jaWVzOiBbIlNvY2tldCIsICJTU0xTZXJ2aWNlIiwgIkNyeXB0b3IiLCAiTG9nZ2VyQVBJIl0pLAogICAgICAgIC50ZXN0VGFyZ2V0KAogICAgICAgICAgICBuYW1lOiAiU3dpZnRTTVRQVGVzdHMiLAogICAgICAgICAgICBkZXBlbmRlbmNpZXM6IFsiU3dpZnRTTVRQIl0pLAogICAgICAgIF0KKQo=" + + override func setUpWithError() throws { + let manyRecipients: [Mail.User] = Array.init(repeating: Mail.User(email: "some_recipient@dumbster.local"), + count: 30) + let absurdlyLongEmail = Mail.User(name: "Unfortunate Joe", + email: "unfortunate_joe_1234567890_123456789_123456783_123456784_123456785_123456786@dumbster.local") + mailMessage = Mail(from: Mail.User(name: "Test User", email: "tester@dumbster.local"), + to: manyRecipients, + cc: [absurdlyLongEmail], + subject: spaciousSubject, + text: "Just some message", + additionalHeaders: ["X-OBNOXIOUS": egregiousLine, + "X-SHORT": spaciousSubject]) + } + + // Here we test two things: 1) a short line will not be folded, + // and 2) a line containing multiple consecutive whitespace characters + // will not be changed. + func testShortHeaderUnchanged() throws { + let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + let unmodified = "X-SHORT: \(spaciousSubject)" + for header in allHeaders where header.hasPrefix("X-SHORT") { + XCTAssertEqual(String(header), unmodified) + } + } + + // This test looks at folding a long name plus email address, which won't + // contain a comma. The fold should happen at the space between the name + // and the email. + func testAbsurdEmail() throws { + let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + var foundCC = false + for header in allHeaders { + if foundCC { + // we are now at the first line *after* the CC: line + XCTAssertEqual(header, " ") + break + } else if header.hasPrefix("CC:") { + foundCC = true + XCTAssertEqual(header, "CC: \("Unfortunate Joe".mimeEncoded!)") + continue + } + } + } + + // This test looks at what happens when a header value does not + // contain any spaces at all, and is thus unfoldable, while still + // being longer than the recommended length. + func testUnfoldableHeader() throws { + let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + for header in allHeaders where header.hasPrefix("X-OBNOXIOUS") { + XCTAssertEqual(header, "X-OBNOXIOUS: \(egregiousLine)") + } + } + + // This test looks at a long list of email addresses. There are plenty + // of commas and whitespaces, so folding is possible. It should happen + // at the whitespace, and not in the middle of an address. + func testFoldOnWhitespace() throws { + let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + for header in allHeaders where header.hasPrefix("TO: ") { + print(header) + XCTAssert(header.hasSuffix("some_recipient@dumbster.local,")) + } + } +} diff --git a/Tests/SwiftSMTPTests/TestWrapUnwrap.swift b/Tests/SwiftSMTPTests/TestWrapUnwrap.swift deleted file mode 100644 index e261e30..0000000 --- a/Tests/SwiftSMTPTests/TestWrapUnwrap.swift +++ /dev/null @@ -1,45 +0,0 @@ -/** - * Copyright Stephen Beitzel 2021 - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - **/ - -@testable import SwiftSMTP -import XCTest - -class TestWrapUnwrap: XCTestCase { - var reallyLongLine: String = "" - - override func setUpWithError() throws { - let address = "test_address@dumbster.local" - reallyLongLine = "To: " - for _ in 1...34 { - reallyLongLine.append(address) - reallyLongLine.append(", ") - } - reallyLongLine.append(address) - } - - func testWrap() throws { - let wrapped = reallyLongLine.wrap(78) - let lines = wrapped.split(separator: "\r\n") - XCTAssertEqual(lines.count, 14) - } - - func testUnwrap() throws { - let wrapped = "Subject: This\r\n is a test" - let unwrapped = wrapped.unwrap() - - XCTAssertEqual(unwrapped, "Subject: This is a test") - } -} From 6163d7e5ef5b165b9051ef233fd79e9f81d1bc1e Mon Sep 17 00:00:00 2001 From: Stephen Beitzel Date: Mon, 3 Jan 2022 12:11:19 -0800 Subject: [PATCH 4/6] Renamed unit test to be more descriptive. Put folding logic in the Mail struct, to keep it close to where it's used. Refactored unit test to remove force unwrapping. --- Sources/SwiftSMTP/FoldHeader.swift | 55 ------------- Sources/SwiftSMTP/Mail.swift | 54 ++++++++++++- ...ldUnfold.swift => TestHeaderFolding.swift} | 81 ++++++++++++++----- 3 files changed, 113 insertions(+), 77 deletions(-) delete mode 100644 Sources/SwiftSMTP/FoldHeader.swift rename Tests/SwiftSMTPTests/{TestFoldUnfold.swift => TestHeaderFolding.swift} (62%) diff --git a/Sources/SwiftSMTP/FoldHeader.swift b/Sources/SwiftSMTP/FoldHeader.swift deleted file mode 100644 index 4309f7d..0000000 --- a/Sources/SwiftSMTP/FoldHeader.swift +++ /dev/null @@ -1,55 +0,0 @@ -/** - * Copyright Stephen Beitzel 2021 - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - **/ - -import Foundation - -/// This extension is intended to encapsulate the wrap/unwrap -/// functionality described at https://datatracker.ietf.org/doc/html/rfc5322#section-2.2.3 - -extension String { - - /// Given the desired line length, split this string into - /// a sequence of lines of nearly that length, then join them - /// back together with the sequence `\r\n ` - public func wrap(_ to: Int = 78) -> String { - /* - The RFC says that we should try to wrap lines at logical separator - tokens, such as the comma-space separating email addresses or the - whitespace separating words, but that it is not actually required. - This implementation just splits at the suggested length, without - trying to be smart about tokens, since we don't know what the tokens - might be in a given string. - */ - var lines: [String] = [] - var workingString = self - while workingString.count > to { - lines.append(String(workingString.prefix(to))) - let sliceIndex = workingString.index(startIndex, offsetBy: to) - workingString = String(workingString.suffix(from: sliceIndex)) - } - lines.append(workingString) - return lines.joined(separator: "\r\n ") - } - - /// Find all occurrences of CRLF followed by a whitespace and - /// replace them with an empty string. - public func unwrap() -> String { - let regex = try! NSRegularExpression(pattern: "\r\n\\s", options: []) - let range = NSMakeRange(0, count) - let modString = regex.stringByReplacingMatches(in: self, options: [], range: range, withTemplate: "") - return modString - } -} diff --git a/Sources/SwiftSMTP/Mail.swift b/Sources/SwiftSMTP/Mail.swift index c4971bc..4158bed 100644 --- a/Sources/SwiftSMTP/Mail.swift +++ b/Sources/SwiftSMTP/Mail.swift @@ -15,6 +15,7 @@ **/ import Foundation +import LoggerAPI /// Represents an email that can be sent through an `SMTP` instance. public struct Mail { @@ -122,13 +123,13 @@ public struct Mail { dictionary["MESSAGE-ID"] = id dictionary["DATE"] = Date().smtpFormatted dictionary["FROM"] = from.mime - dictionary["TO"] = (to.map { $0.mime }.joined(separator: ", ")).wrap() + dictionary["TO"] = foldHeaderValue(key: "TO", value: to.map { $0.mime }.joined(separator: ", ")) if !cc.isEmpty { - dictionary["CC"] = (cc.map { $0.mime }.joined(separator: ", ")).wrap() + dictionary["CC"] = foldHeaderValue(key: "CC", value: cc.map { $0.mime }.joined(separator: ", ")) } - dictionary["SUBJECT"] = (subject.mimeEncoded ?? "").wrap() + dictionary["SUBJECT"] = foldHeaderValue(key: "SUBJECT", value: (subject.mimeEncoded ?? "")) dictionary["MIME-VERSION"] = "1.0 (Swift-SMTP)" for (key, value) in additionalHeaders { @@ -136,13 +137,58 @@ public struct Mail { if keyUppercased != "CONTENT-TYPE" && keyUppercased != "CONTENT-DISPOSITION" && keyUppercased != "CONTENT-TRANSFER-ENCODING" { - dictionary[keyUppercased] = value.wrap() + dictionary[keyUppercased] = foldHeaderValue(key: key, value: value) } } return dictionary } + private func foldHeaderValue(key: String, value: String) -> String { + let initialHeader = "\(key): \(value)" + if initialHeader.count < 79 { + return value + } + // if we're here, it means that RFC 5322 SUGGESTS that we fold this header + var foldedHeader = "" + var register = "\(key): " + var linePosition = 0 + let foldableCharacters = CharacterSet(charactersIn: " ,") + for char in value { + // append the character to the register + register.append(char) + // this test is to detect the end of a token, mid-stream + if let _ = String(char).rangeOfCharacter(from: foldableCharacters) { + if linePosition > 1 && (register.count + linePosition > 78) { + // we already have stuff on the line and the register is too long + // to continue on the current line. So we fold and start a new line. + foldedHeader.append("\r\n ") + linePosition = 1 + } + // now, register contains a complete token, so we put it up to the line + linePosition += register.count + if linePosition > 998 { + Log.error("Header line length exceeds the specified maximum (998 chars) - see RFC 5322 section 2.2.3") + } + foldedHeader.append(register) + register = "" + } + } + // we have the last of the value characters in register, so we put them up + // to the line. We still want to apply the same logic as that inside the loop + // though, and apply folding if it's appropriate. + if linePosition > 1 && (register.count + linePosition > 78) { + foldedHeader.append("\r\n ") + } + if register.count > 997 { + Log.error("Header line length exceeds the specified maximum (998 chars) - see RFC 5322 section 2.2.3") + } + foldedHeader.append(register) + + let valueIndex = foldedHeader.index(foldedHeader.startIndex, offsetBy: key.count + 2) + return String(foldedHeader.suffix(from: valueIndex)) + } + var headersString: String { return headersDictionary.map { (key, value) in return "\(key): \(value)" diff --git a/Tests/SwiftSMTPTests/TestFoldUnfold.swift b/Tests/SwiftSMTPTests/TestHeaderFolding.swift similarity index 62% rename from Tests/SwiftSMTPTests/TestFoldUnfold.swift rename to Tests/SwiftSMTPTests/TestHeaderFolding.swift index 3d2a479..b4db6c4 100644 --- a/Tests/SwiftSMTPTests/TestFoldUnfold.swift +++ b/Tests/SwiftSMTPTests/TestHeaderFolding.swift @@ -15,21 +15,24 @@ **/ @testable import SwiftSMTP +import LoggerAPI import XCTest -class TestFoldUnfold: XCTestCase { - let spaciousSubject = "Sample subject with extra whitespace inside" - var mailMessage: Mail? +internal class TestLogger: Logger { + var messages = [String]() - // this is a base64 encoding of Package.swift - let egregiousLine = "Ly8gc3dpZnQtdG9vbHMtdmVyc2lvbjo1LjAKCmltcG9ydCBQYWNrYWdlRGVzY3JpcHRpb24KCmxldCBwYWNrYWdlID0gUGFja2FnZSgKICAgIG5hbWU6ICJTd2lmdFNNVFAiLAogICAgcHJvZHVjdHM6IFsKICAgICAgICAubGlicmFyeSgKICAgICAgICAgICAgbmFtZTogIlN3aWZ0U01UUCIsCiAgICAgICAgICAgIHRhcmdldHM6IFsiU3dpZnRTTVRQIl0pLAogICAgICAgIF0sCiAgICBkZXBlbmRlbmNpZXM6IFsKICAgICAgICAucGFja2FnZSh1cmw6ICJodHRwczovL2dpdGh1Yi5jb20vS2l0dXJhL0JsdWVTb2NrZXQuZ2l0IiwgZnJvbTogIjIuMC4yIiksCiAgICAgICAgLnBhY2thZ2UodXJsOiAiaHR0cHM6Ly9naXRodWIuY29tL0tpdHVyYS9CbHVlU1NMU2VydmljZS5naXQiLCBmcm9tOiAiMi4wLjEiKSwKICAgICAgICAucGFja2FnZSh1cmw6ICJodHRwczovL2dpdGh1Yi5jb20vS2l0dXJhL0JsdWVDcnlwdG9yLmdpdCIsIGZyb206ICIyLjAuMSIpLAogICAgICAgIC5wYWNrYWdlKHVybDogImh0dHBzOi8vZ2l0aHViLmNvbS9LaXR1cmEvTG9nZ2VyQVBJLmdpdCIsIGZyb206ICIxLjkuMjAwIiksCiAgICAgICAgXSwKICAgIHRhcmdldHM6IFsKICAgICAgICAudGFyZ2V0KAogICAgICAgICAgICBuYW1lOiAiU3dpZnRTTVRQIiwKICAgICAgICAgICAgZGVwZW5kZW5jaWVzOiBbIlNvY2tldCIsICJTU0xTZXJ2aWNlIiwgIkNyeXB0b3IiLCAiTG9nZ2VyQVBJIl0pLAogICAgICAgIC50ZXN0VGFyZ2V0KAogICAgICAgICAgICBuYW1lOiAiU3dpZnRTTVRQVGVzdHMiLAogICAgICAgICAgICBkZXBlbmRlbmNpZXM6IFsiU3dpZnRTTVRQIl0pLAogICAgICAgIF0KKQo=" + func log(_ type: LoggerMessageType, msg: String, functionName: String, lineNum: Int, fileName: String) { + let message = "[\(type.description.uppercased())] \(msg)" + messages.append(message) + } - override func setUpWithError() throws { - let manyRecipients: [Mail.User] = Array.init(repeating: Mail.User(email: "some_recipient@dumbster.local"), - count: 30) - let absurdlyLongEmail = Mail.User(name: "Unfortunate Joe", - email: "unfortunate_joe_1234567890_123456789_123456783_123456784_123456785_123456786@dumbster.local") - mailMessage = Mail(from: Mail.User(name: "Test User", email: "tester@dumbster.local"), + func isLogging(_ level: LoggerMessageType) -> Bool { true } +} + +class TestHeaderFolding: XCTestCase { + let spaciousSubject = "Sample subject with extra whitespace inside" + var mailMessage: Mail { + Mail(from: Mail.User(name: "Test User", email: "tester@dumbster.local"), to: manyRecipients, cc: [absurdlyLongEmail], subject: spaciousSubject, @@ -38,22 +41,53 @@ class TestFoldUnfold: XCTestCase { "X-SHORT": spaciousSubject]) } + let manyRecipients: [Mail.User] = Array.init(repeating: Mail.User(email: "some_recipient@dumbster.local"), + count: 30) + let absurdlyLongEmail = Mail.User(name: "Unfortunate Joe", + email: "unfortunate_joe_1234567890_123456789_123456783_123456784_123456785_123456786@dumbster.local") + + // this is a base64 encoding of Package.swift + let egregiousLine = "Ly8gc3dpZnQtdG9vbHMtdmVyc2lvbjo1LjAKCmltcG9ydCBQYWNrYWdlRGVzY3JpcHRpb24KCmxldCBwYWNrYWdlID0gUGFja2FnZSgKICAgIG5hbWU6ICJTd2lmdFNNVFAiLAogICAgcHJvZHVjdHM6IFsKICAgICAgICAubGlicmFyeSgKICAgICAgICAgICAgbmFtZTogIlN3aWZ0U01UUCIsCiAgICAgICAgICAgIHRhcmdldHM6IFsiU3dpZnRTTVRQIl0pLAogICAgICAgIF0sCiAgICBkZXBlbmRlbmNpZXM6IFsKICAgICAgICAucGFja2FnZSh1cmw6ICJodHRwczovL2dpdGh1Yi5jb20vS2l0dXJhL0JsdWVTb2NrZXQuZ2l0IiwgZnJvbTogIjIuMC4yIiksCiAgICAgICAgLnBhY2thZ2UodXJsOiAiaHR0cHM6Ly9naXRodWIuY29tL0tpdHVyYS9CbHVlU1NMU2VydmljZS5naXQiLCBmcm9tOiAiMi4wLjEiKSwKICAgICAgICAucGFja2FnZSh1cmw6ICJodHRwczovL2dpdGh1Yi5jb20vS2l0dXJhL0JsdWVDcnlwdG9yLmdpdCIsIGZyb206ICIyLjAuMSIpLAogICAgICAgIC5wYWNrYWdlKHVybDogImh0dHBzOi8vZ2l0aHViLmNvbS9LaXR1cmEvTG9nZ2VyQVBJLmdpdCIsIGZyb206ICIxLjkuMjAwIiksCiAgICAgICAgXSwKICAgIHRhcmdldHM6IFsKICAgICAgICAudGFyZ2V0KAogICAgICAgICAgICBuYW1lOiAiU3dpZnRTTVRQIiwKICAgICAgICAgICAgZGVwZW5kZW5jaWVzOiBbIlNvY2tldCIsICJTU0xTZXJ2aWNlIiwgIkNyeXB0b3IiLCAiTG9nZ2VyQVBJIl0pLAogICAgICAgIC50ZXN0VGFyZ2V0KAogICAgICAgICAgICBuYW1lOiAiU3dpZnRTTVRQVGVzdHMiLAogICAgICAgICAgICBkZXBlbmRlbmNpZXM6IFsiU3dpZnRTTVRQIl0pLAogICAgICAgIF0KKQo=" + + var previousLogger: Logger? + var currentLogger: TestLogger? + + override func setUpWithError() throws { + previousLogger = Log.logger + currentLogger = TestLogger() + Log.logger = currentLogger + } + + override func tearDownWithError() throws { + Log.logger = previousLogger + currentLogger = nil + } + // Here we test two things: 1) a short line will not be folded, // and 2) a line containing multiple consecutive whitespace characters // will not be changed. func testShortHeaderUnchanged() throws { - let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + let allHeaders = mailMessage.headersString.split(separator: "\r\n") let unmodified = "X-SHORT: \(spaciousSubject)" for header in allHeaders where header.hasPrefix("X-SHORT") { XCTAssertEqual(String(header), unmodified) } } + // Whatever else is true of the folded header, the folding process should + // not ever produce a line which consists solely of whitespace. + func testNoWSOnlyLines() throws { + let allHeaders = mailMessage.headersString.split(separator: "\r\n") + for header in allHeaders { + XCTAssert(!header.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) + } + } + // This test looks at folding a long name plus email address, which won't // contain a comma. The fold should happen at the space between the name // and the email. func testAbsurdEmail() throws { - let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + let allHeaders = mailMessage.headersString.split(separator: "\r\n") var foundCC = false for header in allHeaders { if foundCC { @@ -62,7 +96,7 @@ class TestFoldUnfold: XCTestCase { break } else if header.hasPrefix("CC:") { foundCC = true - XCTAssertEqual(header, "CC: \("Unfortunate Joe".mimeEncoded!)") + XCTAssertEqual(header, "CC: \("Unfortunate Joe".mimeEncoded!) ") continue } } @@ -72,20 +106,31 @@ class TestFoldUnfold: XCTestCase { // contain any spaces at all, and is thus unfoldable, while still // being longer than the recommended length. func testUnfoldableHeader() throws { - let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + let allHeaders = mailMessage.headersString.split(separator: "\r\n") for header in allHeaders where header.hasPrefix("X-OBNOXIOUS") { XCTAssertEqual(header, "X-OBNOXIOUS: \(egregiousLine)") } + // Incidentally, the line is longer than the RFC mandated maximum length. + // Currently, the library does not throw an error in this case, but + // the folding code should at least log a message about it, so that + // consumers of the library can have some help figuring out why their + // mail message was rejected by a remote SMTP server. + guard let currentLogger = currentLogger else { + return + } + XCTAssert(!currentLogger.messages + .filter({ $0.hasPrefix("[ERROR] Header line length") }) + .isEmpty) } // This test looks at a long list of email addresses. There are plenty // of commas and whitespaces, so folding is possible. It should happen // at the whitespace, and not in the middle of an address. func testFoldOnWhitespace() throws { - let allHeaders = mailMessage!.headersString.split(separator: "\r\n") + let allHeaders = mailMessage + .headersString.split(separator: "\r\n") for header in allHeaders where header.hasPrefix("TO: ") { - print(header) - XCTAssert(header.hasSuffix("some_recipient@dumbster.local,")) + XCTAssert(header.hasSuffix("some_recipient@dumbster.local, ")) } } } From 6e784f938fb96ab04a5e8174aa4fb230ecf34f66 Mon Sep 17 00:00:00 2001 From: Stephen Beitzel Date: Fri, 7 Jan 2022 10:09:47 -0800 Subject: [PATCH 5/6] Isolate magic numbers in the folding code. Minor touch-ups to unit tests. --- Sources/SwiftSMTP/Mail.swift | 20 +++++++++++++------- Tests/SwiftSMTPTests/TestHeaderFolding.swift | 8 +++++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/Sources/SwiftSMTP/Mail.swift b/Sources/SwiftSMTP/Mail.swift index 4158bed..99e01da 100644 --- a/Sources/SwiftSMTP/Mail.swift +++ b/Sources/SwiftSMTP/Mail.swift @@ -145,8 +145,11 @@ public struct Mail { } private func foldHeaderValue(key: String, value: String) -> String { + let suggestedLineLength = 78 + let maximumLineLength = 998 + let initialHeader = "\(key): \(value)" - if initialHeader.count < 79 { + if initialHeader.count <= suggestedLineLength { return value } // if we're here, it means that RFC 5322 SUGGESTS that we fold this header @@ -159,32 +162,35 @@ public struct Mail { register.append(char) // this test is to detect the end of a token, mid-stream if let _ = String(char).rangeOfCharacter(from: foldableCharacters) { - if linePosition > 1 && (register.count + linePosition > 78) { - // we already have stuff on the line and the register is too long + if linePosition > 1 && (register.count + linePosition > suggestedLineLength) { + // We already have stuff on the line and the register is too long // to continue on the current line. So we fold and start a new line. foldedHeader.append("\r\n ") linePosition = 1 } // now, register contains a complete token, so we put it up to the line linePosition += register.count - if linePosition > 998 { + if linePosition > maximumLineLength { Log.error("Header line length exceeds the specified maximum (998 chars) - see RFC 5322 section 2.2.3") } foldedHeader.append(register) register = "" } } - // we have the last of the value characters in register, so we put them up + // We have the last of the value characters in register, so we put them up // to the line. We still want to apply the same logic as that inside the loop // though, and apply folding if it's appropriate. - if linePosition > 1 && (register.count + linePosition > 78) { + if linePosition > 1 && (register.count + linePosition > suggestedLineLength) { foldedHeader.append("\r\n ") + linePosition = 1 } - if register.count > 997 { + if (register.count + linePosition) > maximumLineLength { Log.error("Header line length exceeds the specified maximum (998 chars) - see RFC 5322 section 2.2.3") } foldedHeader.append(register) + // Here is where we remove "\(key): " from the beginning of the folded header, + // so we only return the value. let valueIndex = foldedHeader.index(foldedHeader.startIndex, offsetBy: key.count + 2) return String(foldedHeader.suffix(from: valueIndex)) } diff --git a/Tests/SwiftSMTPTests/TestHeaderFolding.swift b/Tests/SwiftSMTPTests/TestHeaderFolding.swift index b4db6c4..8ddbde4 100644 --- a/Tests/SwiftSMTPTests/TestHeaderFolding.swift +++ b/Tests/SwiftSMTPTests/TestHeaderFolding.swift @@ -89,17 +89,19 @@ class TestHeaderFolding: XCTestCase { func testAbsurdEmail() throws { let allHeaders = mailMessage.headersString.split(separator: "\r\n") var foundCC = false + var testEncountered = false for header in allHeaders { if foundCC { // we are now at the first line *after* the CC: line XCTAssertEqual(header, " ") + testEncountered = true break - } else if header.hasPrefix("CC:") { + } else if header.hasPrefix("CC:") { foundCC = true XCTAssertEqual(header, "CC: \("Unfortunate Joe".mimeEncoded!) ") - continue } } + XCTAssertTrue(testEncountered, "The CC line did not get folded!") } // This test looks at what happens when a header value does not @@ -116,7 +118,7 @@ class TestHeaderFolding: XCTestCase { // consumers of the library can have some help figuring out why their // mail message was rejected by a remote SMTP server. guard let currentLogger = currentLogger else { - return + throw XCTSkip("There is no logger installed!") } XCTAssert(!currentLogger.messages .filter({ $0.hasPrefix("[ERROR] Header line length") }) From b549ca52cc123061bc6f8ca68a9ead9ff9c04905 Mon Sep 17 00:00:00 2001 From: Stephen Beitzel Date: Mon, 10 Jan 2022 14:26:10 -0800 Subject: [PATCH 6/6] Update copyright with project --- Tests/SwiftSMTPTests/TestHeaderFolding.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SwiftSMTPTests/TestHeaderFolding.swift b/Tests/SwiftSMTPTests/TestHeaderFolding.swift index 8ddbde4..5c709cf 100644 --- a/Tests/SwiftSMTPTests/TestHeaderFolding.swift +++ b/Tests/SwiftSMTPTests/TestHeaderFolding.swift @@ -1,5 +1,5 @@ /** - * Copyright Stephen Beitzel 2021 + * Copyright Kitura 2021-2022 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.