infrastructure/mox
1
0
Fork 0
mirror of https://github.com/mjl-/mox.git synced 2025-07-12 17:44:35 +03:00
Code Issues Packages Projects Releases Wiki Activity

make mox compile on windows, without "mox serve" but with working "mox localserve"

Browse Source 
getting mox to compile required changing code in only a few places where
package "syscall" was used: for accessing file access times and for umask
handling. an open problem is how to start a process as an unprivileged user on
windows.  that's why "mox serve" isn't implemented yet. and just finding a way
to implement it now may not be good enough in the near future: we may want to
starting using a more complete privilege separation approach, with a process
handling sensitive tasks (handling private keys, authentication), where we may
want to pass file descriptors between processes. how would that work on
windows?

anyway, getting mox to compile for windows doesn't mean it works properly on
windows. the largest issue: mox would normally open a file, rename or remove
it, and finally close it. this happens during message delivery. that doesn't
work on windows, the rename/remove would fail because the file is still open.
so this commit swaps many "remove" and "close" calls. renames are a longer
story: message delivery had two ways to deliver: with "consuming" the
(temporary) message file (which would rename it to its final destination), and
without consuming (by hardlinking the file, falling back to copying). the last
delivery to a recipient of a message (and the only one in the common case of a
single recipient) would consume the message, and the earlier recipients would
not.  during delivery, the already open message file was used, to parse the
message.  we still want to use that open message file, and the caller now stays
responsible for closing it, but we no longer try to rename (consume) the file.
we always hardlink (or copy) during delivery (this works on windows), and the
caller is responsible for closing and removing (in that order) the original
temporary file. this does cost one syscall more. but it makes the delivery code
(responsibilities) a bit simpler.

there is one more obvious issue: the file system path separator. mox already
used the "filepath" package to join paths in many places, but not everywhere.
and it still used strings with slashes for local file access. with this commit,
the code now uses filepath.FromSlash for path strings with slashes, uses
"filepath" in a few more places where it previously didn't. also switches from
"filepath" to regular "path" package when handling mailbox names in a few
places, because those always use forward slashes, regardless of local file
system conventions.  windows can handle forward slashes when opening files, so
test code that passes path strings with forward slashes straight to go stdlib
file i/o functions are left unchanged to reduce code churn. the regular
non-test code, or test code that uses path strings in places other than
standard i/o functions, does have the paths converted for consistent paths
(otherwise we would end up with paths with mixed forward/backward slashes in
log messages).

windows cannot dup a listening socket. for "mox localserve", it isn't
important, and we can work around the issue. the current approach for "mox
serve" (forking a process and passing file descriptors of listening sockets on
"privileged" ports) won't work on windows. perhaps it isn't needed on windows,
and any user can listen on "privileged" ports? that would be welcome.

on windows, os.Open cannot open a directory, so we cannot call Sync on it after
message delivery. a cursory internet search indicates that directories cannot
be synced on windows. the story is probably much more nuanced than that, with
long deep technical details/discussions/disagreement/confusion, like on unix.
for "mox localserve" we can get away with making syncdir a no-op.
This commit is contained in:
Mechiel Lukkien
2023-10-14 10:54:07 +02:00
parent 96774de8d6
commit 28fae96a9b
78 changed files with 1155 additions and 938 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
store/account.go
View File

