Skip to content

Commit c848482

Browse files
A checkbox that could not say STARTTLS, and three actions with no button
External accounts. The "Use SSL/TLS" checkbox on an external POP3 download account was a boolean over a four-valued setting, and the mapping was lossy in both directions: InterfaceFetchAccount::get_UseSSL is ConnectionSecurity == CSSSL, and put_UseSSL writes CSSSL or CSNone. So an account already set to STARTTLS read back as unticked, and saving that dialog rewrote it to implicit TLS. Implicit TLS on port 110 does not connect, which made STARTTLS effectively unreachable from the GUI for exactly the providers that require it. The server has supported all four values on this path throughout: POP3ClientConnection sends CAPA, issues STLS, and on "STARTTLS required" abandons the download with a log line when the far end does not advertise it. The field is now that choice, worded with the port each option implies, because changing the security without changing the port is the commonest way an external account is configured and then never downloads anything. While doing it, combo columns in every collection editor stopped rendering their raw enum number. A grid cell reading "1" beside a dialog reading "SSL/TLS" is not a cosmetic problem: it is the difference between a list of accounts you can scan for the unencrypted row and one you cannot. Database tuning. NumberOfConnections, ConnectionAttempts and ConnectionAttemptsDelay live in [Database] and had no editor anywhere. The rest of that section decides whether the server starts at all and stays with the setup wizard; these three decide how it behaves once connected, and a server that cannot reach its database for the first half-minute after a reboot had no way to lengthen the window without hand-editing. The pool size card reads the configured database type and says outright, before you change it, that the built-in database forces the value to 1 whatever is set - which is what IniFileSettings.cpp does and what no GUI has ever admitted. That needed a section-aware INI row. IniFeatureStore writes [Settings] only, so a [Database] key edited through the existing row type would have been written into [Settings], where the server never looks - saved, correct on reload, and inert. ReadFrom/WriteTo exist to make that mistake impossible to make quietly. Three actions that existed over COM and nowhere else. MessageIndexing.Index() and .Clear() are now buttons on Performance > Indexing, and Utilities.PerformMaintenance(eUpdateIMAPFolderUID) is a button on the IMAP tab. The indexing buttons say what they actually do rather than what they look like they do. MessageIndexer::IndexNow() does not index - it sets an event that a worker thread waits on - and that worker is only started when indexing is enabled. So "Index now" with indexing switched off would signal a thread that is not running and report success having done nothing; it now checks first and says so. And the counts it reports are labelled as the state before the pass, because they are: the pass has not happened when the button returns. Reporting them as "now" would have looked exactly like a stuck indexer. The folder-UID repair is worth having a button because of what it repairs: Maintenance::RecalculateFolderUID_ raises each IMAP folder's UID counter to the highest UID the folder actually holds. A counter that has fallen behind hands the same UID out twice, and a client that caches by UID then shows one message in place of another. It never lowers a counter, so it cannot cause the fault it fixes - and the card says so, because an administrator will not run a repair they cannot reason about. Control Panel: 533 passed, 0 failed. Server unchanged.
1 parent 069fa77 commit c848482

5 files changed

Lines changed: 498 additions & 9 deletions

File tree

hmailserver/source/Tools/ControlPanel/Services/IniFeatureStore.cs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,37 @@ public void Write(string key, string value)
110110

111111
public void WriteBool(string key, bool value) => Write(key, value ? "1" : "0");
112112

113+
/// <summary>
114+
/// Reads a value from a named section rather than from [Settings].
115+
///
116+
/// Most of hMailServer.INI is [Settings], which is why this class defaults to
117+
/// it - but not all of it, and a page that edits [Database] or [Directories]
118+
/// through Read() above would write keys of the right name into a section the
119+
/// server never looks at. That is the worst shape of bug this project has:
120+
/// the value appears saved, reads back correctly on the next visit, and does
121+
/// nothing.
122+
/// </summary>
123+
public string ReadFrom(string section, string key, string defaultValue = "")
124+
{
125+
if (!IsAvailable)
126+
return defaultValue;
127+
var buffer = new StringBuilder(2048);
128+
GetPrivateProfileString(section, key, defaultValue, buffer, buffer.Capacity, IniPath);
129+
return buffer.ToString();
130+
}
131+
132+
/// <summary>Writes a value to a named section. See <see cref="ReadFrom"/>.</summary>
133+
public void WriteTo(string section, string key, string value)
134+
{
135+
if (!IsAvailable)
136+
throw new InvalidOperationException("hMailServer.INI was not found on this machine.");
137+
WritePrivateProfileString(section, key, value, IniPath);
138+
}
139+
113140
/// <summary>Reads the configured log folder from the [Directories] section.</summary>
114141
public string GetLogFolder()
115142
{
116-
if (!IsAvailable)
117-
return null;
118-
var buffer = new StringBuilder(1024);
119-
GetPrivateProfileString("Directories", "LogFolder", "", buffer, buffer.Capacity, IniPath);
120-
string folder = buffer.ToString();
143+
string folder = ReadFrom("Directories", "LogFolder", "");
121144
return string.IsNullOrWhiteSpace(folder) ? null : folder;
122145
}
123146
}

