FIX: Allow mail from the null sender

FIX: Allow mail from the null sender

Fixes a regression introduced in commit 589222a.

From fcd28fb95f581ca227b60661491cca7187218246 Mon Sep 17 00:00:00 2001
From: Saj Goonatilleke <saj@discourse.org>
Date: Thu, 29 Nov 2018 18:54:36 +1100
Subject: [PATCH] FIX: Allow mail from the null sender

Fixes a regression introduced in commit 589222a.

diff --git a/lib/mail_receiver/fast_rejection.rb b/lib/mail_receiver/fast_rejection.rb
index bd482f0..34e7210 100644
--- a/lib/mail_receiver/fast_rejection.rb
+++ b/lib/mail_receiver/fast_rejection.rb
@@ -49,33 +49,51 @@ class FastRejection < MailReceiverBase
 		elsif args['protocol_state'] != 'RCPT'
 			return 'dunno'
 		elsif args['sender'].nil?
+			# Note that while this key should always exist, its value may be the empty
+			# string.  Postfix will convert the "<>" null sender to "".
 			return 'defer_if_permit No sender specified'
 		elsif args['recipient'].nil?
 			return 'defer_if_permit No recipient specified'
 		end
 
-		domain = domain_from_addrspec(args['sender'])
+		run_filters(args)
+	end
+
+	def run_filters(args)
+		filters = [
+			:maybe_reject_by_sender_domain,
+			:maybe_reject_by_api,
+		]
+
+		for f in filters do
+			action = send(f, args)
+			break unless action == "dunno"
+		end
+		action
+	end
+
+	def maybe_reject_by_sender_domain(args)
+		sender = args['sender']
+
+		return "dunno" if sender.empty?
+
+		domain = domain_from_addrspec(sender)
 		if domain.empty?
-			logger.info("deferred mail with domainless sender #{args['sender']}")
+			logger.info("deferred mail with domainless sender #{sender}")
 			return 'defer_if_permit Invalid sender'
 		end
 		if @blacklisted_sender_domains.include? domain
-			logger.info("rejected mail from blacklisted sender domain #{domain} (from #{args['sender']})")
+			logger.info("rejected mail from blacklisted sender domain #{domain} (from #{sender})")
 			return 'reject Invalid sender'
 		end
 
-		maybe_reject_email(args['sender'], args['recipient'])
+		return "dunno"
 	end
 
-	def endpoint
-		"#{@env['DISCOURSE_BASE_URL']}/admin/email/smtp_should_reject.json"
-	end
-
-	def maybe_reject_email(from, to)
-
+	def maybe_reject_by_api(args)
 		uri = URI.parse(endpoint)
-		fromarg = CGI::escape(from)
-		toarg = CGI::escape(to)
+		fromarg = CGI::escape(args['sender'])
+		toarg = CGI::escape(args['recipient'])
 
 		api_qs = "api_key=#{key}&api_username=#{username}&from=#{fromarg}&to=#{toarg}"
 		if uri.query and !uri.query.empty?
@@ -110,4 +128,8 @@ class FastRejection < MailReceiverBase
 		return "dunno"  # let future tests also be allowed to reject this one.
 	end
 
+	def endpoint
+		"#{@env['DISCOURSE_BASE_URL']}/admin/email/smtp_should_reject.json"
+	end
+
 end
diff --git a/spec/lib/fast_reject_spec.rb b/spec/lib/fast_reject_spec.rb
index 2846d8b..beb6c65 100644
--- a/spec/lib/fast_reject_spec.rb
+++ b/spec/lib/fast_reject_spec.rb
@@ -41,11 +41,27 @@ describe FastRejection do
 		it "returns defer_if_permit if no sender" do
 			response = receiver.process_single_request(
 				'request' => 'smtpd_access_policy',
-				'protocol_state' => 'RCPT'
+				'protocol_state' => 'RCPT',
+				'recipient' => 'discourse@example.com'
 			)
 			expect(response).to start_with('defer_if_permit')
 		end
 
+		it "returns dunno if from the null sender" do
+			expect_any_instance_of(Net::HTTP).to receive(:request) do |http|
+				response = Net::HTTPSuccess.new(http, 200, "OK")
+				expect(response).to receive(:body) { "{}" }
+				response
+			end
+			response = receiver.process_single_request(
+				'request' => 'smtpd_access_policy',
+				'protocol_state' => 'RCPT',
+				'sender' => '',
+				'recipient' => 'discourse@example.com'
+			)
+			expect(response).to eq('dunno')
+		end
+
 		it "returns defer_if_permit if no recipient" do
 			response = receiver.process_single_request(
 				'request' => 'smtpd_access_policy',
diff --git a/spec/lib/mail_spec.rb b/spec/lib/mail_spec.rb
index 92cdd58..0857aa4 100644
--- a/spec/lib/mail_spec.rb
+++ b/spec/lib/mail_spec.rb
@@ -6,6 +6,10 @@ RSpec.describe 'domain_from_addrspec' do
 		expect(domain_from_addrspec("local-part@DOMAIN.NET")).to eq "domain.net"
 	end
 
+	it "returns an empty string when given an empty string" do
+		expect(domain_from_addrspec("")).to be_empty
+	end
+
 	it "returns an empty string if a domain was not found" do
 		expect(domain_from_addrspec("local-part")).to be_empty
 	end

GitHub

1 Like