@ -1189,9 +1189,6 @@ func (a *Account) WithRLock(fn func()) {
// DeliverMessage delivers a mail message to the account.
//
// If consumeFile is set, the original msgFile is moved/renamed or copied and
// removed as part of delivery.
//
// The message, with msg.MsgPrefix and msgFile combined, must have a header
// section. The caller is responsible for adding a header separator to
// msg.MsgPrefix if missing from an incoming message.
@ -1210,7 +1207,7 @@ func (a *Account) WithRLock(fn func()) {
// Caller must broadcast new message.
//
// Caller must update mailbox counts.
func (a *Account) DeliverMessage(log *mlog.Log, tx *bstore.Tx, m *Message, msgFile *os.File, consumeFile, sync, notrain, nothreads bool) error {
func (a *Account) DeliverMessage(log *mlog.Log, tx *bstore.Tx, m *Message, msgFile *os.File, sync, notrain, nothreads bool) error {
if m.Expunged {
return fmt.Errorf("cannot deliver expunged message")
}
@ -1346,12 +1343,7 @@ func (a *Account) DeliverMessage(log *mlog.Log, tx *bstore.Tx, m *Message, msgFi
}
}
if consumeFile {
if err := os.Rename(msgFile.Name(), msgPath); err != nil {
// Could be due to cross-filesystem rename. Users shouldn't configure their systems that way.
return fmt.Errorf("moving msg file to destination directory: %w", err)
}
} else if err := moxio.LinkOrCopy(log, msgPath, msgFile.Name(), &moxio.AtReader{R: msgFile}, true); err != nil {
if err := moxio.LinkOrCopy(log, msgPath, msgFile.Name(), &moxio.AtReader{R: msgFile}, true); err != nil {
return fmt.Errorf("linking/copying message to new file: %w", err)
}
@ -1647,7 +1639,7 @@ ruleset:
// MessagePath returns the file system path of a message.
func (a *Account) MessagePath(messageID int64) string {
return strings.Join(append([]string{a.Dir, "msg"}, messagePathElems(messageID)...), "/")
return strings.Join(append([]string{a.Dir, "msg"}, messagePathElems(messageID)...), string(filepath.Separator))
}
// MessageReader opens a message for reading, transparently combining the
@ -1661,7 +1653,7 @@ func (a *Account) MessageReader(m Message) *MsgReader {
// Caller must hold account wlock (mailbox may be created).
// Message delivery, possible mailbox creation, and updated mailbox counts are
// broadcasted.
func (a *Account) DeliverDestination(log *mlog.Log, dest config.Destination, m *Message, msgFile *os.File, consumeFile bool) error {
func (a *Account) DeliverDestination(log *mlog.Log, dest config.Destination, m *Message, msgFile *os.File) error {
var mailbox string
rs := MessageRuleset(log, dest, m, m.MsgPrefix, msgFile)
if rs != nil {
@ -1671,7 +1663,7 @@ func (a *Account) DeliverDestination(log *mlog.Log, dest config.Destination, m *
} else {
mailbox = dest.Mailbox
}
return a.DeliverMailbox(log, mailbox, m, msgFile, consumeFile)
return a.DeliverMailbox(log, mailbox, m, msgFile)
}
// DeliverMailbox delivers an email to the specified mailbox.
@ -1679,7 +1671,7 @@ func (a *Account) DeliverDestination(log *mlog.Log, dest config.Destination, m *
// Caller must hold account wlock (mailbox may be created).
// Message delivery, possible mailbox creation, and updated mailbox counts are
// broadcasted.
func (a *Account) DeliverMailbox(log *mlog.Log, mailbox string, m *Message, msgFile *os.File, consumeFile bool) error {
func (a *Account) DeliverMailbox(log *mlog.Log, mailbox string, m *Message, msgFile *os.File) error {
var changes []Change
err := a.DB.Write(context.TODO(), func(tx *bstore.Tx) error {
mb, chl, err := a.MailboxEnsure(tx, mailbox, true)
@ -1696,7 +1688,7 @@ func (a *Account) DeliverMailbox(log *mlog.Log, mailbox string, m *Message, msgF
return fmt.Errorf("updating mailbox for delivery: %w", err)
}
if err := a.DeliverMessage(log, tx, m, msgFile, consumeFile, true, false, false); err != nil {
if err := a.DeliverMessage(log, tx, m, msgFile, true, false, false); err != nil {
return err
}
@ -1977,10 +1969,11 @@ func OpenEmail(email string) (*Account, config.Destination, error) {
// 64 characters, must be power of 2 for MessagePath
const msgDirChars = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ-_"
// MessagePath returns the filename of the on-disk filename, relative to the containing directory such as <account>/msg or queue.
// MessagePath returns the filename of the on-disk filename, relative to the
// containing directory such as <account>/msg or queue.
// Returns names like "AB/1".
func MessagePath(messageID int64) string {
return strings.Join(messagePathElems(messageID), "/")
return strings.Join(messagePathElems(messageID), string(filepath.Separator))
}
// messagePathElems returns the elems, for a single join without intermediate

18
store/account_test.go
View File

@ -3,6 +3,7 @@ package store
import (
"context"
"os"
"path/filepath"
"regexp"
"strings"
"testing"
@ -28,7 +29,7 @@ func tcheck(t *testing.T, err error, msg string) {
func TestMailbox(t *testing.T) {
os.RemoveAll("../testdata/store/data")
mox.ConfigStaticPath = "../testdata/store/mox.conf"
mox.ConfigStaticPath = filepath.FromSlash("../testdata/store/mox.conf")
mox.MustLoadConfig(true, false)
acc, err := OpenAccount("mjl")
tcheck(t, err, "open account")
@ -44,6 +45,7 @@ func TestMailbox(t *testing.T) {
if err != nil {
t.Fatalf("creating temp msg file: %s", err)
}
defer os.Remove(msgFile.Name())
defer msgFile.Close()
msgWriter := message.NewWriter(msgFile)
if _, err := msgWriter.Write([]byte(" message")); err != nil {
@ -70,7 +72,7 @@ func TestMailbox(t *testing.T) {
}
acc.WithWLock(func() {
conf, _ := acc.Conf()
err := acc.DeliverDestination(xlog, conf.Destinations["mjl"], &m, msgFile, false)
err := acc.DeliverDestination(xlog, conf.Destinations["mjl"], &m, msgFile)
tcheck(t, err, "deliver without consume")
err = acc.DB.Write(ctxbg, func(tx *bstore.Tx) error {
@ -79,7 +81,7 @@ func TestMailbox(t *testing.T) {
tcheck(t, err, "sent mailbox")
msent.MailboxID = mbsent.ID
msent.MailboxOrigID = mbsent.ID
err = acc.DeliverMessage(xlog, tx, &msent, msgFile, false, true, false, false)
err = acc.DeliverMessage(xlog, tx, &msent, msgFile, true, false, false)
tcheck(t, err, "deliver message")
if !msent.ThreadMuted || !msent.ThreadCollapsed {
t.Fatalf("thread muted & collapsed should have been copied from parent (duplicate message-id) m")
@ -95,7 +97,7 @@ func TestMailbox(t *testing.T) {
tcheck(t, err, "insert rejects mailbox")
mreject.MailboxID = mbrejects.ID
mreject.MailboxOrigID = mbrejects.ID
err = acc.DeliverMessage(xlog, tx, &mreject, msgFile, false, true, false, false)
err = acc.DeliverMessage(xlog, tx, &mreject, msgFile, true, false, false)
tcheck(t, err, "deliver message")
err = tx.Get(&mbrejects)
@ -108,7 +110,7 @@ func TestMailbox(t *testing.T) {
})
tcheck(t, err, "deliver as sent and rejects")
err = acc.DeliverDestination(xlog, conf.Destinations["mjl"], &mconsumed, msgFile, true)
err = acc.DeliverDestination(xlog, conf.Destinations["mjl"], &mconsumed, msgFile)
tcheck(t, err, "deliver with consume")
err = acc.DB.Write(ctxbg, func(tx *bstore.Tx) error {
@ -251,9 +253,11 @@ func TestMailbox(t *testing.T) {
}
func TestMessageRuleset(t *testing.T) {
f, err := os.Open("/dev/null")
tcheck(t, err, "open")
f, err := CreateMessageTemp("msgruleset")
tcheck(t, err, "creating temp msg file")
defer os.Remove(f.Name())
defer f.Close()
msgBuf := []byte(strings.ReplaceAll(`List-ID: <test.mox.example>
test

13
store/export.go
View File

@ -82,10 +82,11 @@ type DirArchiver struct {
}
// Create create name in the file system, in dir.
// name must always use forwarded slashes.
func (a DirArchiver) Create(name string, size int64, mtime time.Time) (io.WriteCloser, error) {
isdir := strings.HasSuffix(name, "/")
name = strings.TrimSuffix(name, "/")
p := filepath.Join(a.Dir, name)
p := filepath.Join(a.Dir, filepath.FromSlash(name))
os.MkdirAll(filepath.Dir(p), 0770)
if isdir {
return nil, os.Mkdir(p, 0770)
@ -213,8 +214,11 @@ func ExportMessages(ctx context.Context, log *mlog.Log, db *bstore.DB, accountDi
var mboxwriter *bufio.Writer
defer func() {
if mboxtmp != nil {
name := mboxtmp.Name()
err := mboxtmp.Close()
log.Check(err, "closing mbox temp file")
err = os.Remove(name)
log.Check(err, "removing mbox temp file", mlog.Field("name", name))
}
}()
@ -287,8 +291,11 @@ func ExportMessages(ctx context.Context, log *mlog.Log, db *bstore.DB, accountDi
if err := w.Close(); err != nil {
return fmt.Errorf("closing message file: %v", err)
}
name := mboxtmp.Name()
err = mboxtmp.Close()
log.Check(err, "closing temporary mbox file")
err = os.Remove(name)
log.Check(err, "removing temporary mbox file", mlog.Field("path", name))
mboxwriter = nil
mboxtmp = nil
return nil
@ -524,10 +531,6 @@ func ExportMessages(ctx context.Context, log *mlog.Log, db *bstore.DB, accountDi
if err != nil {
return fmt.Errorf("creating temp mbox file: %v", err)
}
// Remove file immediately, so we are sure we don't leave it around.
if err := os.Remove(mboxtmp.Name()); err != nil {
return fmt.Errorf("removing temp file just created: %v", err)
}
mboxwriter = bufio.NewWriter(mboxtmp)
}
}

15
store/export_test.go
View File

@ -20,7 +20,7 @@ func TestExport(t *testing.T) {
// and maildir/mbox. check there are 2 files in the repo, no errors.txt.
os.RemoveAll("../testdata/store/data")
mox.ConfigStaticPath = "../testdata/store/mox.conf"
mox.ConfigStaticPath = filepath.FromSlash("../testdata/store/mox.conf")
mox.MustLoadConfig(true, false)
acc, err := OpenAccount("mjl")
tcheck(t, err, "open account")
@ -32,16 +32,17 @@ func TestExport(t *testing.T) {
msgFile, err := CreateMessageTemp("mox-test-export")
tcheck(t, err, "create temp")
defer os.Remove(msgFile.Name()) // To be sure.
defer msgFile.Close()
const msg = "test: test\r\n\r\ntest\r\n"
_, err = msgFile.Write([]byte(msg))
tcheck(t, err, "write message")
m := Message{Received: time.Now(), Size: int64(len(msg))}
err = acc.DeliverMailbox(xlog, "Inbox", &m, msgFile, false)
err = acc.DeliverMailbox(xlog, "Inbox", &m, msgFile)
tcheck(t, err, "deliver")
m = Message{Received: time.Now(), Size: int64(len(msg))}
err = acc.DeliverMailbox(xlog, "Trash", &m, msgFile, true)
err = acc.DeliverMailbox(xlog, "Trash", &m, msgFile)
tcheck(t, err, "deliver")
var maildirZip, maildirTar, mboxZip, mboxTar bytes.Buffer
@ -61,8 +62,8 @@ func TestExport(t *testing.T) {
archive(ZipArchiver{zip.NewWriter(&mboxZip)}, false)
archive(TarArchiver{tar.NewWriter(&maildirTar)}, true)
archive(TarArchiver{tar.NewWriter(&mboxTar)}, false)
archive(DirArchiver{"../testdata/exportmaildir"}, true)
archive(DirArchiver{"../testdata/exportmbox"}, false)
archive(DirArchiver{filepath.FromSlash("../testdata/exportmaildir")}, true)
archive(DirArchiver{filepath.FromSlash("../testdata/exportmbox")}, false)
if r, err := zip.NewReader(bytes.NewReader(maildirZip.Bytes()), int64(maildirZip.Len())); err != nil {
t.Fatalf("reading maildir zip: %v", err)
@ -115,6 +116,6 @@ func TestExport(t *testing.T) {
}
}
checkDirFiles("../testdata/exportmaildir", 2)
checkDirFiles("../testdata/exportmbox", 2)
checkDirFiles(filepath.FromSlash("../testdata/exportmaildir"), 2)
checkDirFiles(filepath.FromSlash("../testdata/exportmbox"), 2)
}

14
store/import.go
View File

@ -84,10 +84,11 @@ func (mr *MboxReader) Next() (*Message, *os.File, string, error) {
}
defer func() {
if f != nil {
err := os.Remove(f.Name())
mr.log.Check(err, "removing temporary message file after mbox read error", mlog.Field("path", f.Name()))
err = f.Close()
name := f.Name()
err := f.Close()
mr.log.Check(err, "closing temporary message file after mbox read error")
err = os.Remove(name)
mr.log.Check(err, "removing temporary message file after mbox read error", mlog.Field("path", name))
}
}()
@ -272,10 +273,11 @@ func (mr *MaildirReader) Next() (*Message, *os.File, string, error) {
}
defer func() {
if f != nil {
err := os.Remove(f.Name())
mr.log.Check(err, "removing temporary message file after maildir read error", mlog.Field("path", f.Name()))
err = f.Close()
name := f.Name()
err := f.Close()
mr.log.Check(err, "closing temporary message file after maildir read error")
err = os.Remove(name)
mr.log.Check(err, "removing temporary message file after maildir read error", mlog.Field("path", name))
}
}()

8
store/import_test.go
View File

@ -24,15 +24,15 @@ func TestMboxReader(t *testing.T) {
if err != nil {
t.Fatalf("next mbox message: %v", err)
}
defer mf0.Close()
defer os.Remove(mf0.Name())
defer mf0.Close()
_, mf1, _, err := mr.Next()
if err != nil {
t.Fatalf("next mbox message: %v", err)
}
defer mf1.Close()
defer os.Remove(mf1.Name())
defer mf1.Close()
_, _, _, err = mr.Next()
if err != io.EOF {
@ -62,15 +62,15 @@ func TestMaildirReader(t *testing.T) {
if err != nil {
t.Fatalf("next maildir message: %v", err)
}
defer mf0.Close()
defer os.Remove(mf0.Name())
defer mf0.Close()
_, mf1, _, err := mr.Next()
if err != nil {
t.Fatalf("next maildir message: %v", err)
}
defer mf1.Close()
defer os.Remove(mf1.Name())
defer mf1.Close()
_, _, _, err = mr.Next()
if err != io.EOF {

11
store/msgreader_test.go
View File

@ -12,7 +12,13 @@ func TestMsgreader(t *testing.T) {
t.Fatalf("expected error for non-existing file, got %s", err)
}
if buf, err := io.ReadAll(&MsgReader{prefix: []byte("hello"), path: "/dev/null", size: int64(len("hello"))}); err != nil {
if err := os.WriteFile("emptyfile_test.txt", []byte{}, 0660); err != nil {
t.Fatalf("writing emptyfile_test.txt: %s", err)
}
defer os.Remove("emptyfile_test.txt")
mr := &MsgReader{prefix: []byte("hello"), path: "emptyfile_test.txt", size: int64(len("hello"))}
defer mr.Close()
if buf, err := io.ReadAll(mr); err != nil {
t.Fatalf("readall: %s", err)
} else if string(buf) != "hello" {
t.Fatalf("got %q, expected %q", buf, "hello")
@ -22,7 +28,8 @@ func TestMsgreader(t *testing.T) {
t.Fatalf("writing msgreader_test.txt: %s", err)
}
defer os.Remove("msgreader_test.txt")
mr := &MsgReader{prefix: []byte("hello"), path: "msgreader_test.txt", size: int64(len("hello world"))}
mr = &MsgReader{prefix: []byte("hello"), path: "msgreader_test.txt", size: int64(len("hello world"))}
defer mr.Close()
if buf, err := io.ReadAll(mr); err != nil {
t.Fatalf("readall: %s", err)
} else if string(buf) != "hello world" {

6
store/threads_test.go
View File

@ -2,6 +2,7 @@ package store
import (
"os"
"path/filepath"
"reflect"
"strings"
"testing"
@ -15,7 +16,7 @@ import (
func TestThreadingUpgrade(t *testing.T) {
os.RemoveAll("../testdata/store/data")
mox.ConfigStaticPath = "../testdata/store/mox.conf"
mox.ConfigStaticPath = filepath.FromSlash("../testdata/store/mox.conf")
mox.MustLoadConfig(true, false)
acc, err := OpenAccount("mjl")
tcheck(t, err, "open account")
@ -32,6 +33,7 @@ func TestThreadingUpgrade(t *testing.T) {
t.Helper()
f, err := CreateMessageTemp("account-test")
tcheck(t, err, "temp file")
defer os.Remove(f.Name())
defer f.Close()
s = strings.ReplaceAll(s, "\n", "\r\n")
@ -40,7 +42,7 @@ func TestThreadingUpgrade(t *testing.T) {
MsgPrefix: []byte(s),
Received: recv,
}
err = acc.DeliverMailbox(log, "Inbox", &m, f, true)
err = acc.DeliverMailbox(log, "Inbox", &m, f)
tcheck(t, err, "deliver")
if expThreadID == 0 {
expThreadID = m.ID