Skip to content

Commit 3d2e265

Browse files
committed
smtp dot-stuffed + null variations
1 parent f28680d commit 3d2e265

File tree

7 files changed

+102
-10
lines changed

7 files changed

+102
-10
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
All notable changes to this project will be documented in this file.
44

5+
## [26.11] - 2026-01-24
6+
7+
- **Broadcasts**: Fixed crash on Broadcasts page when `variations` is null instead of empty array (#233)
8+
- **SMTP Email**: Fixed URL corruption in emails where dots were stripped after quoted-printable line breaks by adding proper SMTP dot-stuffing
9+
510
## [26.10] - 2026-01-23
611

712
- **Automations**: Email node template selector now shows all template categories instead of only marketing templates

config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"github.com/spf13/viper"
1515
)
1616

17-
const VERSION = "26.10"
17+
const VERSION = "26.11"
1818

1919
type Config struct {
2020
Server ServerConfig

console/src/pages/BroadcastsPage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ const BroadcastCard: React.FC<BroadcastCardProps> = ({
649649
<div className="mb-6">
650650
<Table
651651
showHeader={false}
652-
dataSource={broadcast.test_settings.variations.map((variation, index) => {
652+
dataSource={(broadcast.test_settings.variations || []).map((variation, index) => {
653653
const emailProvider = currentWorkspace?.integrations?.find(
654654
(i: Integration) =>
655655
i.id ===

internal/domain/broadcast.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ func (b BroadcastTestSettings) Value() (driver.Value, error) {
5454
return json.Marshal(b)
5555
}
5656

57+
// MarshalJSON implements custom JSON marshaling to ensure Variations is never null
58+
func (b BroadcastTestSettings) MarshalJSON() ([]byte, error) {
59+
type Alias BroadcastTestSettings
60+
// Ensure Variations is an empty array, not null
61+
if b.Variations == nil {
62+
b.Variations = []BroadcastVariation{}
63+
}
64+
return json.Marshal((*Alias)(&b))
65+
}
66+
5767
// Scan implements the sql.Scanner interface for database deserialization
5868
func (b *BroadcastTestSettings) Scan(value interface{}) error {
5969
if value == nil {
@@ -66,7 +76,16 @@ func (b *BroadcastTestSettings) Scan(value interface{}) error {
6676
}
6777

6878
cloned := bytes.Clone(v)
69-
return json.Unmarshal(cloned, b)
79+
if err := json.Unmarshal(cloned, b); err != nil {
80+
return err
81+
}
82+
83+
// Ensure Variations is never nil to prevent frontend crashes
84+
if b.Variations == nil {
85+
b.Variations = []BroadcastVariation{}
86+
}
87+
88+
return nil
7089
}
7190

7291
// BroadcastVariation represents a single variation in an A/B test

internal/domain/broadcast_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,24 @@ func TestBroadcastTestSettings_ValueScan(t *testing.T) {
11071107
err = invalidTarget.Scan("not-a-byte-array")
11081108
require.Error(t, err)
11091109
assert.Contains(t, err.Error(), "type assertion to []byte failed")
1110+
1111+
// Test that null variations becomes empty array after scan
1112+
nullVariationsJSON := []byte(`{"enabled":true,"sample_percentage":10,"auto_send_winner":false,"variations":null}`)
1113+
var nullVariationsTarget domain.BroadcastTestSettings
1114+
err = nullVariationsTarget.Scan(nullVariationsJSON)
1115+
require.NoError(t, err)
1116+
assert.NotNil(t, nullVariationsTarget.Variations, "Variations should not be nil after scanning null")
1117+
assert.Equal(t, 0, len(nullVariationsTarget.Variations), "Variations should be empty array")
1118+
1119+
// Test that MarshalJSON produces empty array instead of null
1120+
emptySettings := domain.BroadcastTestSettings{
1121+
Enabled: true,
1122+
SamplePercentage: 10,
1123+
Variations: nil, // nil variations
1124+
}
1125+
marshaled, err := emptySettings.MarshalJSON()
1126+
require.NoError(t, err)
1127+
assert.Contains(t, string(marshaled), `"variations":[]`, "Variations should be serialized as empty array, not null")
11101128
}
11111129

11121130
// TestBroadcastVariation_ValueScan tests the Value and Scan methods for BroadcastVariation

internal/service/smtp_service.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"encoding/base64"
99
"fmt"
1010
"net"
11+
"net/textproto"
1112
"os"
1213
"strings"
1314
"time"
@@ -297,15 +298,21 @@ func sendRawEmailWithSettings(settings *domain.SMTPSettings, from string, to []s
297298
return fmt.Errorf("DATA rejected with code: %d", code)
298299
}
299300

300-
// Send message body
301-
// Ensure proper line endings and dot-stuffing
302-
if _, err := smtpConn.conn.Write(msg); err != nil {
301+
// Send message body using textproto.DotWriter for automatic dot-stuffing
302+
// Per RFC 5321 Section 4.5.2, lines starting with a period must be escaped
303+
// by doubling the period. This prevents the SMTP server from stripping
304+
// leading periods or misinterpreting them as end-of-message markers.
305+
// The DotWriter handles this automatically and also sends the terminating
306+
// CRLF.CRLF sequence when Close() is called.
307+
// See: https://datatracker.ietf.org/doc/html/rfc5321#section-4.5.2
308+
tw := textproto.NewWriter(bufio.NewWriter(smtpConn.conn))
309+
dw := tw.DotWriter()
310+
if _, err := dw.Write(msg); err != nil {
311+
dw.Close()
303312
return fmt.Errorf("failed to write message: %w", err)
304313
}
305-
306-
// End message with CRLF.CRLF
307-
if _, err := fmt.Fprintf(smtpConn.conn, "\r\n.\r\n"); err != nil {
308-
return fmt.Errorf("failed to write message terminator: %w", err)
314+
if err := dw.Close(); err != nil {
315+
return fmt.Errorf("failed to close message: %w", err)
309316
}
310317

311318
// Read response after DATA

internal/service/smtp_service_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,3 +1662,46 @@ func TestNewSMTPServiceWithOAuth2(t *testing.T) {
16621662
assert.Equal(t, log, service.logger)
16631663
assert.Equal(t, mockProvider, service.oauth2Provider)
16641664
}
1665+
1666+
// TestDotStuffingIntegration tests that emails with URLs crossing line boundaries
1667+
// are properly handled by the SMTP sending logic using textproto.DotWriter.
1668+
// This verifies the fix for the URL corruption bug where dots after soft line
1669+
// breaks in quoted-printable encoded content were being stripped by SMTP servers.
1670+
func TestDotStuffingIntegration(t *testing.T) {
1671+
// Create a mock server that captures the raw message data
1672+
// The server starts serving automatically in newMockSMTPServer
1673+
server := newMockSMTPServer(t, true)
1674+
defer server.Close()
1675+
1676+
// Wait for server to start
1677+
time.Sleep(50 * time.Millisecond)
1678+
1679+
host, portStr, _ := net.SplitHostPort(server.Addr())
1680+
port := 0
1681+
fmt.Sscanf(portStr, "%d", &port)
1682+
1683+
// Create a message with content that would trigger the bug:
1684+
// After quoted-printable encoding, a soft line break before ".com" results in
1685+
// a line starting with ".com". Without proper dot-stuffing, SMTP servers
1686+
// strip the leading dot, corrupting URLs.
1687+
//
1688+
// The test message simulates this by having a line that starts with a period.
1689+
testMessage := []byte("Content-Type: text/html\r\n\r\n" +
1690+
"Line one\r\n" +
1691+
".com/path/to/image.png\r\n" + // This line starts with a dot
1692+
"Line three\r\n")
1693+
1694+
err := sendRawEmail(host, port, "user", "pass", false, "[email protected]", []string{"[email protected]"}, testMessage)
1695+
require.NoError(t, err)
1696+
1697+
// Verify the message was received
1698+
require.Len(t, server.messages, 1)
1699+
receivedData := string(server.messages[0].data)
1700+
1701+
// The SMTP server should have received the message with the dot doubled (dot-stuffed).
1702+
// textproto.DotWriter automatically handles this per RFC 5321.
1703+
// When the server receives "..com", it strips one dot to get ".com" back.
1704+
// Our mock server stores the raw data as received (with dot-stuffing applied).
1705+
assert.Contains(t, receivedData, "..com/path/to/image.png",
1706+
"Expected dot-stuffed content (double dot) but got: %s", receivedData)
1707+
}

0 commit comments

Comments
 (0)