hmailserver/source/Tools/ControlPanel/Services/SettingsSearchIndex.g.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ public static partial class SettingsSearchIndex
201201
new SettingEntry("Account cache TTL (seconds)", "Cache.AccountCacheTTL", "performance"),
202202
new SettingEntry("Alias cache max size (KB, resets at service restart)", "Cache.AliasCacheMaxSizeKb", "performance"),
203203
new SettingEntry("Alias cache TTL (seconds)", "Cache.AliasCacheTTL", "performance"),
204+
new SettingEntry("Attempts to reach the database at start-up", "ConnectionAttempts", "performance"),
205+
new SettingEntry("Connections in the pool", "NumberOfConnections", "performance"),
204206
new SettingEntry("Distribution-list cache max size (KB, resets at service restart)", "Cache.DistributionListCacheMaxSizeKb", "performance"),
205207
new SettingEntry("Distribution-list cache TTL (seconds)", "Cache.DistributionListCacheTTL", "performance"),
206208
new SettingEntry("Domain cache max size (KB, resets at service restart)", "Cache.DomainCacheMaxSizeKb", "performance"),
@@ -213,6 +215,7 @@ public static partial class SettingsSearchIndex
213215
new SettingEntry("Max parallel external POP3 fetch threads (restart required)", "MaxNumberOfExternalFetchThreads", "performance"),
214216
new SettingEntry("Messages per full-index pass", "IndexerFullLimit", "performance"),
215217
new SettingEntry("Messages per quick-index pass", "IndexerQuickLimit", "performance"),
218+
new SettingEntry("Seconds between those attempts", "ConnectionAttemptsDelay", "performance"),
216219
new SettingEntry("TCP/IP threads", "TCPIPThreads", "performance"),
217220
new SettingEntry("Worker thread priority (stored, but the server does not use it)", "WorkerThreadPriority", "performance"),
218221
new SettingEntry("ACL (shared folder permissions)", "IMAPACLEnabled", "protocols"),

hmailserver/source/Tools/ControlPanel/Views/CollectionEditorView.cs

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,16 @@ private void Build()
183183
var col = new DataGridTextColumn
184184
{
185185
Header = f.Label,
186-
Binding = new System.Windows.Data.Binding($"Values[{prop}]") { Converter = CellConverter.Instance },
186+
// A Combo column stores the enum's number, and a grid cell reading
187+
// "1" where the dialog above it reads "SSL/TLS" is not a display
188+
// detail - it is the difference between a list an administrator can
189+
// scan for the unencrypted row and one they cannot.
190+
Binding = new System.Windows.Data.Binding($"Values[{prop}]")
191+
{
192+
Converter = f.Kind == FieldKind.Combo && f.Options != null
193+
? new CellConverter(f.Options)
194+
: CellConverter.Instance
195+
},
187196
Width = double.IsNaN(f.GridWidth)
188197
? new DataGridLength(1, DataGridLengthUnitType.Star)
189198
: new DataGridLength(f.GridWidth)
@@ -383,8 +392,48 @@ internal static string FormatCell(object v)
383392

384393
private sealed class CellConverter : System.Windows.Data.IValueConverter
385394
{
395+
/// <summary>Option labels for a Combo column, or null for every other kind.</summary>
396+
private readonly (int Value, string Label)[] options_;
397+
398+
private CellConverter()
399+
{
400+
}
401+
402+
public CellConverter((int Value, string Label)[] options) => options_ = options;
403+
386404
public static readonly CellConverter Instance = new();
387-
public object Convert(object value, Type t, object p, CultureInfo c) => FormatCell(value);
405+
406+
public object Convert(object value, Type t, object p, CultureInfo c)
407+
{
408+
if (options_ != null && value != null)
409+
{
410+
// A value with no matching option falls through to the number
411+
// rather than being shown as blank or as the first option: an
412+
// enum the GUI does not know about is a real thing to notice,
413+
// and blanking it would hide it.
414+
try
415+
{
416+
int number = System.Convert.ToInt32(value, CultureInfo.InvariantCulture);
417+
foreach ((int Value, string Label) option in options_)
418+
{
419+
if (option.Value == number)
420+
return option.Label;
421+
}
422+
}
423+
catch (FormatException)
424+
{
425+
}
426+
catch (InvalidCastException)
427+
{
428+
}
429+
catch (OverflowException)
430+
{
431+
}
432+
}
433+
434+
return FormatCell(value);
435+
}
436+
388437
public object ConvertBack(object value, Type t, object p, CultureInfo c) => value;
389438
}
390439
}

