diff --git a/internal/bridge/user_event_test.go b/internal/bridge/user_event_test.go index 2cc777d2..9c5bcaff 100644 --- a/internal/bridge/user_event_test.go +++ b/internal/bridge/user_event_test.go @@ -784,6 +784,48 @@ func TestBridge_User_HandleParentLabelRename(t *testing.T) { }) } +// TBD: GODT-2527. +func _TestBridge503DuringEventDoesNotCauseBadEvent(t *testing.T) { //nolint:unused,deadcode + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { + // Create a user. + userID, addrID, err := s.CreateUser("user", password) + require.NoError(t, err) + + labelID, err := s.CreateLabel(userID, "folder", "", proton.LabelTypeFolder) + require.NoError(t, err) + + // Create 10 messages for the user. + withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) { + createNumMessages(ctx, t, c, addrID, labelID, 10) + }) + + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { + userLoginAndSync(ctx, t, bridge, "user", password) + + var messageIDs []string + + // Create 10 more messages for the user, generating events. + withClient(ctx, t, s, "user", password, func(ctx context.Context, c *proton.Client) { + messageIDs = createNumMessages(ctx, t, c, addrID, labelID, 10) + }) + + mocks.Reporter.EXPECT().ReportMessageWithContext(gomock.Any(), gomock.Any()).MinTimes(1) + + s.AddStatusHook(func(req *http.Request) (int, bool) { + if xslices.Index(xslices.Map(messageIDs[0:5], func(messageID string) string { + return "/mail/v4/messages/" + messageID + }), req.URL.Path) < 0 { + return 0, false + } + + return http.StatusServiceUnavailable, true + }) + + userContinueEventProcess(ctx, t, s, bridge) + }) + }) +} + // userLoginAndSync logs in user and waits until user is fully synced. func userLoginAndSync( ctx context.Context, diff --git a/internal/certs/cert_store_darwin.go b/internal/certs/cert_store_darwin.go index c034adec..8e1f9570 100644 --- a/internal/certs/cert_store_darwin.go +++ b/internal/certs/cert_store_darwin.go @@ -17,69 +17,141 @@ package certs -import ( - "os" +/* +#cgo CFLAGS: -x objective-c +#cgo LDFLAGS: -framework Foundation -framework Security +#import +#import - "golang.org/x/sys/execabs" + +int installTrustedCert(char const *bytes, unsigned long long length) { + if (length == 0) { + return errSecInvalidData; + } + + NSData *der = [NSData dataWithBytes:bytes length:length]; + + // Step 1. Import the certificate in the keychain. + SecCertificateRef cert = SecCertificateCreateWithData(NULL, (CFDataRef) der); + NSDictionary* addQuery = @{ + (id)kSecValueRef: (__bridge id) cert, + (id)kSecClass: (id)kSecClassCertificate, + }; + + OSStatus status = SecItemAdd((__bridge CFDictionaryRef) addQuery, NULL); + if ((errSecSuccess != status) && (errSecDuplicateItem != status)) { + CFRelease(cert); + return status; + } + + // Step 2. Set the trust for the certificate. + SecPolicyRef policy = SecPolicyCreateSSL(true, NULL); // we limit our trust to SSL + NSDictionary *trustSettings = @{ + (id)kSecTrustSettingsResult: [NSNumber numberWithInt:kSecTrustSettingsResultTrustRoot], + (id)kSecTrustSettingsPolicy: (__bridge id) policy, + }; + status = SecTrustSettingsSetTrustSettings(cert, kSecTrustSettingsDomainAdmin, (__bridge CFTypeRef)(trustSettings)); + CFRelease(policy); + CFRelease(cert); + + return status; +} + + +int removeTrustedCert(char const *bytes, unsigned long long length) { + if (0 == length) { + return errSecInvalidData; + } + + NSData *der = [NSData dataWithBytes: bytes length: length]; + SecCertificateRef cert = SecCertificateCreateWithData(NULL, (CFDataRef) der); + + // Step 1. Unset the trust for the certificate. + SecPolicyRef policy = SecPolicyCreateSSL(true, NULL); + NSDictionary * trustSettings = @{ + (id)kSecTrustSettingsResult: [NSNumber numberWithInt:kSecTrustSettingsResultUnspecified], + (id)kSecTrustSettingsPolicy: (__bridge id) policy, + }; + OSStatus status = SecTrustSettingsSetTrustSettings(cert, kSecTrustSettingsDomainAdmin, (__bridge CFTypeRef)(trustSettings)); + CFRelease(policy); + if (errSecSuccess != status) { + CFRelease(cert); + return status; + } + + // Step 2. Remove the certificate from the keychain. + NSDictionary *query = @{ (id)kSecClass: (id)kSecClassCertificate, + (id)kSecMatchItemList: @[(__bridge id)cert], + (id)kSecMatchLimit: (id)kSecMatchLimitOne, + }; + status = SecItemDelete((__bridge CFDictionaryRef) query); + + CFRelease(cert); + return status; +} +*/ +import "C" + +import ( + "encoding/pem" + "errors" + "fmt" + "unsafe" ) +// some of the error codes returned by Apple's Security framework. +const ( + errSecSuccess = 0 + errAuthorizationCanceled = -60006 +) + +// certPEMToDER converts a certificate in PEM format to DER format, which is the format required by Apple's Security framework. +func certPEMToDER(certPEM []byte) ([]byte, error) { + + block, left := pem.Decode(certPEM) + if block == nil { + return []byte{}, errors.New("invalid PEM certificate") + } + + if len(left) > 0 { + return []byte{}, errors.New("trailing data found at the end of a PEM certificate") + } + + return block.Bytes, nil +} + func installCert(certPEM []byte) error { - name, err := writeToTempFile(certPEM) + certDER, err := certPEMToDER(certPEM) if err != nil { return err } - return addTrustedCert(name) + p := C.CBytes(certDER) + defer C.free(unsafe.Pointer(p)) + + errCode := C.installTrustedCert((*C.char)(p), (C.ulonglong)(len(certDER))) + switch errCode { + case errSecSuccess: + return nil + case errAuthorizationCanceled: + return fmt.Errorf("the user cancelled the authorization dialog") + default: + return fmt.Errorf("could not install certification into keychain (error %v)", errCode) + } } func uninstallCert(certPEM []byte) error { - name, err := writeToTempFile(certPEM) + certDER, err := certPEMToDER(certPEM) if err != nil { return err } - return removeTrustedCert(name) -} + p := C.CBytes(certDER) + defer C.free(unsafe.Pointer(p)) -func addTrustedCert(certPath string) error { - return execabs.Command( //nolint:gosec - "/usr/bin/security", - "execute-with-privileges", - "/usr/bin/security", - "add-trusted-cert", - "-d", - "-r", "trustRoot", - "-p", "ssl", - "-k", "/Library/Keychains/System.keychain", - certPath, - ).Run() -} - -func removeTrustedCert(certPath string) error { - return execabs.Command( //nolint:gosec - "/usr/bin/security", - "execute-with-privileges", - "/usr/bin/security", - "remove-trusted-cert", - "-d", - certPath, - ).Run() -} - -// writeToTempFile writes the given data to a temporary file and returns the path. -func writeToTempFile(data []byte) (string, error) { - f, err := os.CreateTemp("", "tls") - if err != nil { - return "", err + if errCode := C.removeTrustedCert((*C.char)(p), (C.ulonglong)(len(certDER))); errCode != 0 { + return fmt.Errorf("could not install certificate from keychain (error %v)", errCode) } - if _, err := f.Write(data); err != nil { - return "", err - } - - if err := f.Close(); err != nil { - return "", err - } - - return f.Name(), nil + return nil } diff --git a/internal/certs/cert_store_darwin_test.go b/internal/certs/cert_store_darwin_test.go new file mode 100644 index 00000000..9ac9c698 --- /dev/null +++ b/internal/certs/cert_store_darwin_test.go @@ -0,0 +1,44 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + +//go:build darwin + +package certs + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// This test implies human interactions to enter password and is disabled by default. +func _TestTrustedCertsDarwin(t *testing.T) { + template, err := NewTLSTemplate() + require.NoError(t, err) + + certPEM, _, err := GenerateCert(template) + require.NoError(t, err) + + require.Error(t, installCert([]byte{0})) // Cannot install an invalid cert. + require.Error(t, uninstallCert(certPEM)) // Cannot uninstall a cert that is not installed. + require.NoError(t, installCert(certPEM)) // Can install a valid cert. + require.NoError(t, installCert(certPEM)) // Can install an already installed cert. + require.NoError(t, uninstallCert(certPEM)) // Can uninstall an installed cert. + require.Error(t, uninstallCert(certPEM)) // Cannot uninstall an already uninstalled cert. + require.NoError(t, installCert(certPEM)) // Can reinstall an uninstalled cert. + require.NoError(t, uninstallCert(certPEM)) // Can uninstall a reinstalled cert. +} diff --git a/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt b/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt index af03519a..9c5eb38f 100644 --- a/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt +++ b/internal/frontend/bridge-gui/bridgepp/CMakeLists.txt @@ -160,3 +160,41 @@ target_link_libraries(bridgepp ) target_precompile_headers(bridgepp PRIVATE Pch.h) + +#***************************************************************************************************************************************************** +# GoogleTest +#***************************************************************************************************************************************************** + +if (CMAKE_VERSION VERSION_GREATER_EQUAL "3.24.0") + cmake_policy(SET CMP0135 NEW) # avoid warning DOWNLOAD_EXTRACT_TIMESTAMP +endif() + +include(FetchContent) +FetchContent_Declare( + googletest + URL https://github.com/google/googletest/archive/b796f7d44681514f58a683a3a71ff17c94edb0c1.zip +) + +# For Windows: Prevent overriding the parent project's compiler/linker settings +set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) + +FetchContent_MakeAvailable(googletest) + +enable_testing() + +#***************************************************************************************************************************************************** +# Tests +#***************************************************************************************************************************************************** +add_executable(bridgepp-test + Test/Exception/TestBridgeUtils.cpp + Test/Exception/TestException.cpp + ) +add_dependencies(bridgepp-test bridgepp) +target_precompile_headers(bridgepp-test PRIVATE Pch.h) +target_link_libraries(bridgepp-test + GTest::gtest_main + bridgepp + ) + +include(GoogleTest) +gtest_discover_tests(bridgepp-test) diff --git a/internal/frontend/bridge-gui/bridgepp/Test/Exception/TestBridgeUtils.cpp b/internal/frontend/bridge-gui/bridgepp/Test/Exception/TestBridgeUtils.cpp new file mode 100644 index 00000000..a990eb66 --- /dev/null +++ b/internal/frontend/bridge-gui/bridgepp/Test/Exception/TestBridgeUtils.cpp @@ -0,0 +1,111 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + + +#include +#include + + +using namespace bridgepp; + + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +TEST(BridgeUtils, OS) { +#ifdef Q_OS_MACOS + EXPECT_EQ(os(), OS::MacOS); + EXPECT_FALSE(onLinux()); + EXPECT_TRUE(onMacOS()); + EXPECT_FALSE(onWindows()); + EXPECT_EQ(goos(), "darwin"); + return; +#endif + +#ifdef Q_OS_WIN + EXPECT_EQ(os(), OS::Windows); + EXPECT_FALSE(onLinux()); + EXPECT_FALSE(onMacOS()); + EXPECT_TRUE(onWindows()); + EXPECT_EQ(goos(), "windows"); + return; +#endif + +#ifdef Q_OS_LINUX + EXPECT_EQ(os(), OS::Linux); + EXPECT_TRUE(onLinux()); + EXPECT_FALSE(onMacOS()); + EXPECT_FALSE(onWindows()); + EXPECT_EQ(goos(), "linux"); + return; +#endif + + EXPECT_TRUE(false); // should be unreachable. +} + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +TEST(BridgeUtils, UserFolders) { + typedef QString (*dirFunction)(); + QList functions = { userConfigDir, userCacheDir, userDataDir, sentryCacheDir }; + QString path; + for (dirFunction f: functions) { + EXPECT_NO_THROW(path = f()); + EXPECT_FALSE(path.isEmpty()); + EXPECT_TRUE(QDir(path).exists()); + } +} + + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +TEST(BridgeUtils, Random) { + qint32 repeatCount = 1000; + qint32 const maxValue = 5; + for (qint32 i = 0; i < repeatCount; ++i) { + qint64 n = 0; + EXPECT_NO_THROW(n = randN(maxValue)); + EXPECT_TRUE((n >= 0) && (n < maxValue)); + QString name; + EXPECT_NO_THROW(name = randomFirstName()); + EXPECT_FALSE(name.isEmpty()); + EXPECT_NO_THROW(name = randomLastName()); + EXPECT_FALSE(name.isEmpty()); + EXPECT_NO_THROW(randomUser()); + } +} + + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +TEST(BridgeUtils, ElideLongString) { + std::function const test = [](QString const &input, qint32 maxLength, QString const &expected) -> bool { + QString output; + EXPECT_NO_THROW(output = elideLongString(input, maxLength)); + return output == expected; + }; + + EXPECT_TRUE(test( "", 0, "")); + EXPECT_TRUE(test("1234", 4, "1234")); + EXPECT_TRUE(test("123", 2, "...")); + EXPECT_TRUE(test("1234567890", 8, "12...90")); + EXPECT_TRUE(test("1234567890", 10, "1234567890")); + EXPECT_TRUE(test("1234567890", 100, "1234567890")); +} diff --git a/internal/frontend/bridge-gui/bridgepp/Test/Exception/TestException.cpp b/internal/frontend/bridge-gui/bridgepp/Test/Exception/TestException.cpp new file mode 100644 index 00000000..9c006f56 --- /dev/null +++ b/internal/frontend/bridge-gui/bridgepp/Test/Exception/TestException.cpp @@ -0,0 +1,95 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + +#include +#include + + +using namespace bridgepp; + + +namespace { + QString const testQWhat = "What"; + QString const testDetails = "Some details"; + QString const testFunction = "function"; + QByteArray const testAttachment = QString("Some data").toLocal8Bit(); + Exception const testException(testQWhat, testDetails, testFunction, testAttachment); +} + + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +TEST(Exceptions, ExceptionConstructor) { + // Default exception + Exception const emptyException; + EXPECT_TRUE(emptyException.qwhat().isEmpty()); + EXPECT_EQ(strlen(emptyException.what()), 0); + EXPECT_EQ(emptyException.attachment().size(), 0); + EXPECT_TRUE(emptyException.details().isEmpty()); + EXPECT_TRUE(emptyException.detailedWhat().isEmpty()); + + // Fully detailed exception + EXPECT_EQ(testException.qwhat(), testQWhat); + EXPECT_EQ(QString::fromLocal8Bit(testException.what()), testQWhat); + EXPECT_EQ(testException.details(), testDetails); + EXPECT_EQ(testException.attachment(), testAttachment); + QString const detailed = testException.detailedWhat(); + EXPECT_TRUE(detailed.contains(testQWhat)); + EXPECT_TRUE(detailed.contains(testFunction)); + EXPECT_TRUE(detailed.contains(testDetails)); +} + + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +TEST(Exceptions, ExceptionCopyMoveConstructors) { + Exception const e(testQWhat, testDetails, testFunction, testAttachment); + + // Check copy-constructor + Exception eCopied(e); + EXPECT_EQ(eCopied.qwhat(), testQWhat); + EXPECT_EQ(eCopied.details(), testDetails); + EXPECT_EQ(eCopied.function(), testFunction); + EXPECT_EQ(eCopied.attachment(), testAttachment); + + // Check move-constructor + Exception eMoved(std::move(eCopied)); + EXPECT_EQ(eMoved.qwhat(), testQWhat); + EXPECT_EQ(eMoved.details(), testDetails); + EXPECT_EQ(eMoved.function(), testFunction); + EXPECT_EQ(eMoved.attachment(), testAttachment); +} + + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +TEST(Exceptions, ExceptionThrow) { + std::function t = []() { throw testException; }; + EXPECT_THROW(t(), Exception); + EXPECT_THROW(t(), std::exception); + bool caught = false; + try { + t(); + } catch (Exception const &e) { + caught = true; + EXPECT_EQ(e.detailedWhat(), testException.detailedWhat()); + } + EXPECT_TRUE(caught); +} diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h index 280a0de9..1ca92901 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/BridgeUtils.h @@ -20,7 +20,7 @@ #define BRIDGE_PP_TESTER_BRIDGE_UTILS_H -#include +#include "User/User.h" namespace bridgepp { diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp index dc614b88..db43cac0 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.cpp @@ -87,6 +87,14 @@ QString Exception::details() const noexcept { } +//**************************************************************************************************************************************************** +/// \return The function that threw the exception. +//**************************************************************************************************************************************************** +QString Exception::function() const noexcept { + return function_; +} + + //**************************************************************************************************************************************************** /// \return The attachment for the exception. //**************************************************************************************************************************************************** @@ -109,4 +117,5 @@ QString Exception::detailedWhat() const { return result; } + } // namespace bridgepp diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h index 21a1689d..3b78821f 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Exception/Exception.h @@ -42,6 +42,7 @@ public: // member functions QString qwhat() const noexcept; ///< Return the description of the exception as a QString const char *what() const noexcept override; ///< Return the description of the exception as C style string QString details() const noexcept; ///< Return the details for the exception + QString function() const noexcept; ///< Return the function that threw the exception. QByteArray attachment() const noexcept; ///< Return the attachment for the exception. QString detailedWhat() const; ///< Return the detailed description of the message (i.e. including the function name and the details). diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp index ee3780b2..eb825792 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/Log/LogUtils.cpp @@ -17,8 +17,8 @@ #include "LogUtils.h" -#include -#include +#include "../BridgeUtils.h" +#include "../Exception/Exception.h" namespace bridgepp { diff --git a/internal/user/imap.go b/internal/user/imap.go index 6e607ddb..86a4cf5b 100644 --- a/internal/user/imap.go +++ b/internal/user/imap.go @@ -626,7 +626,7 @@ func (conn *imapConnector) createDraft(ctx context.Context, literal []byte, addr return proton.Message{}, fmt.Errorf("failed to create parser: %w", err) } - message, err := message.ParseWithParser(parser) + message, err := message.ParseWithParser(parser, true) if err != nil { return proton.Message{}, fmt.Errorf("failed to parse message: %w", err) } diff --git a/internal/user/smtp.go b/internal/user/smtp.go index 9e237474..ce42fb23 100644 --- a/internal/user/smtp.go +++ b/internal/user/smtp.go @@ -140,7 +140,7 @@ func (user *User) sendMail(authID string, from string, to []string, r io.Reader) } // Parse the message we want to send (after we have attached the public key). - message, err := message.ParseWithParser(parser) + message, err := message.ParseWithParser(parser, false) if err != nil { return fmt.Errorf("failed to parse message: %w", err) } diff --git a/pkg/message/parser.go b/pkg/message/parser.go index 91fc105c..ead63a8d 100644 --- a/pkg/message/parser.go +++ b/pkg/message/parser.go @@ -73,6 +73,15 @@ type Attachment struct { // Parse parses an RFC822 message. func Parse(r io.Reader) (m Message, err error) { + return parseIOReaderImpl(r, false) +} + +// ParseAndAllowInvalidAddressLists parses an RFC822 message and allows email address lists to be invalid. +func ParseAndAllowInvalidAddressLists(r io.Reader) (m Message, err error) { + return parseIOReaderImpl(r, true) +} + +func parseIOReaderImpl(r io.Reader, allowInvalidAddressLists bool) (m Message, err error) { defer func() { if r := recover(); r != nil { err = fmt.Errorf("panic while parsing message: %v", r) @@ -84,21 +93,21 @@ func Parse(r io.Reader) (m Message, err error) { return Message{}, errors.Wrap(err, "failed to create new parser") } - return parse(p) + return parse(p, allowInvalidAddressLists) } // ParseWithParser parses an RFC822 message using an existing parser. -func ParseWithParser(p *parser.Parser) (m Message, err error) { +func ParseWithParser(p *parser.Parser, allowInvalidAddressLists bool) (m Message, err error) { defer func() { if r := recover(); r != nil { err = fmt.Errorf("panic while parsing message: %v", r) } }() - return parse(p) + return parse(p, allowInvalidAddressLists) } -func parse(p *parser.Parser) (Message, error) { +func parse(p *parser.Parser, allowInvalidAddressLists bool) (Message, error) { if err := convertEncodedTransferEncoding(p); err != nil { return Message{}, errors.Wrap(err, "failed to convert encoded transfer encoding") } @@ -107,7 +116,7 @@ func parse(p *parser.Parser) (Message, error) { return Message{}, errors.Wrap(err, "failed to convert foreign encodings") } - m, err := parseMessageHeader(p.Root().Header) + m, err := parseMessageHeader(p.Root().Header, allowInvalidAddressLists) if err != nil { return Message{}, errors.Wrap(err, "failed to parse message header") } @@ -433,7 +442,7 @@ func getPlainBody(part *parser.Part) []byte { } } -func parseMessageHeader(h message.Header) (Message, error) { +func parseMessageHeader(h message.Header, allowInvalidAddressLists bool) (Message, error) { var m Message for fields := h.Fields(); fields.Next(); { @@ -451,7 +460,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "from": sender, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse from") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse from") + } + + logrus.WithError(err).Warn("failed to parse from") } if len(sender) > 0 { @@ -461,7 +474,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "to": toList, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse to") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse to") + } + + logrus.WithError(err).Warn("failed to parse to") } m.ToList = toList @@ -469,7 +486,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "reply-to": replyTos, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse reply-to") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse reply-to") + } + + logrus.WithError(err).Warn("failed to parse reply-to") } m.ReplyTos = replyTos @@ -477,7 +498,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "cc": ccList, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse cc") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse cc") + } + + logrus.WithError(err).Warn("failed to parse cc") } m.CCList = ccList @@ -485,7 +510,11 @@ func parseMessageHeader(h message.Header) (Message, error) { case "bcc": bccList, err := rfc5322.ParseAddressList(fields.Value()) if err != nil { - return Message{}, errors.Wrap(err, "failed to parse bcc") + if !allowInvalidAddressLists { + return Message{}, errors.Wrap(err, "failed to parse bcc") + } + + logrus.WithError(err).Warn("failed to parse bcc") } m.BCCList = bccList diff --git a/pkg/message/parser_test.go b/pkg/message/parser_test.go index 6942deee..5bbce163 100644 --- a/pkg/message/parser_test.go +++ b/pkg/message/parser_test.go @@ -23,6 +23,7 @@ import ( "io" "os" "path/filepath" + "strings" "testing" "github.com/ProtonMail/go-proton-api" @@ -444,7 +445,7 @@ func TestParseWithAttachedPublicKey(t *testing.T) { p, err := parser.New(f) require.NoError(t, err) - m, err := ParseWithParser(p) + m, err := ParseWithParser(p, false) require.NoError(t, err) p.AttachPublicKey("publickey", "publickeyname") @@ -636,6 +637,32 @@ func TestParseIcsAttachment(t *testing.T) { assert.Equal(t, string(m.Attachments[0].Data), "This is an ics calendar invite") } +func TestParseAllowInvalidAddress(t *testing.T) { + const literal = `To: foo +From: bar +BCC: fff +CC: FFF +Reply-To: AAA +Subject: Test +` + + // This will fail as the addresses are not valid. + { + _, err := Parse(strings.NewReader(literal)) + require.Error(t, err) + } + + // This will work as invalid addresses will be ignored. + m, err := ParseAndAllowInvalidAddressLists(strings.NewReader(literal)) + require.NoError(t, err) + + assert.Empty(t, m.ToList) + assert.Empty(t, m.Sender) + assert.Empty(t, m.CCList) + assert.Empty(t, m.BCCList) + assert.Empty(t, m.ReplyTos) +} + func TestParsePanic(t *testing.T) { var err error