foxcpp / maddy

✉️ Composable all-in-one mail server.
https://maddy.email
GNU General Public License v3.0
5.14k stars 249 forks source link

fix(session): detect canceled lookup correctly #626

Closed mmatous closed 10 months ago

mmatous commented 1 year ago

Hi, this will prevent smtp: rDNS error {"reason":"","src_ip":"x.x.x.x:port"} for every incoming message. Weirdly, it didn't start appearing before using milter/rspamd. It should be fixed regardless, but I don't really know what changed. Maybe timeout expiring while waiting for rspamd response? Or rspamd hogging DNS resolver time for it's own checks.

mmatous commented 1 year ago

I'm having trouble reproducing this error. I prepared the following scripts to simulate your CI and test various go versions

FROM ubuntu:20.04

RUN set -ex && \
    apt update -y && apt dist-upgrade -y && \
    apt install -y build-essential wget git libpam-dev && \
    wget 'https://github.com/mmatous/maddy/archive/refs/heads/dev.tar.gz' && \
    tar -xzf dev.tar.gz

RUN wget 'https://go.dev/dl/go__VER__.linux-amd64.tar.gz' && \
    tar -C /usr/local -xzf 'go__VER__.linux-amd64.tar.gz'

ENV PATH="${PATH}:/usr/local/go/bin"
WORKDIR /maddy-dev
RUN ./build.sh && \
    go test ./... -coverprofile=coverage.out -covermode=atomic
WORKDIR ./tests
ENTRYPOINT ./run.sh
#!/usr/bin/env bash

sed "s/__VER__/$1/g" ./Dockerfile.template > "./Dockerfile.$1"
docker build --tag "maddy-$1" --file "./Dockerfile.$1" .

Usage e.g.:

sudo ./dbuild.sh 1.18.9
sudo docker run --rm --interactive --tty maddy-1.18.9

I install seemingly unnecessary packages for cases where I wanted to look around manually and running integration tests is left outside of build phase so it can be called in a loop. Anyway, I found that the tests almost always (~95 %) pass while intermittent errors can happen. For my changes I managed to get

--- FAIL: TestDNSBLConfig (0.05s)
    t.go:159: Using /tmp/maddy-tests-2406491360
    t.go:209: launching [./maddy.cover -config /tmp/maddy-tests-2406491360/maddy.conf -debug.smtpport 0 -debug.dnsoverride 127.0.0.1:36663 -log stderr -test.coverprofile /tmp/maddy-coverage-re
port.178a71122d6de9c7]
    t.go:254: maddy: dnsbl: List does not contain a test record for 127.0.0.2   {"list":"dnsbl.test"}
    t.go:254: maddy: dnsbl: List does not contain a test record for ::FFFF:7F00:2       {"list":"dnsbl.test"}
    t.go:254: maddy: dnsbl: List does not contain a test record for 'test' TLD  {"list":"dnsbl.test"}
    t.go:263: Log ended before all expected listeners are up. Start-up error?
FAIL
exit status 1
FAIL    github.com/foxcpp/maddy/tests   10.337s

Or a different run:

--- FAIL: TestIMAPEndpointAuthMap (0.11s)
    t.go:159: Using /tmp/maddy-tests-2317279703
    t.go:209: launching [./maddy.cover -config /tmp/maddy-tests-2317279703/maddy.conf -debug.smtpport 0 -debug.dnsoverride 127.0.0.1:42773 -log stderr -test.coverprofile /tmp/maddy-coverage-re
port.178a72c1edaef9dc]
    t.go:254: maddy: imap: listening on tcp://127.0.0.1:43717    (test runner>listener wait trigger<)
    t.go:254: maddy: imap: TLS is disabled, this is insecure configuration and should be used only for testing!
    imap_test.go:39: 47167 < imap: * OK [CAPABILITY IMAP4rev1 LITERAL+ SASL-IR CHILDREN UNSELECT MOVE IDLE APPENDLIMIT LOGINDISABLED COMPRESS] IMAP4rev1 Service Ready
    imap_test.go:40: 47167 > imap: . LOGIN user@example.org 123
    imap_test.go:41: 47167 < imap: . NO Authentication disabled
    imap_test.go:41: Response line not matching the expected pattern, want ". OK *"
    t.go:254: maddy: signal received (interrupt), next signal will force immediate shutdown.
FAIL
exit status 1
FAIL    github.com/foxcpp/maddy/tests   11.100s

And even on current dev HEAD without my changes:

smtp/server 2023/10/03 01:34:41 handler error: read tcp 127.0.0.1:49947->127.0.0.1:46800: read: connection reset by peer
smtp/server 2023/10/03 01:34:41 handler error: read tcp 127.0.0.1:50278->127.0.0.1:53322: read: connection reset by peer
--- FAIL: TestSMTPFlood_EnvelopeAbort_NoLimits_10Conns (0.05s)
    t.go:159: Using /tmp/maddy-tests-3724608267
    t.go:209: launching [./maddy.cover -config /tmp/maddy-tests-3724608267/maddy.conf -debug.smtpport 0 -debug.dnsoverride 127.0.0.1:34535 -log stderr -test.coverprofile /tmp/maddy-coverage-re
port.178a750869db92e1]
    t.go:263: Log ended before all expected listeners are up. Start-up error?
FAIL
exit status 1
FAIL    github.com/foxcpp/maddy/tests   12.543s

I only got consistent failures from go 1.20 onward and that was unit tests (likely expected for dev branch). Unless there's a mistake in my approach, I'm currently inclined to chalk the failure up to brittle tests. Do you think the PR is erroneous?

foxcpp commented 10 months ago

Cherry-picked into master as c67955ef0d5615d493f69bba3fd23f46d27a8d0a. Thanks!