hmailserver/source/Tools/ControlPanel/Views/CollectionSpecs.cs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,22 @@ public static class CollectionSpecs
1616
private static dynamic AntiSpam => ServerSession.Current.Application.Settings.AntiSpam;
1717
private static dynamic Settings => ServerSession.Current.Application.Settings;
1818

19+
/// <summary>
20+
/// hMailServer's eConnectionSecurity, in the order the enum declares it, and
21+
/// worded the same way the SSL/TLS and Delivery pages word it so the same
22+
/// choice does not read as two different things in two places. The wording
23+
/// carries the port each one implies, because choosing the security without
24+
/// changing the port is the single commonest way an external account is
25+
/// configured and then never downloads anything.
26+
/// </summary>
27+
private static readonly (int Value, string Label)[] ConnectionSecurityOptions =
28+
{
29+
(0, "None - no encryption (port 110)"),
30+
(1, "SSL/TLS - encrypted from the first byte (port 995)"),
31+
(2, "STARTTLS, optional - upgrade if offered, continue in the clear if not (port 110)"),
32+
(3, "STARTTLS, required - refuse to download unless the upgrade succeeds (port 110)")
33+
};
34+
1935
public static CollectionEditorView SurblServers() => new(new CollectionSpec
2036
{
2137
Title = "SURBL servers",
@@ -167,7 +183,8 @@ private static dynamic OpenAccount(string domainName, string address)
167183
public static CollectionEditorView FetchAccounts(string domainName, string address) => new(new CollectionSpec
168184
{
169185
Title = "External accounts",
170-
Subtitle = "POP3 mailboxes hMailServer downloads mail from on behalf of this account.",
186+
Subtitle = "POP3 mailboxes hMailServer downloads mail from on behalf of this account. "
187+
+ "Connection security and port go together: SSL/TLS on 995, STARTTLS on 110.",
171188
ItemNoun = "external account",
172189
GetCollection = () =>
173190
{
@@ -186,7 +203,28 @@ private static dynamic OpenAccount(string domainName, string address)
186203
new FieldSpec { Prop = "Password", Label = "Password", Kind = FieldKind.Password, ShowInGrid = false, Default = "" },
187204
new FieldSpec { Prop = "MinutesBetweenFetch", Label = "Minutes between downloads", Kind = FieldKind.Number, ShowInGrid = false, Default = 15 },
188205
new FieldSpec { Prop = "DaysToKeepMessages", Label = "Days to keep on server (0 = delete after download)", Kind = FieldKind.Number, ShowInGrid = false, Default = 0 },
189-
new FieldSpec { Prop = "UseSSL", Label = "Use SSL/TLS", Kind = FieldKind.Bool, ShowInGrid = false, Default = false },
206+
// Was a "Use SSL/TLS" checkbox, which could not express STARTTLS at
207+
// all. The COM property behind that checkbox is UseSSL, and its
208+
// getter is ConnectionSecurity == CSSSL while its setter writes
209+
// CSSSL or CSNone - so an account already set to STARTTLS read back
210+
// as unticked, and ticking anything on that dialog rewrote it to
211+
// implicit TLS on the next save. Implicit TLS on port 110 does not
212+
// connect, which made STARTTLS effectively unreachable from the GUI
213+
// for the very providers that require it.
214+
//
215+
// The server has supported all four values on the fetch path
216+
// throughout: POP3ClientConnection sends CAPA, issues STLS, and on
217+
// CSSTARTTLSRequired abandons the download with a log line when the
218+
// far end does not advertise STLS.
219+
new FieldSpec
220+
{
221+
Prop = "ConnectionSecurity",
222+
Label = "Connection security",
223+
Kind = FieldKind.Combo,
224+
Options = ConnectionSecurityOptions,
225+
GridWidth = 150,
226+
Default = 0
227+
},
190228
new FieldSpec { Prop = "UseAntiSpam", Label = "Run anti-spam on downloaded mail", Kind = FieldKind.Bool, ShowInGrid = false, Default = true },
191229
new FieldSpec { Prop = "UseAntiVirus", Label = "Run anti-virus on downloaded mail", Kind = FieldKind.Bool, ShowInGrid = false, Default = true }
192230
}

0 commit comments

Comments
 (0)