Skip to content

Add SMTP authentication support to Forward dialog and Name property to rules #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jaben wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Add SMTP authentication support to Forward dialog and Name property to rules #364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ac37b2
Add SMTP authentication support to Forward dialog and Name property t…
claude Mar 31, 2026
d78f95d
Address CodeRabbit review feedback
claude Mar 31, 2026
07bb320
Improving the Dialog usability
Jaben Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Papercut.Core/Domain/Rules/IRule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public interface IRule : INotifyPropertyChanged
{
Guid Id { get; }

string Name { get; set; }

bool IsEnabled { get; set; }

string Type { get; }
Expand Down
6 changes: 6 additions & 0 deletions src/Papercut.Rules/App/RulesRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public async Task RunNewMessageRules(INewMessageRule[] rules, MessageEntry messa
if (rules == null) throw new ArgumentNullException(nameof(rules));
if (messageEntry == null) throw new ArgumentNullException(nameof(messageEntry));

token.ThrowIfCancellationRequested();

var ruleTasks = new List<Task>();

foreach (var rule in rules.Where(r => r.IsEnabled))
Expand Down Expand Up @@ -125,6 +127,10 @@ async Task DispatchRuleAsync<TRule>(TRule rule, MessageEntry? messageEntry, Canc
var ruleDispatcher = _lifetimeScope.Resolve<IRuleDispatcher<TRule>>();
await ruleDispatcher.DispatchAsync(rule, messageEntry, token);
}
catch (OperationCanceledException)
{
throw;
}
Comment on lines +130 to +133

@coderabbitai coderabbitai Bot Apr 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Rethrow cancellation only when this runner token is canceled.

At Lines 130-133, rethrowing every OperationCanceledException can propagate unrelated dispatcher-internal cancellations (e.g., timeout-driven TaskCanceledException) and abort the whole run unexpectedly.

Proposed fix 
-        catch (OperationCanceledException)
+        catch (OperationCanceledException) when (token.IsCancellationRequested)
         {
             throw;
         }
+        catch (OperationCanceledException ex)
+        {
+            _logger.Warning(
+                ex,
+                "Rule dispatch canceled without runner cancellation for Rule {Rule} on Message {`@MessageEntry`}",
+                rule,
+                messageEntry);
+        }
         catch (Exception ex)
         {
             _logger.Warning(
                 ex,
                 "Failure Dispatching Rule {Rule} on Message {`@MessageEntry`}",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
catch (OperationCanceledException)
{
throw;
}
catch (OperationCanceledException) when (token.IsCancellationRequested)
{
throw;
}
catch (OperationCanceledException ex)
{
_logger.Warning(
ex,
"Rule dispatch canceled without runner cancellation for Rule {Rule} on Message {`@MessageEntry`}",
rule,
messageEntry);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/Papercut.Rules/App/RulesRunner.cs` around lines 130 - 133, The current
catch block in RulesRunner (in class RulesRunner) unconditionally rethrows
OperationCanceledException; change it to catch (OperationCanceledException ex)
and only rethrow when ex.CancellationToken equals this runner's cancellation
token (the field used by the runner, e.g. _cancellationToken or runnerToken); if
the exception was canceled by a different token, treat it as handled (do not
rethrow) so dispatcher/internal timeouts don't abort this run. Ensure you
reference the runner's cancellation token field when comparing before
rethrowing.

catch (Exception ex)
{
_logger.Warning(
Expand Down
18 changes: 17 additions & 1 deletion src/Papercut.Rules/Domain/Rules/NewMessageRuleBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,25 @@ public abstract class NewMessageRuleBase : INewMessageRule
{
bool _isEnabled;

string _name;

[Category("Information")]
public Guid Id { get; protected set; } = Guid.NewGuid();

[Category("Information")]
[DisplayName("Name")]
[Description("A friendly name for this rule")]
public string Name
{
get => this._name;
set
{
if (value == this._name) return;
this._name = value;
this.OnPropertyChanged(nameof(this.Name));
}
}

[Category("State")]
[Browsable(true)]
[DisplayName("Is Enabled")]
Expand All @@ -59,7 +75,7 @@ public virtual bool IsEnabled
public virtual string Description
=>
this.GetPropertiesForDescription()
.Where(s => !s.Key.IsAny("Id", "Type", "Description"))
.Where(s => !s.Key.IsAny("Id", "Name", "Type", "Description"))
.OrderBy(s => s.Key)
.ToFormattedPairs()
.Join("\r\n");
Expand Down
18 changes: 17 additions & 1 deletion src/Papercut.Rules/Domain/Rules/PeriodicBackgroundRuleBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,25 @@ public abstract class PeriodicBackgroundRuleBase : IPeriodicBackgroundRule
{
bool _isEnabled;

string _name;

[Category("Information")]
public Guid Id { get; protected set; } = Guid.NewGuid();

[Category("Information")]
[DisplayName("Name")]
[Description("A friendly name for this rule")]
public string Name
{
get => this._name;
set
{
if (value == this._name) return;
this._name = value;
this.OnPropertyChanged(nameof(this.Name));
}
}

[Category("State")]
[Browsable(true)]
[DisplayName("Is Enabled")]
Expand All @@ -59,7 +75,7 @@ public virtual bool IsEnabled
public virtual string Description
=>
this.GetPropertiesForDescription()
.Where(s => !s.Key.IsAny("Id", "Type", "Description"))
.Where(s => !s.Key.IsAny("Id", "Name", "Type", "Description"))
.OrderBy(s => s.Key)
.ToFormattedPairs()
.Join("\r\n");
Expand Down
48 changes: 48 additions & 0 deletions src/Papercut.UI/Properties/Settings.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions src/Papercut.UI/Properties/Settings.settings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@
<Setting Name="ForwardTo" Type="System.String" Scope="User">
<Value Profile="(Default)" />
</Setting>
<Setting Name="ForwardSmtpUsername" Type="System.String" Scope="User">
<Value Profile="(Default)" />
</Setting>
<Setting Name="ForwardSmtpPassword" Type="System.String" Scope="User">
<Value Profile="(Default)" />
</Setting>
<Setting Name="ForwardSmtpPort" Type="System.Int32" Scope="User">
<Value Profile="(Default)">25</Value>
</Setting>
<Setting Name="ForwardSmtpUseSsl" Type="System.Boolean" Scope="User">
<Value Profile="(Default)">False</Value>
</Setting>
<Setting Name="MinimizeOnClose" Type="System.Boolean" Scope="User">
<Value Profile="(Default)">True</Value>
</Setting>
Expand Down
Loading
Loading