The default paths for the web interfaces, such as /admin/, /account/, /webmail/
and /webapi/ end with a slash. They should end with a slash because we use the
path when restricting cookies to just that web interface. You could configure
paths not ending with a slash, but due to using http.StripPrefix, and our
handler, some of those requests may not work properly.
We now warn if configured paths don't end with a trailing slash when parsing
the config file. We normally error out when such things happen, but users
probably have paths without trailing slashes configured, and we don't want to
break them on a future upgrade. We now use an effective path that includes the
trailing slash.
We would always redirect requests to the configured paths but without trailing
slash to the path with trailing slash, and that stays.
For issue #325 by odama626.
Download as eml is useful with firefox, because opening the raw message in a
new tab, and then downloading it, causes firefox to request the url without
cookies, causing it to save a "403 - forbidden" response.
Exporting a selection is useful during all kinds of testing. Makes it easy to
an entire thread, or just some messages.
The export popover now has buttons for each combination of mbox/maildir vs
zip/tgz/tar. Before you may have had to select the email format and archive
format first, followed by a click. Now it's just a click.
Attackers scanning the internet can use it to easily create a database of
hosts, software and versions. Let's not make it too easy to find old versions
that may be vulnerable to potential bugs found in the future. We could try
hiding the name "mox" as well, but the banner will still be identifyable, so
there isn't much point, and the public knowing approximately which software is
running can be useful for debugging.
The ID command in IMAP is used by clients to announce their software and
version. We only respond with our version when the user is authenticated.
There are still ways to discover the version number. But they don't involve
standard banner scanning, so someone would have to specifically target mox. We
could tighten that in the future.
For issue #322, based on email. Thanks everyone for discussing.
We were already generating previews of plain text parts for the webmail
interface, but we didn't store them, so were generating the previews each time
messages were listed.
Now we store previews in the database for faster handling. And we also generate
previews for html parts if needed. We use the first part that has textual
content.
For IMAP, the previews can be requested by an IMAP client. When we get the
"LAZY" variant, which doesn't require us to generate a preview, we generate it
anyway, because it should be fast enough. So don't make clients first ask for
"PREVIEW (LAZY)" and then again a request for "PREVIEW".
We now also generate a preview when a message is added to the account. Except
for imports. It would slow us down, the previews aren't urgent, and they will
be generated on-demand at first-request.
We normally check errors for all operations. But for some cleanup calls, eg
"defer file.Close()", we didn't. Now we also check and log most of those.
Partially because those errors can point to some mishandling or unexpected code
paths (eg file unexpected already closed). And in part to make it easier to use
"errcheck" to find the real missing error checks, there is too much noise now.
The log.Check function can now be used unconditionally for checking and logging
about errors. It adjusts the log level if the error is caused by a network
connection being closed, or a context is canceled or its deadline reached, or a
socket deadline is reached.
In store/search.go, we would make a copy of a byte array, but then still use
the original instead of the copy. Could result in search operations not finding
messages that do have the content, but under very unlikely conditions only.
We'll keep running ineffassign with "make check", useful enough.
Adding to the queue is done in a transaction, the queue db file is mox-global.
Appending the message to the Sent folder, removing it from Drafts, marking the
original message as answered/forwarded, is done in a separate database
transaction that gets the ctx passed in. If the ctx was canceled in between,
the queueing was finished, but the rest wasn't completed.
Reported by mteege, thanks!
On reply, with too many Cc/Bcc, I usually hit ctrl+backspace a few time. I just
want to clear the addresses, but I practically always still want a To address.
When removing an address, we want to make sure any queued messages for the
account still still have their address associated with the account. E.g.
through a catchall address.
Before removing an account, we fail deliveries still in the queue for the
account. We remove any addresses on the suppression list (which are stored in
the queue database, not the account database file that is removed completely).
We also clear all sessions for the webmail/webaccount interfaces. For the
webmail, further operations will fail, and the reconnection attempt will cause
the login popup with a message about an unknown session token.
Example symptom, when deleting a message in the webmail (which moves to Trash):
l=error m="duplicating message in old mailbox for current sessions" err="link data/accounts/mjl/msg/I/368638 data/accounts/mjl/msg/J/368640: no such file or directory" pkg=webmail
Problem introduced a few weeks ago, where moving messages starting duplicating
the message first, and the copy is erased once all references (in IMAP
sessions) to the old mailbox have been removed.
The simplistic logging approach we've followed so far is to not log struct
fields that are themselves structs, which time.Time is. So we skipped, but do
log it now.
The intent to remove the account is stored in the database. At startup, if
there are any such referenes, they are applied by removing the account
directories and the entry in the database. This ensures the account directory
is properly removed, even on incomplete shutdown.
Don't add an account when its directory already exits.
Initialize store and switchboard first, then open account, and close in reverse
order.
Replace all "CheckClosed" calls with "WaitClosed", future changings will keep
an account reference open for a bit after the last regular close, so we can't
know that an account should be closed during tests.
Remove one parameter from the (still too long) "start test server" function in
imapserver testing code.
We weren't appending the individual changes to the slice, but the entire slice.
Since "Change" is an "any", this isn't a type error. So make a Change a
non-empty interface (I had seen an issue like this coming, should have made it
an interface then, at least now we have a reasonable method, to get the modseq
of a change).
Found while working on an imap webpush prototype.
Doesn't seem like it's a common thing to do. And it's just a bit risky, it's
too easy to forget to clear some part of the authentication state on a
connection (especially future changes that forget to update clear a new field
during unauthenticate). If a strong use case ever pops up, we can reconsider.
Also update the roadmap a bit.
And merge the duplicate list of capabilities. We had each on a line for
cross-referencing with the RFC, and all capabilities again but on a single line
to use in the server greeting. Now it's just one list.
Since a recent change (likely since implementing MULTIAPPEND), the temporary
files weren't removed any more. When changing it, I must have had the wrong
mental model about the MessageAdd method, assuming it would remove the temp
file.
Noticed during tests.
Erasing a message (removing the message file from the file system) was made a
separate step a few days ago. The verifydata command checks for consistency of
the data, but didn't correctly skip checking expunged-but-not-yet-erased
messages, leading to the warning.
A similar consistency check in store/account.go does check for that.
I was warned by my nightly backup+veridata script.
In case the precis check failed, our return of a nil account cleared acc, and
we were then trying to close it, returning in a nil pointer dereference.
Rewrite the return statements so we don't overwrite the named return variables.
Especially relevant when the name contains a comma, e.g. "lastname, firstname".
Or when it contains parentheses, e.g. "(organization)".
When sending to an address with a comma that isn't quoted, we would actually
interpret it as two addresses: One without an "@" before the comma, and the
second part after the comma with half of the name and the email addrss. This
resulted in an error message.
When sending to a recipient with unquoted parentheses in the name, those
parentheses would be interpreted as an generic email header comment, and left
out.
For issue #305 by mattfbacon.
The original config option stays, and we still use it for the common case where
we have a single separator. The "+" is configured by default. It is optional,
just like the new option "LocalpartCatchallSeparators" (plural).
When parsing the config file, we combine LocalpartCatchallSeparator and
LocalpartCatchallSeparators into a single list
LocalpartCatchallSeparatorsEffective, which we use throughout the code.
For issue #301 by janc13
Mox does not look up names from the /etc/hosts file, only through DNS. But
"localhost" may not resolve through DNS, or when offline a DNS server may not
even be available. We will want deliveries to work in "mox localserve" mode.
Found by dstotijn.
We'll need RSA DKIM keys for a long time to come because many systems don't
support ed25519 DKIM signatures. We've been adding both types of keys when
adding a new domain, and adding both two DKIM signatures to outgoing messages.
This works fine in practice, other mail servers are correctly ignoring the
ed25519 signature if they don't understand it. Unfortunately, it causess noise
in DMARC reports: Systems will warn that a DKIM check failed. Sometimes with a
vague message about a missing key, or a 0-bit key. Sometimes they leave the
selector out of the report, making it hard to understand what's going on. This
causes postmasters to investigate because they think something is wrong, only
to eventually find out it's all fine. So we're causing needless chores for
postmasters. By having only an RSA DKIM signature, we skip that noise. This
also reduces the number of DNS records postmasters have to add for a domain.
The small ed25519 DKIM DNS TXT records would make them preferrable over the
long multi-string RSA DKIM DNS TXT records (which are often hard to add
correctly through DNS operator web interfaces), but as mentioned, we'll have to
add the RSA DKIM keys anyway.
Another reason why RSA keys _may_ be preferrable over ed25519 keys is that with
RSA, signing is more computationally expensive than verifying, while it's the
other way around for ed25519 keys.
Admins can always add an ed25519 DKIM key to their domain. And we can always
switch back to adding them to new domains by default in the future.
For issue #299.
Just to be cautious. This hasn't happened yet in practice that I'm aware of.
But in theory, mox could crash after it has written the message file during
delivery, but before the database transaction was committed. In that case, a
message file for the "next message id" is already present. Any future delivery
attempts will get assigned the same message id by the database, but writing the
file will fail because there already is one, causing delivery to fail (until
the file is moved away).
When opening an account, we now check in the file system if newer files exist
than we expect based on the last existing message in the database. If so, we
adjust the message ID the database will assign next.
It still blocks on reading partial flushes from clients, preventing progress
and eventually timing out. The flate library needs more changes to make this
work.
Connections from iOS mail sometimes timed out, not always.
The extension is simply not announced, code is still present.
