From 6f57c51934e7f93ffd7800caffea3e932c529bea Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sun, 17 Dec 2017 20:13:14 -0800 Subject: [PATCH 01/25] Update release procedures, cleanup goxc config --- .gitignore | 4 ---- .goxc.json | 18 ------------------ CHANGELOG.md | 13 ++++++------- 3 files changed, 6 insertions(+), 29 deletions(-) delete mode 100644 .goxc.json diff --git a/.gitignore b/.gitignore index 8a701bd..52aeb50 100644 --- a/.gitignore +++ b/.gitignore @@ -29,9 +29,5 @@ _testmain.go /inbucket /inbucket.exe /dist/** -/target/** /cmd/client/client /cmd/client/client.exe - -# local goxc config -.goxc.local.json diff --git a/.goxc.json b/.goxc.json deleted file mode 100644 index b78010a..0000000 --- a/.goxc.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "ArtifactsDest": "target", - "TasksExclude": [ - "pkg-build" - ], - "Arch": "amd64", - "Os": "darwin freebsd linux windows", - "ResourcesInclude": "README*,LICENSE*,CHANGELOG*,inbucket.bat,etc,themes", - "PackageVersion": "1.2.0", - "PrereleaseInfo": "rc2", - "ConfigVersion": "0.9", - "BuildSettings": { - "LdFlagsXVars": { - "TimeNow": "main.BUILDDATE", - "Version": "main.VERSION" - } - } -} \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f2972f..554bfd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,12 +108,11 @@ Release Checklist - Ensure *Unreleased* section is up to date - Rename *Unreleased* section to release name and date. - Add new GitHub `/compare` link -3. Update goxc version info: `goxc -wc -pv=1.x.0 -pr=rc1` -4. Run: `goxc interpolate-source` to update VERSION var -5. Run tests -6. Test cross-compile: `goxc` -7. Commit changes and merge release: `git flow release finish 1.x.0` -8. Upload to bintray: `goxc bintray` -9. Update `binary_versions` option in `inbucket-site/_config.yml` +3. Run tests +4. Test cross-compile: `goreleaser --snapshot` +5. Commit changes and merge release: `git flow release finish` +6. Push tags and wait for https://travis-ci.org/jhillyerd/inbucket build to + complete +7. Update `binary_versions` option in `inbucket-site/_config.yml` See http://keepachangelog.com/ for additional instructions on how to update this file. From 8040b07c2876a9055332c60a88f5f677ac85528d Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Mon, 18 Dec 2017 05:36:14 +0100 Subject: [PATCH 02/25] Button to delete the mailbox from the UI (#65), closes #55 --- themes/bootstrap/public/mailbox.js | 13 +++++++++++++ themes/bootstrap/templates/mailbox/index.html | 8 ++++++++ 2 files changed, 21 insertions(+) diff --git a/themes/bootstrap/public/mailbox.js b/themes/bootstrap/public/mailbox.js index f7e229a..3b26a99 100644 --- a/themes/bootstrap/public/mailbox.js +++ b/themes/bootstrap/public/mailbox.js @@ -22,6 +22,19 @@ function deleteMessage(id) { }) } +// Delete the mailbox +function deleteMailBox() { + cont = confirm("Are you sure you want delete the Mailbox?") + if (cont == false) { + return; + } + $.ajax({ + type: 'DELETE', + url: '/api/v1/mailbox/' + mailbox, + success: loadList + }) +} + // flashTooltip temporarily changes the text of a tooltip function flashTooltip(el, text) { var prevText = $(el).attr('data-original-title'); diff --git a/themes/bootstrap/templates/mailbox/index.html b/themes/bootstrap/templates/mailbox/index.html index 3af855e..8103bac 100644 --- a/themes/bootstrap/templates/mailbox/index.html +++ b/themes/bootstrap/templates/mailbox/index.html @@ -57,6 +57,14 @@ $(document).ready(function() { onclick="loadList()"> +
From 10cce5c7512062d5a0c4a19370931420763d4692 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sun, 17 Dec 2017 21:05:48 -0800 Subject: [PATCH 03/25] Fix version & date in Docker containers for #64 --- etc/docker/install.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/etc/docker/install.sh b/etc/docker/install.sh index 10a1a2b..036de83 100755 --- a/etc/docker/install.sh +++ b/etc/docker/install.sh @@ -15,11 +15,12 @@ apk add --no-cache --virtual .build-deps git # Setup export GOBIN="$bindir" -builddate="$(date -Iseconds)" cd "$srcdir" -go clean +builddate="$(date -Iseconds)" +buildver="$(git describe --tags)" # Build +go clean echo "### Fetching Dependencies" go get -t -v ./... @@ -27,7 +28,7 @@ echo "### Testing Inbucket" go test ./... echo "### Building Inbucket" -go build -o inbucket -ldflags "-X 'main.BUILDDATE=$builddate'" -v . +go build -o inbucket -ldflags "-X 'main.version=$buildver' -X 'main.date=$builddate'" -v . echo "### Installing Inbucket" set -x From 5bca2ae7380870887c89b5f39e7abc4be400b333 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sun, 17 Dec 2017 21:29:57 -0800 Subject: [PATCH 04/25] Fetch tags during docker build --- etc/docker/install.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/docker/install.sh b/etc/docker/install.sh index 036de83..f13ed94 100755 --- a/etc/docker/install.sh +++ b/etc/docker/install.sh @@ -16,6 +16,7 @@ apk add --no-cache --virtual .build-deps git # Setup export GOBIN="$bindir" cd "$srcdir" +git fetch -t builddate="$(date -Iseconds)" buildver="$(git describe --tags)" From 9d68e2c0a5d4955b10f520e1dc86d590fb363875 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sun, 17 Dec 2017 21:43:46 -0800 Subject: [PATCH 05/25] Docker version will now fall back to commit if no tag --- etc/docker/install.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/etc/docker/install.sh b/etc/docker/install.sh index f13ed94..916c745 100755 --- a/etc/docker/install.sh +++ b/etc/docker/install.sh @@ -16,9 +16,10 @@ apk add --no-cache --virtual .build-deps git # Setup export GOBIN="$bindir" cd "$srcdir" +# Fetch tags for describe git fetch -t builddate="$(date -Iseconds)" -buildver="$(git describe --tags)" +buildver="$(git describe --tags --always)" # Build go clean From 52de1b2bfef2916486d4da62350b16f4dec2d53b Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sat, 23 Dec 2017 23:22:51 -0800 Subject: [PATCH 06/25] Initial gcloud setup.sh, not yet tested as metadata --- etc/gcloud/setup.sh | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 etc/gcloud/setup.sh diff --git a/etc/gcloud/setup.sh b/etc/gcloud/setup.sh new file mode 100644 index 0000000..52fa23b --- /dev/null +++ b/etc/gcloud/setup.sh @@ -0,0 +1,33 @@ +#!/bin/bash +# setup.sh +# description: Install Inbucket on Google Cloud debian9 instance + +release="inbucket_1.2.0-rc2_linux_amd64" +url="https://dl.bintray.com/content/jhillyerd/golang/${release}.tar.gz?direct" + +set -eo pipefail +[ $TRACE ] && set -x + +# Prerequisites +apt-get --yes update +apt-get --yes install curl libcap2-bin +id inbucket &>/dev/null || useradd -r -m inbucket + +# Extract +cd /opt +curl --location "$url" | tar xzvf - +ln -s "$release/" inbucket + +# Install +cd /opt/inbucket/etc/ubuntu +install -o inbucket -g inbucket -m 775 -d /var/opt/inbucket +touch /var/log/inbucket.log +chown inbucket: /var/log/inbucket.log +install -o root -g root -m 644 inbucket.logrotate /etc/logrotate.d/inbucket +install -o root -g root -m 644 inbucket.service /lib/systemd/system/inbucket.service +install -o root -g root -m 644 ../unix-sample.conf /etc/opt/inbucket.conf + +# Setup +setcap 'cap_net_bind_service=+ep' /opt/inbucket/inbucket +systemctl enable inbucket.service +systemctl start inbucket From 0e72b414c4396f3c7d1444da3b67c626a8303c6e Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sun, 24 Dec 2017 10:40:12 -0800 Subject: [PATCH 07/25] Add fauxmailer to gcloud, custom greeting --- etc/gcloud/create-demo-instance.sh | 8 ++++++++ etc/gcloud/{setup.sh => debian-startup.sh} | 19 +++++++++++++++---- etc/gcloud/demo-greeting.html | 15 +++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100755 etc/gcloud/create-demo-instance.sh rename etc/gcloud/{setup.sh => debian-startup.sh} (53%) mode change 100644 => 100755 create mode 100644 etc/gcloud/demo-greeting.html diff --git a/etc/gcloud/create-demo-instance.sh b/etc/gcloud/create-demo-instance.sh new file mode 100755 index 0000000..039be6f --- /dev/null +++ b/etc/gcloud/create-demo-instance.sh @@ -0,0 +1,8 @@ +#!/bin/bash +# create-demo-instance.sh + +set -x +gcloud compute instances create inbucket-1 --machine-type=f1-micro \ + --image-project=debian-cloud --image-family=debian-9 \ + --metadata-from-file=startup-script=debian-startup.sh,greeting=demo-greeting.html \ + --tags=http-server --address=inbucket-demo diff --git a/etc/gcloud/setup.sh b/etc/gcloud/debian-startup.sh old mode 100644 new mode 100755 similarity index 53% rename from etc/gcloud/setup.sh rename to etc/gcloud/debian-startup.sh index 52fa23b..a5989e7 --- a/etc/gcloud/setup.sh +++ b/etc/gcloud/debian-startup.sh @@ -2,8 +2,13 @@ # setup.sh # description: Install Inbucket on Google Cloud debian9 instance -release="inbucket_1.2.0-rc2_linux_amd64" -url="https://dl.bintray.com/content/jhillyerd/golang/${release}.tar.gz?direct" +inbucket_rel="1.2.0-rc2" +inbucket_pkg="inbucket_${inbucket_rel}_linux_amd64" +inbucket_url="https://dl.bintray.com/content/jhillyerd/golang/${inbucket_pkg}.tar.gz?direct" + +fauxmailer_rel="0.1" +fauxmailer_pkg="fauxmailer_${fauxmailer_rel}_linux_amd64" +fauxmailer_url="https://github.com/jhillyerd/fauxmailer/releases/download/0.1/${fauxmailer_pkg}.tar.gz" set -eo pipefail [ $TRACE ] && set -x @@ -15,8 +20,8 @@ id inbucket &>/dev/null || useradd -r -m inbucket # Extract cd /opt -curl --location "$url" | tar xzvf - -ln -s "$release/" inbucket +curl --location "$inbucket_url" | tar xzvf - +ln -s "$inbucket_pkg/" inbucket # Install cd /opt/inbucket/etc/ubuntu @@ -29,5 +34,11 @@ install -o root -g root -m 644 ../unix-sample.conf /etc/opt/inbucket.conf # Setup setcap 'cap_net_bind_service=+ep' /opt/inbucket/inbucket +curl -sL -o /opt/inbucket/themes/greeting.html "http://metadata.google.internal/computeMetadata/v1/instance/attributes/greeting" -H "Metadata-Flavor: Google" systemctl enable inbucket.service systemctl start inbucket + +# Fauxmailer +cd /opt +curl --location "$fauxmailer_url" | tar xzvf - +ln -s "$fauxmailer_pkg/" fauxmailer diff --git a/etc/gcloud/demo-greeting.html b/etc/gcloud/demo-greeting.html new file mode 100644 index 0000000..6a4e85b --- /dev/null +++ b/etc/gcloud/demo-greeting.html @@ -0,0 +1,15 @@ +

Welcome to the Inbucket demonstration instance

+ +

tl;dr - click Monitor above to get started.

+ +

Inbucket is an email testing service; it will accept email for any email +address and make it available to view without a password.

+ +

This instance is running on Google Compute Engine, and utilizes +fauxmailer to generate +a stream of fake email. Feel free to poke around, delete mails, or try out +the REST API.

+ +

To protect our visitors, this instance does not accept external mail. You +will need to install your own copy of Inbucket to use it in your +environment.

From 81eba8f51ace7b9495aa947d94ef50611bac104c Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sun, 24 Dec 2017 13:37:47 -0800 Subject: [PATCH 08/25] Only deploy with one version of Go --- .travis.yml | 15 +++++++++++---- etc/travis-deploy.sh | 10 ++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100755 etc/travis-deploy.sh diff --git a/.travis.yml b/.travis.yml index a407e1f..59d36e9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,14 +1,21 @@ language: go sudo: false +env: + - DEPLOY_WITH_MAJOR="1.9" + before_script: - go vet ./... go: - - 1.8.5 - - 1.9.2 + - 1.8.x + - 1.9.x script: go test -race -v ./... -after_success: - - test -n "$TRAVIS_TAG" && curl -sL https://git.io/goreleaser | bash +deploy: + provider: script + script: etc/travis-deploy.sh + on: + tags: true + branch: master diff --git a/etc/travis-deploy.sh b/etc/travis-deploy.sh new file mode 100755 index 0000000..e93c363 --- /dev/null +++ b/etc/travis-deploy.sh @@ -0,0 +1,10 @@ +#!/bin/bash +# travis-deploy.sh +# description: Trigger goreleaser deployment in correct build scenarios + +set -eo pipefail +set -x + +if [[ "$TRAVIS_GO_VERSION" == "$DEPLOY_WITH_MAJOR."* ]]; then + curl -sL https://git.io/goreleaser | bash +fi From 76a77beca99c498814adb4e24c03b3f9fa0c5644 Mon Sep 17 00:00:00 2001 From: adrium Date: Sun, 24 Dec 2017 22:59:04 +0100 Subject: [PATCH 09/25] Reverse message display sort order (#59) Closes #60 --- themes/bootstrap/public/mailbox.js | 2 +- themes/bootstrap/public/monitor.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/themes/bootstrap/public/mailbox.js b/themes/bootstrap/public/mailbox.js index 3b26a99..886c9a5 100644 --- a/themes/bootstrap/public/mailbox.js +++ b/themes/bootstrap/public/mailbox.js @@ -56,7 +56,7 @@ function loadList() { dataType: "json", url: '/api/v1/mailbox/' + mailbox, success: function(data) { - messageListData = data; + messageListData = data.reverse(); // Render list $('#message-list').loadTemplate($('#list-entry-template'), data); $('.message-list-entry').click(onMessageListClick); diff --git a/themes/bootstrap/public/monitor.js b/themes/bootstrap/public/monitor.js index e670621..3a03ed6 100644 --- a/themes/bootstrap/public/monitor.js +++ b/themes/bootstrap/public/monitor.js @@ -30,7 +30,7 @@ function startMonitor(mailbox) { $('#monitor-message-list').loadTemplate( $('#message-template'), msg, - { append: true }); + { prepend: true }); }); ws.addEventListener('close', function (e) { $('#conn-status').text('Disconnected!'); From cc47895d713b76abc3723710c5153edeb0f1bcd1 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 13:57:04 -0800 Subject: [PATCH 10/25] Pass cfg and ds as params, helps #26 #67 --- inbucket.go | 3 +-- pop3d/listener.go | 14 ++++---------- smtpd/listener.go | 6 +++--- 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/inbucket.go b/inbucket.go index 5d1c5ac..7593000 100644 --- a/inbucket.go +++ b/inbucket.go @@ -124,8 +124,7 @@ func main() { go httpd.Start(rootCtx) // Start POP3 server - // TODO pass datastore - pop3Server = pop3d.New(shutdownChan) + pop3Server = pop3d.New(config.GetPOP3Config(), shutdownChan, ds) go pop3Server.Start(rootCtx) // Startup SMTP server diff --git a/pop3d/listener.go b/pop3d/listener.go index 8536881..d1b11d5 100644 --- a/pop3d/listener.go +++ b/pop3d/listener.go @@ -14,6 +14,7 @@ import ( // Server defines an instance of our POP3 server type Server struct { + host string domain string maxIdleSeconds int dataStore smtpd.DataStore @@ -23,14 +24,9 @@ type Server struct { } // New creates a new Server struct -func New(shutdownChan chan bool) *Server { - // Get a new instance of the the FileDataStore - the locking and counting - // mechanisms are both global variables in the smtpd package. If that - // changes in the future, this should be modified to use the same DataStore - // instance. - ds := smtpd.DefaultFileDataStore() - cfg := config.GetPOP3Config() +func New(cfg config.POP3Config, shutdownChan chan bool, ds smtpd.DataStore) *Server { return &Server{ + host: fmt.Sprintf("%v:%v", cfg.IP4address, cfg.IP4port), domain: cfg.Domain, dataStore: ds, maxIdleSeconds: cfg.MaxIdleSeconds, @@ -41,9 +37,7 @@ func New(shutdownChan chan bool) *Server { // Start the server and listen for connections func (s *Server) Start(ctx context.Context) { - cfg := config.GetPOP3Config() - addr, err := net.ResolveTCPAddr("tcp4", fmt.Sprintf("%v:%v", - cfg.IP4address, cfg.IP4port)) + addr, err := net.ResolveTCPAddr("tcp4", s.host) if err != nil { log.Errorf("POP3 Failed to build tcp4 address: %v", err) s.emergencyShutdown() diff --git a/smtpd/listener.go b/smtpd/listener.go index 5fd4529..11c4632 100644 --- a/smtpd/listener.go +++ b/smtpd/listener.go @@ -18,6 +18,7 @@ import ( // Server holds the configuration and state of our SMTP server type Server struct { // Configuration + host string domain string domainNoStore string maxRecips int @@ -64,6 +65,7 @@ func NewServer( ds DataStore, msgHub *msghub.Hub) *Server { return &Server{ + host: fmt.Sprintf("%v:%v", cfg.IP4address, cfg.IP4port), domain: cfg.Domain, domainNoStore: strings.ToLower(cfg.DomainNoStore), maxRecips: cfg.MaxRecipients, @@ -80,9 +82,7 @@ func NewServer( // Start the listener and handle incoming connections func (s *Server) Start(ctx context.Context) { - cfg := config.GetSMTPConfig() - addr, err := net.ResolveTCPAddr("tcp4", fmt.Sprintf("%v:%v", - cfg.IP4address, cfg.IP4port)) + addr, err := net.ResolveTCPAddr("tcp4", s.host) if err != nil { log.Errorf("Failed to build tcp4 address: %v", err) s.emergencyShutdown() From 3a4fd3f09394e3fbe82eaece30415eac089df386 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 14:54:49 -0800 Subject: [PATCH 11/25] Refactor datastore into it's own package for #67 --- {smtpd => datastore}/datastore.go | 2 +- httpd/context.go | 4 ++-- httpd/server.go | 6 +++--- pop3d/handler.go | 30 +++++++++++++++--------------- pop3d/listener.go | 6 +++--- rest/apiv1_controller.go | 7 ++++--- rest/apiv1_controller_test.go | 10 +++++----- rest/testmocks_test.go | 22 +++++++++++----------- rest/testutils_test.go | 4 ++-- smtpd/filemsg.go | 5 +++-- smtpd/filestore.go | 19 ++++++++++--------- smtpd/handler.go | 3 ++- smtpd/handler_test.go | 3 ++- smtpd/listener.go | 11 ++++++----- smtpd/retention.go | 5 +++-- smtpd/retention_test.go | 29 +++++++++++++++-------------- webui/mailbox_controller.go | 11 ++++++----- 17 files changed, 93 insertions(+), 84 deletions(-) rename {smtpd => datastore}/datastore.go (98%) diff --git a/smtpd/datastore.go b/datastore/datastore.go similarity index 98% rename from smtpd/datastore.go rename to datastore/datastore.go index 180a3ec..dee5753 100644 --- a/smtpd/datastore.go +++ b/datastore/datastore.go @@ -1,4 +1,4 @@ -package smtpd +package datastore import ( "errors" diff --git a/httpd/context.go b/httpd/context.go index 52abfc6..6a54541 100644 --- a/httpd/context.go +++ b/httpd/context.go @@ -7,15 +7,15 @@ import ( "github.com/gorilla/mux" "github.com/gorilla/sessions" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/msghub" - "github.com/jhillyerd/inbucket/smtpd" ) // Context is passed into every request handler function type Context struct { Vars map[string]string Session *sessions.Session - DataStore smtpd.DataStore + DataStore datastore.DataStore MsgHub *msghub.Hub WebConfig config.WebConfig IsJSON bool diff --git a/httpd/server.go b/httpd/server.go index 103e0f1..f89b9ff 100644 --- a/httpd/server.go +++ b/httpd/server.go @@ -13,9 +13,9 @@ import ( "github.com/gorilla/securecookie" "github.com/gorilla/sessions" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/msghub" - "github.com/jhillyerd/inbucket/smtpd" ) // Handler is a function type that handles an HTTP request in Inbucket @@ -23,7 +23,7 @@ type Handler func(http.ResponseWriter, *http.Request, *Context) error var ( // DataStore is where all the mailboxes and messages live - DataStore smtpd.DataStore + DataStore datastore.DataStore // msgHub holds a reference to the message pub/sub system msgHub *msghub.Hub @@ -51,7 +51,7 @@ func init() { func Initialize( cfg config.WebConfig, shutdownChan chan bool, - ds smtpd.DataStore, + ds datastore.DataStore, mh *msghub.Hub) { webConfig = cfg diff --git a/pop3d/handler.go b/pop3d/handler.go index 782cf8e..7f5967e 100644 --- a/pop3d/handler.go +++ b/pop3d/handler.go @@ -11,8 +11,8 @@ import ( "strings" "time" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" - "github.com/jhillyerd/inbucket/smtpd" ) // State tracks the current mode of our POP3 state machine @@ -57,18 +57,18 @@ var commands = map[string]bool{ // Session defines an active POP3 session type Session struct { - server *Server // Reference to the server we belong to - id int // Session ID number - conn net.Conn // Our network connection - remoteHost string // IP address of client - sendError error // Used to bail out of read loop on send error - state State // Current session state - reader *bufio.Reader // Buffered reader for our net conn - user string // Mailbox name - mailbox smtpd.Mailbox // Mailbox instance - messages []smtpd.Message // Slice of messages in mailbox - retain []bool // Messages to retain upon UPDATE (true=retain) - msgCount int // Number of undeleted messages + server *Server // Reference to the server we belong to + id int // Session ID number + conn net.Conn // Our network connection + remoteHost string // IP address of client + sendError error // Used to bail out of read loop on send error + state State // Current session state + reader *bufio.Reader // Buffered reader for our net conn + user string // Mailbox name + mailbox datastore.Mailbox // Mailbox instance + messages []datastore.Message // Slice of messages in mailbox + retain []bool // Messages to retain upon UPDATE (true=retain) + msgCount int // Number of undeleted messages } // NewSession creates a new POP3 session @@ -432,7 +432,7 @@ func (ses *Session) transactionHandler(cmd string, args []string) { } // Send the contents of the message to the client -func (ses *Session) sendMessage(msg smtpd.Message) { +func (ses *Session) sendMessage(msg datastore.Message) { reader, err := msg.RawReader() if err != nil { ses.logError("Failed to read message for RETR command") @@ -465,7 +465,7 @@ func (ses *Session) sendMessage(msg smtpd.Message) { } // Send the headers plus the top N lines to the client -func (ses *Session) sendMessageTop(msg smtpd.Message, lineCount int) { +func (ses *Session) sendMessageTop(msg datastore.Message, lineCount int) { reader, err := msg.RawReader() if err != nil { ses.logError("Failed to read message for RETR command") diff --git a/pop3d/listener.go b/pop3d/listener.go index d1b11d5..58b0d82 100644 --- a/pop3d/listener.go +++ b/pop3d/listener.go @@ -8,8 +8,8 @@ import ( "time" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" - "github.com/jhillyerd/inbucket/smtpd" ) // Server defines an instance of our POP3 server @@ -17,14 +17,14 @@ type Server struct { host string domain string maxIdleSeconds int - dataStore smtpd.DataStore + dataStore datastore.DataStore listener net.Listener globalShutdown chan bool waitgroup *sync.WaitGroup } // New creates a new Server struct -func New(cfg config.POP3Config, shutdownChan chan bool, ds smtpd.DataStore) *Server { +func New(cfg config.POP3Config, shutdownChan chan bool, ds datastore.DataStore) *Server { return &Server{ host: fmt.Sprintf("%v:%v", cfg.IP4address, cfg.IP4port), domain: cfg.Domain, diff --git a/rest/apiv1_controller.go b/rest/apiv1_controller.go index 67a3528..b31b454 100644 --- a/rest/apiv1_controller.go +++ b/rest/apiv1_controller.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "strconv" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/httpd" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/rest/model" @@ -64,7 +65,7 @@ func MailboxShowV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } msg, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } @@ -149,7 +150,7 @@ func MailboxSourceV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Contex return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } message, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } @@ -183,7 +184,7 @@ func MailboxDeleteV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Contex return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } message, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } diff --git a/rest/apiv1_controller_test.go b/rest/apiv1_controller_test.go index 9297f88..a7aa445 100644 --- a/rest/apiv1_controller_test.go +++ b/rest/apiv1_controller_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/jhillyerd/inbucket/smtpd" + "github.com/jhillyerd/inbucket/datastore" ) const ( @@ -47,7 +47,7 @@ func TestRestMailboxList(t *testing.T) { // Test empty mailbox emptybox := &MockMailbox{} ds.On("MailboxFor", "empty").Return(emptybox, nil) - emptybox.On("GetMessages").Return([]smtpd.Message{}, nil) + emptybox.On("GetMessages").Return([]datastore.Message{}, nil) w, err = testRestGet(baseURL + "/mailbox/empty") expectCode = 200 @@ -79,7 +79,7 @@ func TestRestMailboxList(t *testing.T) { // Test MailboxFor error error2box := &MockMailbox{} ds.On("MailboxFor", "error2").Return(error2box, nil) - error2box.On("GetMessages").Return([]smtpd.Message{}, fmt.Errorf("Internal error 2")) + error2box.On("GetMessages").Return([]datastore.Message{}, fmt.Errorf("Internal error 2")) w, err = testRestGet(baseURL + "/mailbox/error2") expectCode = 500 @@ -111,7 +111,7 @@ func TestRestMailboxList(t *testing.T) { ds.On("MailboxFor", "good").Return(goodbox, nil) msg1 := data1.MockMessage() msg2 := data2.MockMessage() - goodbox.On("GetMessages").Return([]smtpd.Message{msg1, msg2}, nil) + goodbox.On("GetMessages").Return([]datastore.Message{msg1, msg2}, nil) // Check return code w, err = testRestGet(baseURL + "/mailbox/good") @@ -171,7 +171,7 @@ func TestRestMessage(t *testing.T) { // Test requesting a message that does not exist emptybox := &MockMailbox{} ds.On("MailboxFor", "empty").Return(emptybox, nil) - emptybox.On("GetMessage", "0001").Return(&MockMessage{}, smtpd.ErrNotExist) + emptybox.On("GetMessage", "0001").Return(&MockMessage{}, datastore.ErrNotExist) w, err = testRestGet(baseURL + "/mailbox/empty/0001") expectCode = 404 diff --git a/rest/testmocks_test.go b/rest/testmocks_test.go index bc720fc..0092714 100644 --- a/rest/testmocks_test.go +++ b/rest/testmocks_test.go @@ -6,7 +6,7 @@ import ( "time" "github.com/jhillyerd/enmime" - "github.com/jhillyerd/inbucket/smtpd" + "github.com/jhillyerd/inbucket/datastore" "github.com/stretchr/testify/mock" ) @@ -15,14 +15,14 @@ type MockDataStore struct { mock.Mock } -func (m *MockDataStore) MailboxFor(name string) (smtpd.Mailbox, error) { +func (m *MockDataStore) MailboxFor(name string) (datastore.Mailbox, error) { args := m.Called(name) - return args.Get(0).(smtpd.Mailbox), args.Error(1) + return args.Get(0).(datastore.Mailbox), args.Error(1) } -func (m *MockDataStore) AllMailboxes() ([]smtpd.Mailbox, error) { +func (m *MockDataStore) AllMailboxes() ([]datastore.Mailbox, error) { args := m.Called() - return args.Get(0).([]smtpd.Mailbox), args.Error(1) + return args.Get(0).([]datastore.Mailbox), args.Error(1) } // Mock Mailbox object @@ -30,14 +30,14 @@ type MockMailbox struct { mock.Mock } -func (m *MockMailbox) GetMessages() ([]smtpd.Message, error) { +func (m *MockMailbox) GetMessages() ([]datastore.Message, error) { args := m.Called() - return args.Get(0).([]smtpd.Message), args.Error(1) + return args.Get(0).([]datastore.Message), args.Error(1) } -func (m *MockMailbox) GetMessage(id string) (smtpd.Message, error) { +func (m *MockMailbox) GetMessage(id string) (datastore.Message, error) { args := m.Called(id) - return args.Get(0).(smtpd.Message), args.Error(1) + return args.Get(0).(datastore.Message), args.Error(1) } func (m *MockMailbox) Purge() error { @@ -45,9 +45,9 @@ func (m *MockMailbox) Purge() error { return args.Error(0) } -func (m *MockMailbox) NewMessage() (smtpd.Message, error) { +func (m *MockMailbox) NewMessage() (datastore.Message, error) { args := m.Called() - return args.Get(0).(smtpd.Message), args.Error(1) + return args.Get(0).(datastore.Message), args.Error(1) } func (m *MockMailbox) Name() string { diff --git a/rest/testutils_test.go b/rest/testutils_test.go index 7cfd223..d0e37d7 100644 --- a/rest/testutils_test.go +++ b/rest/testutils_test.go @@ -11,9 +11,9 @@ import ( "github.com/jhillyerd/enmime" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/httpd" "github.com/jhillyerd/inbucket/msghub" - "github.com/jhillyerd/inbucket/smtpd" ) type InputMessageData struct { @@ -188,7 +188,7 @@ func testRestGet(url string) (*httptest.ResponseRecorder, error) { return w, nil } -func setupWebServer(ds smtpd.DataStore) *bytes.Buffer { +func setupWebServer(ds datastore.DataStore) *bytes.Buffer { // Capture log output buf := new(bytes.Buffer) log.SetOutput(buf) diff --git a/smtpd/filemsg.go b/smtpd/filemsg.go index edf3ec9..308746a 100644 --- a/smtpd/filemsg.go +++ b/smtpd/filemsg.go @@ -11,6 +11,7 @@ import ( "time" "github.com/jhillyerd/enmime" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" ) @@ -33,7 +34,7 @@ type FileMessage struct { // NewMessage creates a new FileMessage object and sets the Date and Id fields. // It will also delete messages over messageCap if configured. -func (mb *FileMailbox) NewMessage() (Message, error) { +func (mb *FileMailbox) NewMessage() (datastore.Message, error) { // Load index if !mb.indexLoaded { if err := mb.readIndex(); err != nil { @@ -165,7 +166,7 @@ func (m *FileMessage) ReadRaw() (raw *string, err error) { func (m *FileMessage) Append(data []byte) error { // Prevent Appending to a pre-existing Message if !m.writable { - return ErrNotWritable + return datastore.ErrNotWritable } // Open file for writing if we haven't yet if m.writer == nil { diff --git a/smtpd/filestore.go b/smtpd/filestore.go index 9f8f615..4717e3a 100644 --- a/smtpd/filestore.go +++ b/smtpd/filestore.go @@ -12,6 +12,7 @@ import ( "time" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" ) @@ -55,7 +56,7 @@ type FileDataStore struct { } // NewFileDataStore creates a new DataStore object using the specified path -func NewFileDataStore(cfg config.DataStoreConfig) DataStore { +func NewFileDataStore(cfg config.DataStoreConfig) datastore.DataStore { path := cfg.Path if path == "" { log.Errorf("No value configured for datastore path") @@ -73,14 +74,14 @@ func NewFileDataStore(cfg config.DataStoreConfig) DataStore { // DefaultFileDataStore creates a new DataStore object. It uses the inbucket.Config object to // construct it's path. -func DefaultFileDataStore() DataStore { +func DefaultFileDataStore() datastore.DataStore { cfg := config.GetDataStoreConfig() return NewFileDataStore(cfg) } // MailboxFor retrieves the Mailbox object for a specified email address, if the mailbox // does not exist, it will attempt to create it. -func (ds *FileDataStore) MailboxFor(emailAddress string) (Mailbox, error) { +func (ds *FileDataStore) MailboxFor(emailAddress string) (datastore.Mailbox, error) { name, err := ParseMailboxName(emailAddress) if err != nil { return nil, err @@ -96,8 +97,8 @@ func (ds *FileDataStore) MailboxFor(emailAddress string) (Mailbox, error) { } // AllMailboxes returns a slice with all Mailboxes -func (ds *FileDataStore) AllMailboxes() ([]Mailbox, error) { - mailboxes := make([]Mailbox, 0, 100) +func (ds *FileDataStore) AllMailboxes() ([]datastore.Mailbox, error) { + mailboxes := make([]datastore.Mailbox, 0, 100) infos1, err := ioutil.ReadDir(ds.mailPath) if err != nil { return nil, err @@ -159,14 +160,14 @@ func (mb *FileMailbox) String() string { // GetMessages scans the mailbox directory for .gob files and decodes them into // a slice of Message objects. -func (mb *FileMailbox) GetMessages() ([]Message, error) { +func (mb *FileMailbox) GetMessages() ([]datastore.Message, error) { if !mb.indexLoaded { if err := mb.readIndex(); err != nil { return nil, err } } - messages := make([]Message, len(mb.messages)) + messages := make([]datastore.Message, len(mb.messages)) for i, m := range mb.messages { messages[i] = m } @@ -174,7 +175,7 @@ func (mb *FileMailbox) GetMessages() ([]Message, error) { } // GetMessage decodes a single message by Id and returns a Message object -func (mb *FileMailbox) GetMessage(id string) (Message, error) { +func (mb *FileMailbox) GetMessage(id string) (datastore.Message, error) { if !mb.indexLoaded { if err := mb.readIndex(); err != nil { return nil, err @@ -191,7 +192,7 @@ func (mb *FileMailbox) GetMessage(id string) (Message, error) { } } - return nil, ErrNotExist + return nil, datastore.ErrNotExist } // Purge deletes all messages in this mailbox diff --git a/smtpd/handler.go b/smtpd/handler.go index 7353194..a5dd34b 100644 --- a/smtpd/handler.go +++ b/smtpd/handler.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/msghub" ) @@ -71,7 +72,7 @@ var commands = map[string]bool{ // recipientDetails for message delivery type recipientDetails struct { address, localPart, domainPart string - mailbox Mailbox + mailbox datastore.Mailbox } // Session holds the state of an SMTP session diff --git a/smtpd/handler_test.go b/smtpd/handler_test.go index 61d4f69..633eecf 100644 --- a/smtpd/handler_test.go +++ b/smtpd/handler_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/msghub" ) @@ -376,7 +377,7 @@ func (m *mockConn) SetDeadline(t time.Time) error { return nil } func (m *mockConn) SetReadDeadline(t time.Time) error { return nil } func (m *mockConn) SetWriteDeadline(t time.Time) error { return nil } -func setupSMTPServer(ds DataStore) (s *Server, buf *bytes.Buffer, teardown func()) { +func setupSMTPServer(ds datastore.DataStore) (s *Server, buf *bytes.Buffer, teardown func()) { // Test Server Config cfg := config.SMTPConfig{ IP4address: net.IPv4(127, 0, 0, 1), diff --git a/smtpd/listener.go b/smtpd/listener.go index 11c4632..8c477a2 100644 --- a/smtpd/listener.go +++ b/smtpd/listener.go @@ -11,6 +11,7 @@ import ( "time" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/msghub" ) @@ -27,10 +28,10 @@ type Server struct { storeMessages bool // Dependencies - dataStore DataStore // Mailbox/message store - globalShutdown chan bool // Shuts down Inbucket - msgHub *msghub.Hub // Pub/sub for message info - retentionScanner *RetentionScanner // Deletes expired messages + dataStore datastore.DataStore // Mailbox/message store + globalShutdown chan bool // Shuts down Inbucket + msgHub *msghub.Hub // Pub/sub for message info + retentionScanner *RetentionScanner // Deletes expired messages // State listener net.Listener // Incoming network connections @@ -62,7 +63,7 @@ var ( func NewServer( cfg config.SMTPConfig, globalShutdown chan bool, - ds DataStore, + ds datastore.DataStore, msgHub *msghub.Hub) *Server { return &Server{ host: fmt.Sprintf("%v:%v", cfg.IP4address, cfg.IP4port), diff --git a/smtpd/retention.go b/smtpd/retention.go index 79c28b5..a562e5c 100644 --- a/smtpd/retention.go +++ b/smtpd/retention.go @@ -7,6 +7,7 @@ import ( "time" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" ) @@ -42,14 +43,14 @@ func init() { type RetentionScanner struct { globalShutdown chan bool // Closes when Inbucket needs to shut down retentionShutdown chan bool // Closed after the scanner has shut down - ds DataStore + ds datastore.DataStore retentionPeriod time.Duration retentionSleep time.Duration } // NewRetentionScanner launches a go-routine that scans for expired // messages, following the configured interval -func NewRetentionScanner(ds DataStore, shutdownChannel chan bool) *RetentionScanner { +func NewRetentionScanner(ds datastore.DataStore, shutdownChannel chan bool) *RetentionScanner { cfg := config.GetDataStoreConfig() rs := &RetentionScanner{ globalShutdown: shutdownChannel, diff --git a/smtpd/retention_test.go b/smtpd/retention_test.go index 5e76b42..9bd49c6 100644 --- a/smtpd/retention_test.go +++ b/smtpd/retention_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/jhillyerd/enmime" + "github.com/jhillyerd/inbucket/datastore" "github.com/stretchr/testify/mock" ) @@ -28,12 +29,12 @@ func TestDoRetentionScan(t *testing.T) { old3 := mockMessage(24) // First it should ask for all mailboxes - mds.On("AllMailboxes").Return([]Mailbox{mb1, mb2, mb3}, nil) + mds.On("AllMailboxes").Return([]datastore.Mailbox{mb1, mb2, mb3}, nil) // Then for all messages on each box - mb1.On("GetMessages").Return([]Message{new1, old1, old2}, nil) - mb2.On("GetMessages").Return([]Message{old3, new2}, nil) - mb3.On("GetMessages").Return([]Message{new3}, nil) + mb1.On("GetMessages").Return([]datastore.Message{new1, old1, old2}, nil) + mb2.On("GetMessages").Return([]datastore.Message{old3, new2}, nil) + mb3.On("GetMessages").Return([]datastore.Message{new3}, nil) // Test 4 hour retention rs := &RetentionScanner{ @@ -76,14 +77,14 @@ type MockDataStore struct { mock.Mock } -func (m *MockDataStore) MailboxFor(name string) (Mailbox, error) { +func (m *MockDataStore) MailboxFor(name string) (datastore.Mailbox, error) { args := m.Called() - return args.Get(0).(Mailbox), args.Error(1) + return args.Get(0).(datastore.Mailbox), args.Error(1) } -func (m *MockDataStore) AllMailboxes() ([]Mailbox, error) { +func (m *MockDataStore) AllMailboxes() ([]datastore.Mailbox, error) { args := m.Called() - return args.Get(0).([]Mailbox), args.Error(1) + return args.Get(0).([]datastore.Mailbox), args.Error(1) } // Mock Mailbox object @@ -91,14 +92,14 @@ type MockMailbox struct { mock.Mock } -func (m *MockMailbox) GetMessages() ([]Message, error) { +func (m *MockMailbox) GetMessages() ([]datastore.Message, error) { args := m.Called() - return args.Get(0).([]Message), args.Error(1) + return args.Get(0).([]datastore.Message), args.Error(1) } -func (m *MockMailbox) GetMessage(id string) (Message, error) { +func (m *MockMailbox) GetMessage(id string) (datastore.Message, error) { args := m.Called(id) - return args.Get(0).(Message), args.Error(1) + return args.Get(0).(datastore.Message), args.Error(1) } func (m *MockMailbox) Purge() error { @@ -106,9 +107,9 @@ func (m *MockMailbox) Purge() error { return args.Error(0) } -func (m *MockMailbox) NewMessage() (Message, error) { +func (m *MockMailbox) NewMessage() (datastore.Message, error) { args := m.Called() - return args.Get(0).(Message), args.Error(1) + return args.Get(0).(datastore.Message), args.Error(1) } func (m *MockMailbox) Name() string { diff --git a/webui/mailbox_controller.go b/webui/mailbox_controller.go index 35126db..d569106 100644 --- a/webui/mailbox_controller.go +++ b/webui/mailbox_controller.go @@ -7,6 +7,7 @@ import ( "net/http" "strconv" + "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/httpd" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/smtpd" @@ -103,7 +104,7 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } msg, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } @@ -143,7 +144,7 @@ func MailboxHTML(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } message, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } @@ -180,7 +181,7 @@ func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } message, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } @@ -226,7 +227,7 @@ func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *httpd. return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } message, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } @@ -279,7 +280,7 @@ func MailboxViewAttach(w http.ResponseWriter, req *http.Request, ctx *httpd.Cont return fmt.Errorf("Failed to get mailbox for %q: %v", name, err) } message, err := mb.GetMessage(id) - if err == smtpd.ErrNotExist { + if err == datastore.ErrNotExist { http.NotFound(w, req) return nil } From 11033a53592d3980fbe7d03c1efe7b546147ed8b Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 15:45:18 -0800 Subject: [PATCH 12/25] Move datastore mocks into correct package - Start of work for #48 - Continues #67 --- .../testmocks_test.go => datastore/testing.go | 23 +++++++++--------- rest/apiv1_controller_test.go | 24 +++++++++---------- rest/testutils_test.go | 4 ++-- 3 files changed, 25 insertions(+), 26 deletions(-) rename rest/testmocks_test.go => datastore/testing.go (75%) diff --git a/rest/testmocks_test.go b/datastore/testing.go similarity index 75% rename from rest/testmocks_test.go rename to datastore/testing.go index 0092714..5b891d8 100644 --- a/rest/testmocks_test.go +++ b/datastore/testing.go @@ -1,4 +1,4 @@ -package rest +package datastore import ( "io" @@ -6,7 +6,6 @@ import ( "time" "github.com/jhillyerd/enmime" - "github.com/jhillyerd/inbucket/datastore" "github.com/stretchr/testify/mock" ) @@ -15,14 +14,14 @@ type MockDataStore struct { mock.Mock } -func (m *MockDataStore) MailboxFor(name string) (datastore.Mailbox, error) { +func (m *MockDataStore) MailboxFor(name string) (Mailbox, error) { args := m.Called(name) - return args.Get(0).(datastore.Mailbox), args.Error(1) + return args.Get(0).(Mailbox), args.Error(1) } -func (m *MockDataStore) AllMailboxes() ([]datastore.Mailbox, error) { +func (m *MockDataStore) AllMailboxes() ([]Mailbox, error) { args := m.Called() - return args.Get(0).([]datastore.Mailbox), args.Error(1) + return args.Get(0).([]Mailbox), args.Error(1) } // Mock Mailbox object @@ -30,14 +29,14 @@ type MockMailbox struct { mock.Mock } -func (m *MockMailbox) GetMessages() ([]datastore.Message, error) { +func (m *MockMailbox) GetMessages() ([]Message, error) { args := m.Called() - return args.Get(0).([]datastore.Message), args.Error(1) + return args.Get(0).([]Message), args.Error(1) } -func (m *MockMailbox) GetMessage(id string) (datastore.Message, error) { +func (m *MockMailbox) GetMessage(id string) (Message, error) { args := m.Called(id) - return args.Get(0).(datastore.Message), args.Error(1) + return args.Get(0).(Message), args.Error(1) } func (m *MockMailbox) Purge() error { @@ -45,9 +44,9 @@ func (m *MockMailbox) Purge() error { return args.Error(0) } -func (m *MockMailbox) NewMessage() (datastore.Message, error) { +func (m *MockMailbox) NewMessage() (Message, error) { args := m.Called() - return args.Get(0).(datastore.Message), args.Error(1) + return args.Get(0).(Message), args.Error(1) } func (m *MockMailbox) Name() string { diff --git a/rest/apiv1_controller_test.go b/rest/apiv1_controller_test.go index a7aa445..b1c9a00 100644 --- a/rest/apiv1_controller_test.go +++ b/rest/apiv1_controller_test.go @@ -31,7 +31,7 @@ const ( func TestRestMailboxList(t *testing.T) { // Setup - ds := &MockDataStore{} + ds := &datastore.MockDataStore{} logbuf := setupWebServer(ds) // Test invalid mailbox name @@ -45,7 +45,7 @@ func TestRestMailboxList(t *testing.T) { } // Test empty mailbox - emptybox := &MockMailbox{} + emptybox := &datastore.MockMailbox{} ds.On("MailboxFor", "empty").Return(emptybox, nil) emptybox.On("GetMessages").Return([]datastore.Message{}, nil) @@ -59,7 +59,7 @@ func TestRestMailboxList(t *testing.T) { } // Test MailboxFor error - ds.On("MailboxFor", "error").Return(&MockMailbox{}, fmt.Errorf("Internal error")) + ds.On("MailboxFor", "error").Return(&datastore.MockMailbox{}, fmt.Errorf("Internal error")) w, err = testRestGet(baseURL + "/mailbox/error") expectCode = 500 if err != nil { @@ -77,7 +77,7 @@ func TestRestMailboxList(t *testing.T) { } // Test MailboxFor error - error2box := &MockMailbox{} + error2box := &datastore.MockMailbox{} ds.On("MailboxFor", "error2").Return(error2box, nil) error2box.On("GetMessages").Return([]datastore.Message{}, fmt.Errorf("Internal error 2")) @@ -107,7 +107,7 @@ func TestRestMailboxList(t *testing.T) { Subject: "subject 2", Date: time.Date(2012, 7, 1, 10, 11, 12, 253, time.FixedZone("PDT", -700)), } - goodbox := &MockMailbox{} + goodbox := &datastore.MockMailbox{} ds.On("MailboxFor", "good").Return(goodbox, nil) msg1 := data1.MockMessage() msg2 := data2.MockMessage() @@ -155,7 +155,7 @@ func TestRestMailboxList(t *testing.T) { func TestRestMessage(t *testing.T) { // Setup - ds := &MockDataStore{} + ds := &datastore.MockDataStore{} logbuf := setupWebServer(ds) // Test invalid mailbox name @@ -169,9 +169,9 @@ func TestRestMessage(t *testing.T) { } // Test requesting a message that does not exist - emptybox := &MockMailbox{} + emptybox := &datastore.MockMailbox{} ds.On("MailboxFor", "empty").Return(emptybox, nil) - emptybox.On("GetMessage", "0001").Return(&MockMessage{}, datastore.ErrNotExist) + emptybox.On("GetMessage", "0001").Return(&datastore.MockMessage{}, datastore.ErrNotExist) w, err = testRestGet(baseURL + "/mailbox/empty/0001") expectCode = 404 @@ -183,7 +183,7 @@ func TestRestMessage(t *testing.T) { } // Test MailboxFor error - ds.On("MailboxFor", "error").Return(&MockMailbox{}, fmt.Errorf("Internal error")) + ds.On("MailboxFor", "error").Return(&datastore.MockMailbox{}, fmt.Errorf("Internal error")) w, err = testRestGet(baseURL + "/mailbox/error/0001") expectCode = 500 if err != nil { @@ -201,9 +201,9 @@ func TestRestMessage(t *testing.T) { } // Test GetMessage error - error2box := &MockMailbox{} + error2box := &datastore.MockMailbox{} ds.On("MailboxFor", "error2").Return(error2box, nil) - error2box.On("GetMessage", "0001").Return(&MockMessage{}, fmt.Errorf("Internal error 2")) + error2box.On("GetMessage", "0001").Return(&datastore.MockMessage{}, fmt.Errorf("Internal error 2")) w, err = testRestGet(baseURL + "/mailbox/error2/0001") expectCode = 500 @@ -228,7 +228,7 @@ func TestRestMessage(t *testing.T) { Text: "This is some text", HTML: "This is some HTML", } - goodbox := &MockMailbox{} + goodbox := &datastore.MockMailbox{} ds.On("MailboxFor", "good").Return(goodbox, nil) msg1 := data1.MockMessage() goodbox.On("GetMessage", "0001").Return(msg1, nil) diff --git a/rest/testutils_test.go b/rest/testutils_test.go index d0e37d7..6ef4182 100644 --- a/rest/testutils_test.go +++ b/rest/testutils_test.go @@ -25,8 +25,8 @@ type InputMessageData struct { HTML, Text string } -func (d *InputMessageData) MockMessage() *MockMessage { - msg := &MockMessage{} +func (d *InputMessageData) MockMessage() *datastore.MockMessage { + msg := &datastore.MockMessage{} msg.On("ID").Return(d.ID) msg.On("From").Return(d.From) msg.On("To").Return(d.To) From dec67622babe776f59909457aca4777e98c3e5a7 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 16:34:48 -0800 Subject: [PATCH 13/25] Move handler tests to shared datastore mocks for #48 --- smtpd/handler_test.go | 24 ++++++++++-------------- smtpd/retention_test.go | 2 +- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/smtpd/handler_test.go b/smtpd/handler_test.go index 633eecf..2a65d14 100644 --- a/smtpd/handler_test.go +++ b/smtpd/handler_test.go @@ -26,9 +26,7 @@ type scriptStep struct { // Test commands in GREET state func TestGreetState(t *testing.T) { // Setup mock objects - mds := &MockDataStore{} - mb1 := &MockMailbox{} - mds.On("MailboxFor").Return(mb1, nil) + mds := &datastore.MockDataStore{} server, logbuf, teardown := setupSMTPServer(mds) defer teardown() @@ -87,9 +85,7 @@ func TestGreetState(t *testing.T) { // Test commands in READY state func TestReadyState(t *testing.T) { // Setup mock objects - mds := &MockDataStore{} - mb1 := &MockMailbox{} - mds.On("MailboxFor").Return(mb1, nil) + mds := &datastore.MockDataStore{} server, logbuf, teardown := setupSMTPServer(mds) defer teardown() @@ -152,10 +148,10 @@ func TestReadyState(t *testing.T) { // Test commands in MAIL state func TestMailState(t *testing.T) { // Setup mock objects - mds := &MockDataStore{} - mb1 := &MockMailbox{} - msg1 := &MockMessage{} - mds.On("MailboxFor").Return(mb1, nil) + mds := &datastore.MockDataStore{} + mb1 := &datastore.MockMailbox{} + msg1 := &datastore.MockMessage{} + mds.On("MailboxFor", "u1").Return(mb1, nil) mb1.On("NewMessage").Return(msg1, nil) mb1.On("Name").Return("u1") msg1.On("ID").Return("") @@ -269,10 +265,10 @@ func TestMailState(t *testing.T) { // Test commands in DATA state func TestDataState(t *testing.T) { // Setup mock objects - mds := &MockDataStore{} - mb1 := &MockMailbox{} - msg1 := &MockMessage{} - mds.On("MailboxFor").Return(mb1, nil) + mds := &datastore.MockDataStore{} + mb1 := &datastore.MockMailbox{} + msg1 := &datastore.MockMessage{} + mds.On("MailboxFor", "u1").Return(mb1, nil) mb1.On("NewMessage").Return(msg1, nil) mb1.On("Name").Return("u1") msg1.On("ID").Return("") diff --git a/smtpd/retention_test.go b/smtpd/retention_test.go index 9bd49c6..e553ac4 100644 --- a/smtpd/retention_test.go +++ b/smtpd/retention_test.go @@ -78,7 +78,7 @@ type MockDataStore struct { } func (m *MockDataStore) MailboxFor(name string) (datastore.Mailbox, error) { - args := m.Called() + args := m.Called(name) return args.Get(0).(datastore.Mailbox), args.Error(1) } From fcc0848bc0ca4703ae19b51c6cf5c194f40e5e27 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 18:15:38 -0800 Subject: [PATCH 14/25] Move metrics ticker to log pkg for #67 --- log/metrics.go | 62 ++++++++++++++++++++++++++++++++++++++++++++++ smtpd/listener.go | 63 +++++++++++++++++------------------------------ smtpd/utils.go | 13 ---------- 3 files changed, 84 insertions(+), 54 deletions(-) create mode 100644 log/metrics.go diff --git a/log/metrics.go b/log/metrics.go new file mode 100644 index 0000000..c16f1e8 --- /dev/null +++ b/log/metrics.go @@ -0,0 +1,62 @@ +package log + +import ( + "container/list" + "expvar" + "strings" + "time" +) + +// TickerFunc is the type of metrics function accepted by AddTickerFunc +type TickerFunc func() + +var tickerFuncChan = make(chan TickerFunc) + +func init() { + go metricsTicker() +} + +// AddTickerFunc adds a new function callback to the list of metrics TickerFuncs that get +// called each minute. +func AddTickerFunc(f TickerFunc) { + tickerFuncChan <- f +} + +// PushMetric adds the metric to the end of the list and returns a comma separated string of the +// previous 61 entries. We return 61 instead of 60 (an hour) because the chart on the client +// tracks deltas between these values - there is nothing to compare the first value against. +func PushMetric(history *list.List, ev expvar.Var) string { + history.PushBack(ev.String()) + if history.Len() > 61 { + history.Remove(history.Front()) + } + return joinStringList(history) +} + +// joinStringList joins a List containing strings by commas +func joinStringList(listOfStrings *list.List) string { + if listOfStrings.Len() == 0 { + return "" + } + s := make([]string, 0, listOfStrings.Len()) + for e := listOfStrings.Front(); e != nil; e = e.Next() { + s = append(s, e.Value.(string)) + } + return strings.Join(s, ",") +} + +func metricsTicker() { + funcs := make([]TickerFunc, 0) + ticker := time.NewTicker(time.Minute) + + for { + select { + case <-ticker.C: + for _, f := range funcs { + f() + } + case f := <-tickerFuncChan: + funcs = append(funcs, f) + } + } +} diff --git a/smtpd/listener.go b/smtpd/listener.go index 8c477a2..06bbf9c 100644 --- a/smtpd/listener.go +++ b/smtpd/listener.go @@ -16,6 +16,28 @@ import ( "github.com/jhillyerd/inbucket/msghub" ) +func init() { + m := expvar.NewMap("smtp") + m.Set("ConnectsTotal", expConnectsTotal) + m.Set("ConnectsHist", expConnectsHist) + m.Set("ConnectsCurrent", expConnectsCurrent) + m.Set("ReceivedTotal", expReceivedTotal) + m.Set("ReceivedHist", expReceivedHist) + m.Set("ErrorsTotal", expErrorsTotal) + m.Set("ErrorsHist", expErrorsHist) + m.Set("WarnsTotal", expWarnsTotal) + m.Set("WarnsHist", expWarnsHist) + + log.AddTickerFunc(func() { + expReceivedHist.Set(log.PushMetric(deliveredHist, expReceivedTotal)) + expConnectsHist.Set(log.PushMetric(connectsHist, expConnectsTotal)) + expErrorsHist.Set(log.PushMetric(errorsHist, expErrorsTotal)) + expWarnsHist.Set(log.PushMetric(warnsHist, expWarnsTotal)) + expRetentionDeletesHist.Set(log.PushMetric(retentionDeletesHist, expRetentionDeletesTotal)) + expRetainedHist.Set(log.PushMetric(retainedHist, expRetainedCurrent)) + }) +} + // Server holds the configuration and state of our SMTP server type Server struct { // Configuration @@ -179,44 +201,3 @@ func (s *Server) Drain() { log.Tracef("SMTP connections have drained") s.retentionScanner.Join() } - -// When the provided Ticker ticks, we update our metrics history -func metricsTicker(t *time.Ticker) { - ok := true - for ok { - _, ok = <-t.C - expReceivedHist.Set(pushMetric(deliveredHist, expReceivedTotal)) - expConnectsHist.Set(pushMetric(connectsHist, expConnectsTotal)) - expErrorsHist.Set(pushMetric(errorsHist, expErrorsTotal)) - expWarnsHist.Set(pushMetric(warnsHist, expWarnsTotal)) - expRetentionDeletesHist.Set(pushMetric(retentionDeletesHist, expRetentionDeletesTotal)) - expRetainedHist.Set(pushMetric(retainedHist, expRetainedCurrent)) - } -} - -// pushMetric adds the metric to the end of the list and returns a comma separated string of the -// previous 61 entries. We return 61 instead of 60 (an hour) because the chart on the client -// tracks deltas between these values - there is nothing to compare the first value against. -func pushMetric(history *list.List, ev expvar.Var) string { - history.PushBack(ev.String()) - if history.Len() > 61 { - history.Remove(history.Front()) - } - return JoinStringList(history) -} - -func init() { - m := expvar.NewMap("smtp") - m.Set("ConnectsTotal", expConnectsTotal) - m.Set("ConnectsHist", expConnectsHist) - m.Set("ConnectsCurrent", expConnectsCurrent) - m.Set("ReceivedTotal", expReceivedTotal) - m.Set("ReceivedHist", expReceivedHist) - m.Set("ErrorsTotal", expErrorsTotal) - m.Set("ErrorsHist", expErrorsHist) - m.Set("WarnsTotal", expWarnsTotal) - m.Set("WarnsHist", expWarnsHist) - - t := time.NewTicker(time.Minute) - go metricsTicker(t) -} diff --git a/smtpd/utils.go b/smtpd/utils.go index a8dcfb7..bd57e3c 100644 --- a/smtpd/utils.go +++ b/smtpd/utils.go @@ -2,7 +2,6 @@ package smtpd import ( "bytes" - "container/list" "crypto/sha1" "fmt" "io" @@ -53,18 +52,6 @@ func HashMailboxName(mailbox string) string { return fmt.Sprintf("%x", h.Sum(nil)) } -// JoinStringList joins a List containing strings by commas -func JoinStringList(listOfStrings *list.List) string { - if listOfStrings.Len() == 0 { - return "" - } - s := make([]string, 0, listOfStrings.Len()) - for e := listOfStrings.Front(); e != nil; e = e.Next() { - s = append(s, e.Value.(string)) - } - return strings.Join(s, ",") -} - // ValidateDomainPart returns true if the domain part complies to RFC3696, RFC1035 func ValidateDomainPart(domain string) bool { if len(domain) == 0 { From f62eaa31b97c1e7c0c901ad2890d02d45d9ae655 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 18:33:56 -0800 Subject: [PATCH 15/25] Move retention scanner into datastore pkg for #67 --- {smtpd => datastore}/retention.go | 12 +- datastore/retention_test.go | 67 ++++++++++ smtpd/listener.go | 12 +- smtpd/retention_test.go | 198 ------------------------------ 4 files changed, 80 insertions(+), 209 deletions(-) rename {smtpd => datastore}/retention.go (93%) create mode 100644 datastore/retention_test.go delete mode 100644 smtpd/retention_test.go diff --git a/smtpd/retention.go b/datastore/retention.go similarity index 93% rename from smtpd/retention.go rename to datastore/retention.go index a562e5c..b5ede4d 100644 --- a/smtpd/retention.go +++ b/datastore/retention.go @@ -1,4 +1,4 @@ -package smtpd +package datastore import ( "container/list" @@ -7,7 +7,6 @@ import ( "time" "github.com/jhillyerd/inbucket/config" - "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" ) @@ -37,20 +36,25 @@ func init() { rm.Set("Period", expRetentionPeriod) rm.Set("RetainedHist", expRetainedHist) rm.Set("RetainedCurrent", expRetainedCurrent) + + log.AddTickerFunc(func() { + expRetentionDeletesHist.Set(log.PushMetric(retentionDeletesHist, expRetentionDeletesTotal)) + expRetainedHist.Set(log.PushMetric(retainedHist, expRetainedCurrent)) + }) } // RetentionScanner looks for messages older than the configured retention period and deletes them. type RetentionScanner struct { globalShutdown chan bool // Closes when Inbucket needs to shut down retentionShutdown chan bool // Closed after the scanner has shut down - ds datastore.DataStore + ds DataStore retentionPeriod time.Duration retentionSleep time.Duration } // NewRetentionScanner launches a go-routine that scans for expired // messages, following the configured interval -func NewRetentionScanner(ds datastore.DataStore, shutdownChannel chan bool) *RetentionScanner { +func NewRetentionScanner(ds DataStore, shutdownChannel chan bool) *RetentionScanner { cfg := config.GetDataStoreConfig() rs := &RetentionScanner{ globalShutdown: shutdownChannel, diff --git a/datastore/retention_test.go b/datastore/retention_test.go new file mode 100644 index 0000000..c357f7e --- /dev/null +++ b/datastore/retention_test.go @@ -0,0 +1,67 @@ +package datastore + +import ( + "fmt" + "testing" + "time" +) + +func TestDoRetentionScan(t *testing.T) { + // Create mock objects + mds := &MockDataStore{} + + mb1 := &MockMailbox{} + mb2 := &MockMailbox{} + mb3 := &MockMailbox{} + + // Mockup some different aged messages (num is in hours) + new1 := mockMessage(0) + new2 := mockMessage(1) + new3 := mockMessage(2) + old1 := mockMessage(4) + old2 := mockMessage(12) + old3 := mockMessage(24) + + // First it should ask for all mailboxes + mds.On("AllMailboxes").Return([]Mailbox{mb1, mb2, mb3}, nil) + + // Then for all messages on each box + mb1.On("GetMessages").Return([]Message{new1, old1, old2}, nil) + mb2.On("GetMessages").Return([]Message{old3, new2}, nil) + mb3.On("GetMessages").Return([]Message{new3}, nil) + + // Test 4 hour retention + rs := &RetentionScanner{ + ds: mds, + retentionPeriod: 4*time.Hour - time.Minute, + retentionSleep: 0, + } + if err := rs.doScan(); err != nil { + t.Error(err) + } + + // Check our assertions + mds.AssertExpectations(t) + mb1.AssertExpectations(t) + mb2.AssertExpectations(t) + mb3.AssertExpectations(t) + + // Delete should not have been called on new messages + new1.AssertNotCalled(t, "Delete") + new2.AssertNotCalled(t, "Delete") + new3.AssertNotCalled(t, "Delete") + + // Delete should have been called once on old messages + old1.AssertNumberOfCalls(t, "Delete", 1) + old2.AssertNumberOfCalls(t, "Delete", 1) + old3.AssertNumberOfCalls(t, "Delete", 1) +} + +// Make a MockMessage of a specific age +func mockMessage(ageHours int) *MockMessage { + msg := &MockMessage{} + msg.On("ID").Return(fmt.Sprintf("MSG[age=%vh]", ageHours)) + msg.On("Date").Return(time.Now().Add(time.Duration(ageHours*-1) * time.Hour)) + msg.On("Delete").Return(nil) + return msg +} diff --git a/smtpd/listener.go b/smtpd/listener.go index 06bbf9c..6b4634e 100644 --- a/smtpd/listener.go +++ b/smtpd/listener.go @@ -33,8 +33,6 @@ func init() { expConnectsHist.Set(log.PushMetric(connectsHist, expConnectsTotal)) expErrorsHist.Set(log.PushMetric(errorsHist, expErrorsTotal)) expWarnsHist.Set(log.PushMetric(warnsHist, expWarnsTotal)) - expRetentionDeletesHist.Set(log.PushMetric(retentionDeletesHist, expRetentionDeletesTotal)) - expRetainedHist.Set(log.PushMetric(retainedHist, expRetainedCurrent)) }) } @@ -50,10 +48,10 @@ type Server struct { storeMessages bool // Dependencies - dataStore datastore.DataStore // Mailbox/message store - globalShutdown chan bool // Shuts down Inbucket - msgHub *msghub.Hub // Pub/sub for message info - retentionScanner *RetentionScanner // Deletes expired messages + dataStore datastore.DataStore // Mailbox/message store + globalShutdown chan bool // Shuts down Inbucket + msgHub *msghub.Hub // Pub/sub for message info + retentionScanner *datastore.RetentionScanner // Deletes expired messages // State listener net.Listener // Incoming network connections @@ -98,7 +96,7 @@ func NewServer( globalShutdown: globalShutdown, dataStore: ds, msgHub: msgHub, - retentionScanner: NewRetentionScanner(ds, globalShutdown), + retentionScanner: datastore.NewRetentionScanner(ds, globalShutdown), waitgroup: new(sync.WaitGroup), } } diff --git a/smtpd/retention_test.go b/smtpd/retention_test.go deleted file mode 100644 index e553ac4..0000000 --- a/smtpd/retention_test.go +++ /dev/null @@ -1,198 +0,0 @@ -package smtpd - -import ( - "fmt" - "io" - "net/mail" - "testing" - "time" - - "github.com/jhillyerd/enmime" - "github.com/jhillyerd/inbucket/datastore" - "github.com/stretchr/testify/mock" -) - -func TestDoRetentionScan(t *testing.T) { - // Create mock objects - mds := &MockDataStore{} - - mb1 := &MockMailbox{} - mb2 := &MockMailbox{} - mb3 := &MockMailbox{} - - // Mockup some different aged messages (num is in hours) - new1 := mockMessage(0) - new2 := mockMessage(1) - new3 := mockMessage(2) - old1 := mockMessage(4) - old2 := mockMessage(12) - old3 := mockMessage(24) - - // First it should ask for all mailboxes - mds.On("AllMailboxes").Return([]datastore.Mailbox{mb1, mb2, mb3}, nil) - - // Then for all messages on each box - mb1.On("GetMessages").Return([]datastore.Message{new1, old1, old2}, nil) - mb2.On("GetMessages").Return([]datastore.Message{old3, new2}, nil) - mb3.On("GetMessages").Return([]datastore.Message{new3}, nil) - - // Test 4 hour retention - rs := &RetentionScanner{ - ds: mds, - retentionPeriod: 4*time.Hour - time.Minute, - retentionSleep: 0, - } - if err := rs.doScan(); err != nil { - t.Error(err) - } - - // Check our assertions - mds.AssertExpectations(t) - mb1.AssertExpectations(t) - mb2.AssertExpectations(t) - mb3.AssertExpectations(t) - - // Delete should not have been called on new messages - new1.AssertNotCalled(t, "Delete") - new2.AssertNotCalled(t, "Delete") - new3.AssertNotCalled(t, "Delete") - - // Delete should have been called once on old messages - old1.AssertNumberOfCalls(t, "Delete", 1) - old2.AssertNumberOfCalls(t, "Delete", 1) - old3.AssertNumberOfCalls(t, "Delete", 1) -} - -// Make a MockMessage of a specific age -func mockMessage(ageHours int) *MockMessage { - msg := &MockMessage{} - msg.On("ID").Return(fmt.Sprintf("MSG[age=%vh]", ageHours)) - msg.On("Date").Return(time.Now().Add(time.Duration(ageHours*-1) * time.Hour)) - msg.On("Delete").Return(nil) - return msg -} - -// Mock DataStore object -type MockDataStore struct { - mock.Mock -} - -func (m *MockDataStore) MailboxFor(name string) (datastore.Mailbox, error) { - args := m.Called(name) - return args.Get(0).(datastore.Mailbox), args.Error(1) -} - -func (m *MockDataStore) AllMailboxes() ([]datastore.Mailbox, error) { - args := m.Called() - return args.Get(0).([]datastore.Mailbox), args.Error(1) -} - -// Mock Mailbox object -type MockMailbox struct { - mock.Mock -} - -func (m *MockMailbox) GetMessages() ([]datastore.Message, error) { - args := m.Called() - return args.Get(0).([]datastore.Message), args.Error(1) -} - -func (m *MockMailbox) GetMessage(id string) (datastore.Message, error) { - args := m.Called(id) - return args.Get(0).(datastore.Message), args.Error(1) -} - -func (m *MockMailbox) Purge() error { - args := m.Called() - return args.Error(0) -} - -func (m *MockMailbox) NewMessage() (datastore.Message, error) { - args := m.Called() - return args.Get(0).(datastore.Message), args.Error(1) -} - -func (m *MockMailbox) Name() string { - args := m.Called() - return args.String(0) -} - -func (m *MockMailbox) String() string { - args := m.Called() - return args.String(0) -} - -// Mock Message object -type MockMessage struct { - mock.Mock -} - -func (m *MockMessage) ID() string { - args := m.Called() - return args.String(0) -} - -func (m *MockMessage) From() string { - args := m.Called() - return args.String(0) -} - -func (m *MockMessage) To() []string { - args := m.Called() - return args.Get(0).([]string) -} - -func (m *MockMessage) Date() time.Time { - args := m.Called() - return args.Get(0).(time.Time) -} - -func (m *MockMessage) Subject() string { - args := m.Called() - return args.String(0) -} - -func (m *MockMessage) ReadHeader() (msg *mail.Message, err error) { - args := m.Called() - return args.Get(0).(*mail.Message), args.Error(1) -} - -func (m *MockMessage) ReadBody() (body *enmime.Envelope, err error) { - args := m.Called() - return args.Get(0).(*enmime.Envelope), args.Error(1) -} - -func (m *MockMessage) ReadRaw() (raw *string, err error) { - args := m.Called() - return args.Get(0).(*string), args.Error(1) -} - -func (m *MockMessage) RawReader() (reader io.ReadCloser, err error) { - args := m.Called() - return args.Get(0).(io.ReadCloser), args.Error(1) -} - -func (m *MockMessage) Size() int64 { - args := m.Called() - return int64(args.Int(0)) -} - -func (m *MockMessage) Append(data []byte) error { - // []byte arg seems to mess up testify/mock - return nil -} - -func (m *MockMessage) Close() error { - args := m.Called() - return args.Error(0) -} - -func (m *MockMessage) Delete() error { - args := m.Called() - return args.Error(0) -} - -func (m *MockMessage) String() string { - args := m.Called() - return args.String(0) -} From ac21675bd75280f6e0742a198c6d145b34ed57aa Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 18:54:02 -0800 Subject: [PATCH 16/25] Clean up datastore related linter findings --- datastore/datastore.go | 1 + datastore/retention.go | 10 ++++------ datastore/testing.go | 28 +++++++++++++++++++++++++--- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/datastore/datastore.go b/datastore/datastore.go index dee5753..67aebf6 100644 --- a/datastore/datastore.go +++ b/datastore/datastore.go @@ -1,3 +1,4 @@ +// Package datastore contains implementation independent datastore logic package datastore import ( diff --git a/datastore/retention.go b/datastore/retention.go index b5ede4d..7d52c27 100644 --- a/datastore/retention.go +++ b/datastore/retention.go @@ -90,9 +90,9 @@ retentionLoop: dur := time.Minute - since log.Tracef("Retention scanner sleeping for %v", dur) select { - case _ = <-rs.globalShutdown: + case <-rs.globalShutdown: break retentionLoop - case _ = <-time.After(dur): + case <-time.After(dur): } } // Kickoff scan @@ -102,7 +102,7 @@ retentionLoop: } // Check for global shutdown select { - case _ = <-rs.globalShutdown: + case <-rs.globalShutdown: break retentionLoop default: } @@ -159,9 +159,7 @@ func (rs *RetentionScanner) doScan() error { // Join does not retun until the retention scanner has shut down func (rs *RetentionScanner) Join() { if rs.retentionShutdown != nil { - select { - case <-rs.retentionShutdown: - } + <-rs.retentionShutdown } } diff --git a/datastore/testing.go b/datastore/testing.go index 5b891d8..23474f6 100644 --- a/datastore/testing.go +++ b/datastore/testing.go @@ -9,126 +9,148 @@ import ( "github.com/stretchr/testify/mock" ) -// Mock DataStore object +// MockDataStore is a shared mock for unit testing type MockDataStore struct { mock.Mock } +// MailboxFor mock function func (m *MockDataStore) MailboxFor(name string) (Mailbox, error) { args := m.Called(name) return args.Get(0).(Mailbox), args.Error(1) } +// AllMailboxes mock function func (m *MockDataStore) AllMailboxes() ([]Mailbox, error) { args := m.Called() return args.Get(0).([]Mailbox), args.Error(1) } -// Mock Mailbox object +// MockMailbox is a shared mock for unit testing type MockMailbox struct { mock.Mock } +// GetMessages mock function func (m *MockMailbox) GetMessages() ([]Message, error) { args := m.Called() return args.Get(0).([]Message), args.Error(1) } +// GetMessage mock function func (m *MockMailbox) GetMessage(id string) (Message, error) { args := m.Called(id) return args.Get(0).(Message), args.Error(1) } +// Purge mock function func (m *MockMailbox) Purge() error { args := m.Called() return args.Error(0) } +// NewMessage mock function func (m *MockMailbox) NewMessage() (Message, error) { args := m.Called() return args.Get(0).(Message), args.Error(1) } +// Name mock function func (m *MockMailbox) Name() string { args := m.Called() return args.String(0) } +// String mock function func (m *MockMailbox) String() string { args := m.Called() return args.String(0) } -// Mock Message object +// MockMessage is a shared mock for unit testing type MockMessage struct { mock.Mock } +// ID mock function func (m *MockMessage) ID() string { args := m.Called() return args.String(0) } +// From mock function func (m *MockMessage) From() string { args := m.Called() return args.String(0) } +// To mock function func (m *MockMessage) To() []string { args := m.Called() return args.Get(0).([]string) } +// Date mock function func (m *MockMessage) Date() time.Time { args := m.Called() return args.Get(0).(time.Time) } +// Subject mock function func (m *MockMessage) Subject() string { args := m.Called() return args.String(0) } +// ReadHeader mock function func (m *MockMessage) ReadHeader() (msg *mail.Message, err error) { args := m.Called() return args.Get(0).(*mail.Message), args.Error(1) } +// ReadBody mock function func (m *MockMessage) ReadBody() (body *enmime.Envelope, err error) { args := m.Called() return args.Get(0).(*enmime.Envelope), args.Error(1) } +// ReadRaw mock function func (m *MockMessage) ReadRaw() (raw *string, err error) { args := m.Called() return args.Get(0).(*string), args.Error(1) } +// RawReader mock function func (m *MockMessage) RawReader() (reader io.ReadCloser, err error) { args := m.Called() return args.Get(0).(io.ReadCloser), args.Error(1) } +// Size mock function func (m *MockMessage) Size() int64 { args := m.Called() return int64(args.Int(0)) } +// Append mock function func (m *MockMessage) Append(data []byte) error { // []byte arg seems to mess up testify/mock return nil } +// Close mock function func (m *MockMessage) Close() error { args := m.Called() return args.Error(0) } +// Delete mock function func (m *MockMessage) Delete() error { args := m.Called() return args.Error(0) } +// String mock function func (m *MockMessage) String() string { args := m.Called() return args.String(0) From 06165cb3d3d20420d1dacca265a8130b0f5c45dc Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 22:16:47 -0800 Subject: [PATCH 17/25] Many linter fixes for smtpd pkg --- smtpd/filemsg.go | 2 +- smtpd/filestore.go | 12 +++++++----- smtpd/filestore_test.go | 2 +- smtpd/handler.go | 18 +++++++----------- smtpd/handler_test.go | 12 +++--------- smtpd/listener.go | 8 +++----- smtpd/utils.go | 35 ++++++++++++++++++++++++++++------- 7 files changed, 50 insertions(+), 39 deletions(-) diff --git a/smtpd/filemsg.go b/smtpd/filemsg.go index 308746a..9030dbe 100644 --- a/smtpd/filemsg.go +++ b/smtpd/filemsg.go @@ -72,7 +72,7 @@ func (m *FileMessage) From() string { return m.Ffrom } -// From returns the value of the Message To header +// To returns the value of the Message To header func (m *FileMessage) To() []string { return m.Fto } diff --git a/smtpd/filestore.go b/smtpd/filestore.go index 4717e3a..900c480 100644 --- a/smtpd/filestore.go +++ b/smtpd/filestore.go @@ -150,10 +150,12 @@ type FileMailbox struct { messages []*FileMessage } +// Name of the mailbox func (mb *FileMailbox) Name() string { return mb.name } +// String renders the name and directory path of the mailbox func (mb *FileMailbox) String() string { return mb.name + "[" + mb.dirName + "]" } @@ -184,11 +186,11 @@ func (mb *FileMailbox) GetMessage(id string) (datastore.Message, error) { if id == "latest" && len(mb.messages) != 0 { return mb.messages[len(mb.messages)-1], nil - } else { - for _, m := range mb.messages { - if m.Fid == id { - return m, nil - } + } + + for _, m := range mb.messages { + if m.Fid == id { + return m, nil } } diff --git a/smtpd/filestore_test.go b/smtpd/filestore_test.go index 2ca3104..a27e5ca 100644 --- a/smtpd/filestore_test.go +++ b/smtpd/filestore_test.go @@ -496,7 +496,7 @@ func TestGetLatestMessage(t *testing.T) { assert.True(t, msg.ID() == id3, "Expected %q to be equal to %q", msg.ID(), id3) // Test wrong id - msg, err = mb.GetMessage("wrongid") + _, err = mb.GetMessage("wrongid") assert.Error(t, err) if t.Failed() { diff --git a/smtpd/handler.go b/smtpd/handler.go index a5dd34b..fdb0177 100644 --- a/smtpd/handler.go +++ b/smtpd/handler.go @@ -511,20 +511,16 @@ func (ss *Session) send(msg string) { // readByteLine reads a line of input into the provided buffer. Does // not reset the Buffer - please do so prior to calling. -func (ss *Session) readByteLine(buf *bytes.Buffer) error { +func (ss *Session) readByteLine(buf io.Writer) error { if err := ss.conn.SetReadDeadline(ss.nextDeadline()); err != nil { return err } - for { - line, err := ss.reader.ReadBytes('\n') - if err != nil { - return err - } - if _, err = buf.Write(line); err != nil { - return err - } - return nil + line, err := ss.reader.ReadBytes('\n') + if err != nil { + return err } + _, err = buf.Write(line) + return err } // Reads a line of input @@ -573,7 +569,7 @@ func (ss *Session) parseCmd(line string) (cmd string, arg string, ok bool) { // The leading space is mandatory. func (ss *Session) parseArgs(arg string) (args map[string]string, ok bool) { args = make(map[string]string) - re := regexp.MustCompile(" (\\w+)=(\\w+)") + re := regexp.MustCompile(` (\w+)=(\w+)`) pm := re.FindAllStringSubmatch(arg, -1) if pm == nil { ss.logWarn("Failed to parse arg string: %q") diff --git a/smtpd/handler_test.go b/smtpd/handler_test.go index 2a65d14..e541f4e 100644 --- a/smtpd/handler_test.go +++ b/smtpd/handler_test.go @@ -31,10 +31,8 @@ func TestGreetState(t *testing.T) { server, logbuf, teardown := setupSMTPServer(mds) defer teardown() - var script []scriptStep - // Test out some mangled HELOs - script = []scriptStep{ + script := []scriptStep{ {"HELO", 501}, {"EHLO", 501}, {"HELLO", 500}, @@ -90,10 +88,8 @@ func TestReadyState(t *testing.T) { server, logbuf, teardown := setupSMTPServer(mds) defer teardown() - var script []scriptStep - // Test out some mangled READY commands - script = []scriptStep{ + script := []scriptStep{ {"HELO localhost", 250}, {"FOOB", 500}, {"HELO", 503}, @@ -165,10 +161,8 @@ func TestMailState(t *testing.T) { server, logbuf, teardown := setupSMTPServer(mds) defer teardown() - var script []scriptStep - // Test out some mangled READY commands - script = []scriptStep{ + script := []scriptStep{ {"HELO localhost", 250}, {"MAIL FROM:", 250}, {"FOOB", 500}, diff --git a/smtpd/listener.go b/smtpd/listener.go index 6b4634e..60953b6 100644 --- a/smtpd/listener.go +++ b/smtpd/listener.go @@ -131,10 +131,8 @@ func (s *Server) Start(ctx context.Context) { go s.serve(ctx) // Wait for shutdown - select { - case <-ctx.Done(): - log.Tracef("SMTP shutdown requested, connections will be drained") - } + <-ctx.Done() + log.Tracef("SMTP shutdown requested, connections will be drained") // Closing the listener will cause the serve() go routine to exit if err := s.listener.Close(); err != nil { @@ -186,7 +184,7 @@ func (s *Server) serve(ctx context.Context) { func (s *Server) emergencyShutdown() { // Shutdown Inbucket select { - case _ = <-s.globalShutdown: + case <-s.globalShutdown: default: close(s.globalShutdown) } diff --git a/smtpd/utils.go b/smtpd/utils.go index bd57e3c..9b3eb17 100644 --- a/smtpd/utils.go +++ b/smtpd/utils.go @@ -130,15 +130,24 @@ LOOP: switch { case ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'): // Letters are OK - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case '0' <= c && c <= '9': // Numbers are OK - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case bytes.IndexByte([]byte("!#$%&'*+-/=?^_`{|}~"), c) >= 0: // These specials can be used unquoted - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case c == '.': // A single period is OK @@ -146,13 +155,19 @@ LOOP: // Sequence of periods is not permitted return "", "", fmt.Errorf("Sequence of periods is not permitted") } - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case c == '\\': inCharQuote = true case c == '"': if inCharQuote { - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false } else if inStringQuote { inStringQuote = false @@ -165,7 +180,10 @@ LOOP: } case c == '@': if inCharQuote || inStringQuote { - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false } else { // End of local-part @@ -182,7 +200,10 @@ LOOP: return "", "", fmt.Errorf("Characters outside of US-ASCII range not permitted") default: if inCharQuote || inStringQuote { - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false } else { return "", "", fmt.Errorf("Character %q must be quoted", c) From 25815767a728f868de390ea894328011b17ab6ca Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 22:55:20 -0800 Subject: [PATCH 18/25] Move smtpd/utils.go into dedicated stringutil pkg --- rest/apiv1_controller.go | 12 ++++++------ rest/socketv1_controller.go | 4 ++-- smtpd/filestore.go | 5 +++-- smtpd/handler.go | 7 ++++--- {smtpd => stringutil}/utils.go | 4 ++-- {smtpd => stringutil}/utils_test.go | 2 +- webui/mailbox_controller.go | 18 +++++++++--------- webui/root_controller.go | 4 ++-- 8 files changed, 29 insertions(+), 27 deletions(-) rename {smtpd => stringutil}/utils.go (98%) rename {smtpd => stringutil}/utils_test.go (99%) diff --git a/rest/apiv1_controller.go b/rest/apiv1_controller.go index b31b454..9037eb1 100644 --- a/rest/apiv1_controller.go +++ b/rest/apiv1_controller.go @@ -14,13 +14,13 @@ import ( "github.com/jhillyerd/inbucket/httpd" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/rest/model" - "github.com/jhillyerd/inbucket/smtpd" + "github.com/jhillyerd/inbucket/stringutil" ) // MailboxListV1 renders a list of messages in a mailbox func MailboxListV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -55,7 +55,7 @@ func MailboxListV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) func MailboxShowV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -117,7 +117,7 @@ func MailboxShowV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) // MailboxPurgeV1 deletes all messages from a mailbox func MailboxPurgeV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -140,7 +140,7 @@ func MailboxPurgeV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context func MailboxSourceV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -174,7 +174,7 @@ func MailboxSourceV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Contex func MailboxDeleteV1(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } diff --git a/rest/socketv1_controller.go b/rest/socketv1_controller.go index 764519a..78bec2d 100644 --- a/rest/socketv1_controller.go +++ b/rest/socketv1_controller.go @@ -9,7 +9,7 @@ import ( "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/msghub" "github.com/jhillyerd/inbucket/rest/model" - "github.com/jhillyerd/inbucket/smtpd" + "github.com/jhillyerd/inbucket/stringutil" ) const ( @@ -169,7 +169,7 @@ func MonitorAllMessagesV1( func MonitorMailboxMessagesV1( w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } diff --git a/smtpd/filestore.go b/smtpd/filestore.go index 900c480..4bef294 100644 --- a/smtpd/filestore.go +++ b/smtpd/filestore.go @@ -14,6 +14,7 @@ import ( "github.com/jhillyerd/inbucket/config" "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" + "github.com/jhillyerd/inbucket/stringutil" ) // Name of index file in each mailbox @@ -82,11 +83,11 @@ func DefaultFileDataStore() datastore.DataStore { // MailboxFor retrieves the Mailbox object for a specified email address, if the mailbox // does not exist, it will attempt to create it. func (ds *FileDataStore) MailboxFor(emailAddress string) (datastore.Mailbox, error) { - name, err := ParseMailboxName(emailAddress) + name, err := stringutil.ParseMailboxName(emailAddress) if err != nil { return nil, err } - dir := HashMailboxName(name) + dir := stringutil.HashMailboxName(name) s1 := dir[0:3] s2 := dir[0:6] path := filepath.Join(ds.mailPath, s1, s2, dir) diff --git a/smtpd/handler.go b/smtpd/handler.go index fdb0177..f6c4397 100644 --- a/smtpd/handler.go +++ b/smtpd/handler.go @@ -15,6 +15,7 @@ import ( "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/msghub" + "github.com/jhillyerd/inbucket/stringutil" ) // State tracks the current mode of our SMTP state machine @@ -266,7 +267,7 @@ func (ss *Session) readyHandler(cmd string, arg string) { return } from := m[1] - if _, _, err := ParseEmailAddress(from); err != nil { + if _, _, err := stringutil.ParseEmailAddress(from); err != nil { ss.send("501 Bad sender address syntax") ss.logWarn("Bad address as MAIL arg: %q, %s", from, err) return @@ -315,7 +316,7 @@ func (ss *Session) mailHandler(cmd string, arg string) { } // This trim is probably too forgiving recip := strings.Trim(arg[3:], "<> ") - if _, _, err := ParseEmailAddress(recip); err != nil { + if _, _, err := stringutil.ParseEmailAddress(recip); err != nil { ss.send("501 Bad recipient address syntax") ss.logWarn("Bad address as RCPT arg: %q, %s", recip, err) return @@ -355,7 +356,7 @@ func (ss *Session) dataHandler() { if ss.server.storeMessages { for e := ss.recipients.Front(); e != nil; e = e.Next() { recip := e.Value.(string) - local, domain, err := ParseEmailAddress(recip) + local, domain, err := stringutil.ParseEmailAddress(recip) if err != nil { ss.logError("Failed to parse address for %q", recip) ss.send(fmt.Sprintf("451 Failed to open mailbox for %v", recip)) diff --git a/smtpd/utils.go b/stringutil/utils.go similarity index 98% rename from smtpd/utils.go rename to stringutil/utils.go index 9b3eb17..450ae1f 100644 --- a/smtpd/utils.go +++ b/stringutil/utils.go @@ -1,4 +1,4 @@ -package smtpd +package stringutil import ( "bytes" @@ -41,7 +41,7 @@ func ParseMailboxName(localPart string) (result string, err error) { return result, nil } -// HashMailboxName accepts a mailbox name and hashes it. Inbucket uses this as +// HashMailboxName accepts a mailbox name and hashes it. filestore uses this as // the directory to house the mailbox func HashMailboxName(mailbox string) string { h := sha1.New() diff --git a/smtpd/utils_test.go b/stringutil/utils_test.go similarity index 99% rename from smtpd/utils_test.go rename to stringutil/utils_test.go index ae4fda3..330bfbd 100644 --- a/smtpd/utils_test.go +++ b/stringutil/utils_test.go @@ -1,4 +1,4 @@ -package smtpd +package stringutil import ( "strings" diff --git a/webui/mailbox_controller.go b/webui/mailbox_controller.go index d569106..b7c8a4e 100644 --- a/webui/mailbox_controller.go +++ b/webui/mailbox_controller.go @@ -10,7 +10,7 @@ import ( "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/httpd" "github.com/jhillyerd/inbucket/log" - "github.com/jhillyerd/inbucket/smtpd" + "github.com/jhillyerd/inbucket/stringutil" ) // MailboxIndex renders the index page for a particular mailbox @@ -24,7 +24,7 @@ func MailboxIndex(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) http.Redirect(w, req, httpd.Reverse("RootIndex"), http.StatusSeeOther) return nil } - name, err = smtpd.ParseMailboxName(name) + name, err = stringutil.ParseMailboxName(name) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) @@ -51,7 +51,7 @@ func MailboxIndex(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) @@ -67,7 +67,7 @@ func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( // MailboxList renders a list of messages in a mailbox. Renders a partial func MailboxList(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -94,7 +94,7 @@ func MailboxList(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -134,7 +134,7 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( func MailboxHTML(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -171,7 +171,7 @@ func MailboxHTML(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -206,7 +206,7 @@ func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) @@ -258,7 +258,7 @@ func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *httpd. // MailboxViewAttach sends the attachment to the client for online viewing func MailboxViewAttach(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) diff --git a/webui/root_controller.go b/webui/root_controller.go index 651e686..9a64bc5 100644 --- a/webui/root_controller.go +++ b/webui/root_controller.go @@ -8,7 +8,7 @@ import ( "github.com/jhillyerd/inbucket/config" "github.com/jhillyerd/inbucket/httpd" - "github.com/jhillyerd/inbucket/smtpd" + "github.com/jhillyerd/inbucket/stringutil" ) // RootIndex serves the Inbucket landing page @@ -58,7 +58,7 @@ func RootMonitorMailbox(w http.ResponseWriter, req *http.Request, ctx *httpd.Con http.Redirect(w, req, httpd.Reverse("RootIndex"), http.StatusSeeOther) return nil } - name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) + name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) From 6431b71cfe445234f7dc516c1a0f2491762abc3b Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 23:04:39 -0800 Subject: [PATCH 19/25] Refactor filestore into a dedicated pkg, closes #67 --- smtpd/filemsg.go => filestore/fmessage.go | 2 +- smtpd/filestore.go => filestore/fstore.go | 2 +- smtpd/filestore_test.go => filestore/fstore_test.go | 4 ++-- inbucket.go | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) rename smtpd/filemsg.go => filestore/fmessage.go (99%) rename smtpd/filestore.go => filestore/fstore.go (99%) rename smtpd/filestore_test.go => filestore/fstore_test.go (99%) diff --git a/smtpd/filemsg.go b/filestore/fmessage.go similarity index 99% rename from smtpd/filemsg.go rename to filestore/fmessage.go index 9030dbe..54da127 100644 --- a/smtpd/filemsg.go +++ b/filestore/fmessage.go @@ -1,4 +1,4 @@ -package smtpd +package filestore import ( "bufio" diff --git a/smtpd/filestore.go b/filestore/fstore.go similarity index 99% rename from smtpd/filestore.go rename to filestore/fstore.go index 4bef294..4ac7bc1 100644 --- a/smtpd/filestore.go +++ b/filestore/fstore.go @@ -1,4 +1,4 @@ -package smtpd +package filestore import ( "bufio" diff --git a/smtpd/filestore_test.go b/filestore/fstore_test.go similarity index 99% rename from smtpd/filestore_test.go rename to filestore/fstore_test.go index a27e5ca..9cac985 100644 --- a/smtpd/filestore_test.go +++ b/filestore/fstore_test.go @@ -1,4 +1,4 @@ -package smtpd +package filestore import ( "bytes" @@ -470,8 +470,8 @@ func TestGetLatestMessage(t *testing.T) { mb, err := ds.MailboxFor(mbName) assert.Nil(t, err) msg, err := mb.GetMessage("latest") + assert.Nil(t, msg) assert.Error(t, err) - fmt.Println(msg) // Deliver test message deliverMessage(ds, mbName, "test", time.Now()) diff --git a/inbucket.go b/inbucket.go index 7593000..83c39f4 100644 --- a/inbucket.go +++ b/inbucket.go @@ -13,6 +13,7 @@ import ( "time" "github.com/jhillyerd/inbucket/config" + "github.com/jhillyerd/inbucket/filestore" "github.com/jhillyerd/inbucket/httpd" "github.com/jhillyerd/inbucket/log" "github.com/jhillyerd/inbucket/msghub" @@ -115,7 +116,7 @@ func main() { msgHub := msghub.New(rootCtx, config.GetWebConfig().MonitorHistory) // Grab our datastore - ds := smtpd.DefaultFileDataStore() + ds := filestore.DefaultFileDataStore() // Start HTTP server httpd.Initialize(config.GetWebConfig(), shutdownChan, ds, msgHub) From 26c38b11489a033f3ce15b35e947e0aa591e99b8 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sat, 6 Jan 2018 16:45:12 -0800 Subject: [PATCH 20/25] Simple HTML sanitizer implementation --- sanitize/html.go | 9 +++ sanitize/html_test.go | 77 +++++++++++++++++++ themes/bootstrap/public/inbucket.css | 9 ++- themes/bootstrap/public/mailbox.js | 1 + themes/bootstrap/templates/mailbox/_show.html | 19 ++++- webui/mailbox_controller.go | 10 +++ 6 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 sanitize/html.go create mode 100644 sanitize/html_test.go diff --git a/sanitize/html.go b/sanitize/html.go new file mode 100644 index 0000000..d8e1c6e --- /dev/null +++ b/sanitize/html.go @@ -0,0 +1,9 @@ +package sanitize + +import "github.com/microcosm-cc/bluemonday" + +func HTML(html string) (output string, err error) { + policy := bluemonday.UGCPolicy() + output = policy.Sanitize(html) + return +} diff --git a/sanitize/html_test.go b/sanitize/html_test.go new file mode 100644 index 0000000..fa72e0c --- /dev/null +++ b/sanitize/html_test.go @@ -0,0 +1,77 @@ +package sanitize_test + +import ( + "testing" + + "github.com/jhillyerd/inbucket/sanitize" +) + +// TestHTMLPlainStrings test plain text passthrough +func TestHTMLPlainStrings(t *testing.T) { + testStrings := []string{ + "", + "plain string", + "one < two", + } + for _, ts := range testStrings { + t.Run(ts, func(t *testing.T) { + got, err := sanitize.HTML(ts) + if err != nil { + t.Fatal(err) + } + if got != ts { + t.Errorf("Got: %q, want: %q", got, ts) + } + }) + } +} + +// TestHTMLSimpleFormatting tests basic tags we should allow +func TestHTMLSimpleFormatting(t *testing.T) { + testStrings := []string{ + "

paragraph

", + "bold", + "italic", + "emphasis", + "strong", + "
text
", + } + for _, ts := range testStrings { + t.Run(ts, func(t *testing.T) { + got, err := sanitize.HTML(ts) + if err != nil { + t.Fatal(err) + } + if got != ts { + t.Errorf("Got: %q, want: %q", got, ts) + } + }) + } +} + +// TestHTMLScriptTags tests some strings with JavaScript +func TestHTMLScriptTags(t *testing.T) { + testCases := []struct { + input, want string + }{ + { + `safe`, + `safe`, + }, + { + `mysite`, + `mysite`, + }, + } + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + got, err := sanitize.HTML(tc.input) + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Errorf("Got: %q, want: %q", got, tc.want) + } + }) + } +} diff --git a/themes/bootstrap/public/inbucket.css b/themes/bootstrap/public/inbucket.css index c3c9f49..c895626 100644 --- a/themes/bootstrap/public/inbucket.css +++ b/themes/bootstrap/public/inbucket.css @@ -51,12 +51,17 @@ body { } } +#body-tabs > li > a { + padding-top: 6px; + padding-bottom: 6px; +} + .message-body { - padding: 0 5px; + padding: 10px 4px; } .message-attachments { - margin-top: 20px; + margin-top: 5px; padding: 10px 10px 0 0; } diff --git a/themes/bootstrap/public/mailbox.js b/themes/bootstrap/public/mailbox.js index 886c9a5..e807b1c 100644 --- a/themes/bootstrap/public/mailbox.js +++ b/themes/bootstrap/public/mailbox.js @@ -157,6 +157,7 @@ function onMessageLoaded(responseText, textStatus, XMLHttpRequest) { return; } onDocumentChange(); + $('#body-tabs a:first').tab('show') var top = $('#message-container').offset().top - navBarOffset; $(window).scrollTop(top); } diff --git a/themes/bootstrap/templates/mailbox/_show.html b/themes/bootstrap/templates/mailbox/_show.html index 33a9975..49621f0 100644 --- a/themes/bootstrap/templates/mailbox/_show.html +++ b/themes/bootstrap/templates/mailbox/_show.html @@ -24,7 +24,7 @@ class="btn btn-primary" onClick="htmlView('{{.message.ID}}');"> - HTML + Raw HTML {{end}}
@@ -78,7 +78,22 @@ {{end}} -
{{.body}}
+ +
+
{{.htmlBody}}
+
{{.body}}
+
{{with .attachments}}
diff --git a/webui/mailbox_controller.go b/webui/mailbox_controller.go index b7c8a4e..7f2d6e8 100644 --- a/webui/mailbox_controller.go +++ b/webui/mailbox_controller.go @@ -10,6 +10,7 @@ import ( "github.com/jhillyerd/inbucket/datastore" "github.com/jhillyerd/inbucket/httpd" "github.com/jhillyerd/inbucket/log" + "github.com/jhillyerd/inbucket/sanitize" "github.com/jhillyerd/inbucket/stringutil" ) @@ -118,6 +119,14 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( } body := template.HTML(httpd.TextToHTML(mime.Text)) htmlAvailable := mime.HTML != "" + var htmlBody template.HTML + if htmlAvailable { + if str, err := sanitize.HTML(mime.HTML); err == nil { + htmlBody = template.HTML(str) + } else { + log.Warnf("HTML sanitizer failed: %s", err) + } + } // Render partial template return httpd.RenderPartial("mailbox/_show.html", w, map[string]interface{}{ "ctx": ctx, @@ -125,6 +134,7 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( "message": msg, "body": body, "htmlAvailable": htmlAvailable, + "htmlBody": htmlBody, "mimeErrors": mime.Errors, "attachments": mime.Attachments, }) From 3b9af859248a029ad596260615bdf03f5d066b63 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Mon, 26 Feb 2018 21:25:22 -0800 Subject: [PATCH 21/25] sanitize: naive CSS sanitizer implementation - CSS sanitizer allows a limited set of properties in a style attribute. - Added a CSS inlined version of the tutsplus responsive test mail. - Linter fixes in inbucket.go --- CHANGELOG.md | 30 +- inbucket.go | 4 +- sanitize/css.go | 110 +++++++ sanitize/css_test.go | 34 ++ sanitize/html.go | 85 ++++- sanitize/html_test.go | 94 ++++++ swaks-tests/nonmime-html-inlined.raw | 393 ++++++++++++++++++++++++ swaks-tests/nonmime-html-responsive.raw | 2 +- swaks-tests/nonmime-html.raw | 4 +- swaks-tests/run-tests.sh | 3 +- 10 files changed, 737 insertions(+), 22 deletions(-) create mode 100644 sanitize/css.go create mode 100644 sanitize/css_test.go create mode 100644 swaks-tests/nonmime-html-inlined.raw diff --git a/CHANGELOG.md b/CHANGELOG.md index 554bfd3..a3a1cfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,16 @@ Change Log All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). -[1.2.0-rc2] - 2017-12-15 ------------------------- +## [Unreleased] + +### Added +- Button to purge mailbox contents from the UI. +- Simple HTML/CSS sanitization; `Safe HTML` and `Plain Text` UI tabs. + +### Changed +- Reverse message display sort order in the UI; now newest first. + +## [1.2.0-rc2] - 2017-12-15 ### Added - `rest/client` types `MessageHeader` and `Message` with convenience methods; @@ -20,8 +28,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). types - Fixed panic when `monitor.history` set to 0 -[1.2.0-rc1] - 2017-01-29 ------------------------- +## [1.2.0-rc1] - 2017-01-29 ### Added - Storage of `To:` header in messages (likely breaks existing datastores) @@ -47,8 +54,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Allow increased local-part length of 128 chars for Mailgun - RedHat and Ubuntu now use systemd instead of legacy init systems -[1.1.0] - 2016-09-03 --------------------- +## [1.1.0] - 2016-09-03 ### Added - Homebrew inbucket.conf and formula (see README) @@ -56,8 +62,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Log and continue when unable to delete oldest message during cap enforcement -[1.1.0-rc2] - 2016-03-06 ------------------------- +## [1.1.0-rc2] - 2016-03-06 ### Added - Message Cap to status page @@ -67,8 +72,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Shutdown hang in retention scanner - Display empty subject as `(No Subject)` -[1.1.0-rc1] - 2016-03-04 ------------------------- +## [1.1.0-rc1] - 2016-03-04 ### Added - Inbucket now builds with Go 1.5 or 1.6 @@ -82,8 +86,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - RESTful API moved to `/api/v1` base URI - More graceful shutdown on Ctrl-C or when errors encountered -[1.0] - 2014-04-14 ------------------- +## [1.0] - 2014-04-14 ### Added - Add new configuration option `mailbox.message.cap` to prevent individual @@ -100,8 +103,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). [1.0]: https://github.com/jhillyerd/inbucket/compare/1.0-rc1...1.0 -Release Checklist ------------------ +## Release Checklist 1. Create release branch: `git flow release start 1.x.0` 2. Update CHANGELOG.md: diff --git a/inbucket.go b/inbucket.go index 83c39f4..23c646a 100644 --- a/inbucket.go +++ b/inbucket.go @@ -86,7 +86,7 @@ func main() { } // Setup signal handler - sigChan := make(chan os.Signal) + sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) // Initialize logging @@ -150,7 +150,7 @@ signalLoop: log.Infof("Received SIGTERM, shutting down") close(shutdownChan) } - case _ = <-shutdownChan: + case <-shutdownChan: rootCancel() break signalLoop } diff --git a/sanitize/css.go b/sanitize/css.go new file mode 100644 index 0000000..e37d04e --- /dev/null +++ b/sanitize/css.go @@ -0,0 +1,110 @@ +package sanitize + +import ( + "bytes" + "strings" + + "github.com/gorilla/css/scanner" +) + +// propertyRule may someday allow control of what values are valid for a particular property. +type propertyRule struct{} + +var allowedProperties = map[string]propertyRule{ + "align": {}, + "background-color": {}, + "border": {}, + "border-bottom": {}, + "border-left": {}, + "border-radius": {}, + "border-right": {}, + "border-top": {}, + "box-sizing": {}, + "clear": {}, + "color": {}, + "content": {}, + "display": {}, + "font-family": {}, + "font-size": {}, + "font-weight": {}, + "height": {}, + "line-height": {}, + "margin": {}, + "margin-bottom": {}, + "margin-left": {}, + "margin-right": {}, + "margin-top": {}, + "max-height": {}, + "max-width": {}, + "overflow": {}, + "padding": {}, + "padding-bottom": {}, + "padding-left": {}, + "padding-right": {}, + "padding-top": {}, + "table-layout": {}, + "text-align": {}, + "text-decoration": {}, + "text-shadow": {}, + "vertical-align": {}, + "width": {}, + "word-break": {}, +} + +// Handler Token, return next state. +type stateHandler func(b *bytes.Buffer, t *scanner.Token) stateHandler + +func sanitizeStyle(input string) string { + b := &bytes.Buffer{} + scan := scanner.New(input) + state := stateStart + for { + t := scan.Next() + if t.Type == scanner.TokenEOF { + return b.String() + } + if t.Type == scanner.TokenError { + return "" + } + state = state(b, t) + if state == nil { + return "" + } + } +} + +func stateStart(b *bytes.Buffer, t *scanner.Token) stateHandler { + switch t.Type { + case scanner.TokenIdent: + _, ok := allowedProperties[strings.ToLower(t.Value)] + if !ok { + return stateEat + } + b.WriteString(t.Value) + return stateValid + case scanner.TokenS: + return stateStart + } + // Unexpected type. + b.WriteString("/*" + t.Type.String() + "*/") + return stateEat +} + +func stateEat(b *bytes.Buffer, t *scanner.Token) stateHandler { + if t.Type == scanner.TokenChar && t.Value == ";" { + // Done eating. + return stateStart + } + // Throw away this token. + return stateEat +} + +func stateValid(b *bytes.Buffer, t *scanner.Token) stateHandler { + state := stateValid + if t.Type == scanner.TokenChar && t.Value == ";" { + // End of property. + state = stateStart + } + b.WriteString(t.Value) + return state +} diff --git a/sanitize/css_test.go b/sanitize/css_test.go new file mode 100644 index 0000000..dc72a82 --- /dev/null +++ b/sanitize/css_test.go @@ -0,0 +1,34 @@ +package sanitize + +import ( + "testing" +) + +func TestSanitizeStyle(t *testing.T) { + testCases := []struct { + input, want string + }{ + {"", ""}, + { + "color: red;", + "color: red;", + }, + { + "background-color: black; color: white", + "background-color: black;color: white", + }, + { + "background-color: black; invalid: true; color: white", + "background-color: black;color: white", + }, + } + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + got := sanitizeStyle(tc.input) + if got != tc.want { + t.Errorf("got: %q, want: %q, input: %q", got, tc.want, tc.input) + } + }) + } + +} diff --git a/sanitize/html.go b/sanitize/html.go index d8e1c6e..475880f 100644 --- a/sanitize/html.go +++ b/sanitize/html.go @@ -1,9 +1,88 @@ package sanitize -import "github.com/microcosm-cc/bluemonday" +import ( + "bufio" + "bytes" + "io" + "regexp" + "strings" + + "github.com/microcosm-cc/bluemonday" + "golang.org/x/net/html" +) + +var ( + cssSafe = regexp.MustCompile(".*") + policy = bluemonday.UGCPolicy(). + AllowElements("center"). + AllowAttrs("style").Matching(cssSafe).Globally() +) func HTML(html string) (output string, err error) { - policy := bluemonday.UGCPolicy() - output = policy.Sanitize(html) + output, err = sanitizeStyleTags(html) + if err != nil { + return "", err + } + output = policy.Sanitize(output) return } + +func sanitizeStyleTags(input string) (string, error) { + r := strings.NewReader(input) + b := &bytes.Buffer{} + if err := styleTagFilter(b, r); err != nil { + return "", err + } + return b.String(), nil +} + +func styleTagFilter(w io.Writer, r io.Reader) error { + bw := bufio.NewWriter(w) + b := make([]byte, 256) + z := html.NewTokenizer(r) + for { + b = b[:0] + tt := z.Next() + switch tt { + case html.ErrorToken: + err := z.Err() + if err == io.EOF { + return bw.Flush() + } + return err + case html.StartTagToken, html.SelfClosingTagToken: + name, hasAttr := z.TagName() + if !hasAttr { + bw.Write(z.Raw()) + continue + } + b = append(b, '<') + b = append(b, name...) + for { + key, val, more := z.TagAttr() + strval := string(val) + style := false + if strings.ToLower(string(key)) == "style" { + style = true + strval = sanitizeStyle(strval) + } + if !style || strval != "" { + b = append(b, ' ') + b = append(b, key...) + b = append(b, '=', '"') + b = append(b, []byte(html.EscapeString(strval))...) + b = append(b, '"') + } + if !more { + break + } + } + if tt == html.SelfClosingTagToken { + b = append(b, '/') + } + bw.Write(append(b, '>')) + default: + bw.Write(z.Raw()) + } + } +} diff --git a/sanitize/html_test.go b/sanitize/html_test.go index fa72e0c..c6acf09 100644 --- a/sanitize/html_test.go +++ b/sanitize/html_test.go @@ -35,6 +35,7 @@ func TestHTMLSimpleFormatting(t *testing.T) { "emphasis", "strong", "
text
", + "
text
", } for _, ts := range testStrings { t.Run(ts, func(t *testing.T) { @@ -75,3 +76,96 @@ func TestHTMLScriptTags(t *testing.T) { }) } } + +func TestSanitizeStyleTags(t *testing.T) { + testCases := []struct { + name, input, want string + }{ + { + "empty", + ``, + ``, + }, + { + "open", + `
`, + `
`, + }, + { + "open close", + `
`, + `
`, + }, + { + "inner text", + `
foo bar
`, + `
foo bar
`, + }, + { + "self close", + `
`, + `
`, + }, + { + "open params", + `
`, + `
`, + }, + { + "open params squote", + `
`, + `
`, + }, + { + "open style", + `
`, + `
`, + }, + { + "open style squote", + `
`, + `
`, + }, + { + "open style mixed case", + `
`, + `
`, + }, + { + "closed style", + `
`, + `
`, + }, + { + "mixed case style", + `
`, + `
`, + }, + { + "mixed case invalid style", + `
`, + `
`, + }, + { + "mixed", + `

some text

`, + `

some text

`, + }, + { + "invalid styles", + `
`, + `
`, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := sanitize.HTML(tc.input) + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Errorf("input: %s\ngot : %s\nwant: %s", tc.input, got, tc.want) + } + }) + } +} diff --git a/swaks-tests/nonmime-html-inlined.raw b/swaks-tests/nonmime-html-inlined.raw new file mode 100644 index 0000000..99c80cf --- /dev/null +++ b/swaks-tests/nonmime-html-inlined.raw @@ -0,0 +1,393 @@ +Date: %DATE% +To: %TO_ADDRESS% +From: %FROM_ADDRESS% +Subject: tutsplus responsive inlined CSS +MIME-Version: 1.0 +Content-Type: text/html; charset="UTF-8" + + + + + + + + + + + + + +
+
+ + + + + + + + + + + + + + + + + + + + + + + +
+ +
+ + + + +
+

Lorem ipsum dolor sit amet

+

+ Compare to: + + tutsplus sample +

+ +

Copyright (c) 2015, Envato Tuts+
+ All rights reserved.

+ +

Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met:

+ +
    +
  • Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer.
    • + +
  • Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution.
    • +
+ +

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE + FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

+
+
+ +
+ + + + +
+ + + + + + + +
+ +
+ Maecenas sed ante pellentesque, posuere leo id, eleifend dolor. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. +
+
+
+ +
+ + + + +
+ + + + + + + +
+ +
+ Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Maecenas sed ante pellentesque, posuere leo id, eleifend dolor. +
+
+
+ +
+ +
+ + + + +
+ + + + + + + +
+ +
+ Scelerisque congue eros eu posuere. Praesent in felis ut velit pretium lobortis rhoncus ut erat. +
+
+
+ +
+ + + + +
+ + + + + + + +
+ +
+ Maecenas sed ante pellentesque, posuere leo id, eleifend dolor. +
+
+
+ +
+ + + + +
+ + + + + + + +
+ +
+ Praesent laoreet malesuada cursus. Maecenas scelerisque congue eros eu posuere. +
+
+
+ +
+ +
+ + + + +
+

Fashion

+

Class eleifend aptent taciti sociosqu ad litora torquent conubia

+

Read requirements

+
+
+ +
+ + + + +
+

Photography

+

Maecenas sed ante pellentesque, posuere leo id, eleifend dolor

+

See examples

+
+
+ +
+ + + + +
+

Design

+

Class aptent taciti sociosqu eleifend ad litora per conubia nostra

+

See the winners

+
+
+ +
+ + + + +
+

Cooking

+

Class aptent taciti eleifend sociosqu ad litora torquent conubia

+

Read recipes

+
+
+ +
+ + + + +
+

Woodworking

+

Maecenas sed ante pellentesque, posuere leo id, eleifend dolor

+

See examples

+
+
+ +
+ + + + +
+

Craft

+

Class aptent taciti sociosqu ad eleifend litora per conubia nostra

+

Vote now

+
+
+ +
+ +
+ + + + +
+ +
+
+ +
+ + + + +
+ Praesent laoreet malesuada cursus. Maecenas scelerisque congue eros eu posuere. Praesent in felis ut velit pretium lobortis rhoncus ut erat. Read on +
+
+ +
+ +
+ + + + +
+ +
+
+ +
+ + + + +
+ Maecenas sed ante pellentesque, posuere leo id, eleifend dolor. Class aptent taciti sociosqu ad litora torquent per conubia nostra. Per inceptos +
+
+ +
+ +
+
+ + diff --git a/swaks-tests/nonmime-html-responsive.raw b/swaks-tests/nonmime-html-responsive.raw index 84fd54e..7db14df 100644 --- a/swaks-tests/nonmime-html-responsive.raw +++ b/swaks-tests/nonmime-html-responsive.raw @@ -1,7 +1,7 @@ Date: %DATE% To: %TO_ADDRESS% From: %FROM_ADDRESS% -Subject: tutsplus responsive +Subject: tutsplus responsive external CSS MIME-Version: 1.0 Content-Type: text/html; charset="UTF-8" diff --git a/swaks-tests/nonmime-html.raw b/swaks-tests/nonmime-html.raw index 7f45889..3079d65 100644 --- a/swaks-tests/nonmime-html.raw +++ b/swaks-tests/nonmime-html.raw @@ -5,4 +5,6 @@ Subject: Swaks HTML MIME-Version: 1.0 Content-Type: text/html; charset="UTF-8" -This is a test of HTML at the top level. +

+ This is a test of HTML at the top level. +

diff --git a/swaks-tests/run-tests.sh b/swaks-tests/run-tests.sh index 0784a11..c9a93b8 100755 --- a/swaks-tests/run-tests.sh +++ b/swaks-tests/run-tests.sh @@ -53,5 +53,6 @@ swaks $* --data gmail.raw # Outlook test swaks $* --data outlook.raw -# Nonemime responsive HTML test +# Non-mime responsive HTML test swaks $* --data nonmime-html-responsive.raw +swaks $* --data nonmime-html-inlined.raw From 3c19e0820b021f12f1a7fa9dcc54d3d2cb7ce28b Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 27 Feb 2018 20:41:45 -0800 Subject: [PATCH 22/25] Add Makefile for developer convenience. --- .travis.yml | 6 ++---- Makefile | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 Makefile diff --git a/.travis.yml b/.travis.yml index 59d36e9..b9ea160 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,13 +5,11 @@ env: - DEPLOY_WITH_MAJOR="1.9" before_script: - - go vet ./... + - go get github.com/golang/lint/golint go: - - 1.8.x - 1.9.x - -script: go test -race -v ./... + - "1.10" deploy: provider: script diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..8101866 --- /dev/null +++ b/Makefile @@ -0,0 +1,35 @@ +PKG := inbucket +SHELL := /bin/sh + +SRC := $(shell find . -type f -name '*.go' -not -path "./vendor/*") +PKGS := $$(go list ./... | grep -v /vendor/) + +.PHONY: all build clean fmt install lint simplify test + +all: test lint build + +clean: + go clean + +deps: + go get -t ./... + +build: clean deps + go build + +install: build + go install + +test: clean deps + go test -race ./... + +fmt: + @gofmt -l -w $(SRC) + +simplify: + @gofmt -s -l -w $(SRC) + +lint: + @test -z "$(shell gofmt -l . | tee /dev/stderr)" || echo "[WARN] Fix formatting issues with 'make fmt'" + @golint -set_exit_status $${PKGS} + @go vet $${PKGS} From ffa756d8954fe0f79d1f5ee9a60a23d7ff9868d3 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 27 Feb 2018 21:15:45 -0800 Subject: [PATCH 23/25] gcloud: removed - Dockerized and moved to https://github.com/jhillyerd/demo.inbucket.org - Merge master changelog entry --- CHANGELOG.md | 6 ++++ etc/gcloud/create-demo-instance.sh | 8 ------ etc/gcloud/debian-startup.sh | 44 ------------------------------ etc/gcloud/demo-greeting.html | 15 ---------- 4 files changed, 6 insertions(+), 67 deletions(-) delete mode 100755 etc/gcloud/create-demo-instance.sh delete mode 100755 etc/gcloud/debian-startup.sh delete mode 100644 etc/gcloud/demo-greeting.html diff --git a/CHANGELOG.md b/CHANGELOG.md index a3a1cfe..d953fef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Changed - Reverse message display sort order in the UI; now newest first. +## [1.2.0] - 2017-12-27 + +### Changed +- No significant code changes from rc2 + ## [1.2.0-rc2] - 2017-12-15 ### Added @@ -95,6 +100,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). specific message. [Unreleased]: https://github.com/jhillyerd/inbucket/compare/master...develop +[1.2.0]: https://github.com/jhillyerd/inbucket/compare/1.2.0-rc2...1.2.0 [1.2.0-rc2]: https://github.com/jhillyerd/inbucket/compare/1.2.0-rc1...1.2.0-rc2 [1.2.0-rc1]: https://github.com/jhillyerd/inbucket/compare/1.1.0...1.2.0-rc1 [1.1.0]: https://github.com/jhillyerd/inbucket/compare/1.1.0-rc2...1.1.0 diff --git a/etc/gcloud/create-demo-instance.sh b/etc/gcloud/create-demo-instance.sh deleted file mode 100755 index 039be6f..0000000 --- a/etc/gcloud/create-demo-instance.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash -# create-demo-instance.sh - -set -x -gcloud compute instances create inbucket-1 --machine-type=f1-micro \ - --image-project=debian-cloud --image-family=debian-9 \ - --metadata-from-file=startup-script=debian-startup.sh,greeting=demo-greeting.html \ - --tags=http-server --address=inbucket-demo diff --git a/etc/gcloud/debian-startup.sh b/etc/gcloud/debian-startup.sh deleted file mode 100755 index a5989e7..0000000 --- a/etc/gcloud/debian-startup.sh +++ /dev/null @@ -1,44 +0,0 @@ -#!/bin/bash -# setup.sh -# description: Install Inbucket on Google Cloud debian9 instance - -inbucket_rel="1.2.0-rc2" -inbucket_pkg="inbucket_${inbucket_rel}_linux_amd64" -inbucket_url="https://dl.bintray.com/content/jhillyerd/golang/${inbucket_pkg}.tar.gz?direct" - -fauxmailer_rel="0.1" -fauxmailer_pkg="fauxmailer_${fauxmailer_rel}_linux_amd64" -fauxmailer_url="https://github.com/jhillyerd/fauxmailer/releases/download/0.1/${fauxmailer_pkg}.tar.gz" - -set -eo pipefail -[ $TRACE ] && set -x - -# Prerequisites -apt-get --yes update -apt-get --yes install curl libcap2-bin -id inbucket &>/dev/null || useradd -r -m inbucket - -# Extract -cd /opt -curl --location "$inbucket_url" | tar xzvf - -ln -s "$inbucket_pkg/" inbucket - -# Install -cd /opt/inbucket/etc/ubuntu -install -o inbucket -g inbucket -m 775 -d /var/opt/inbucket -touch /var/log/inbucket.log -chown inbucket: /var/log/inbucket.log -install -o root -g root -m 644 inbucket.logrotate /etc/logrotate.d/inbucket -install -o root -g root -m 644 inbucket.service /lib/systemd/system/inbucket.service -install -o root -g root -m 644 ../unix-sample.conf /etc/opt/inbucket.conf - -# Setup -setcap 'cap_net_bind_service=+ep' /opt/inbucket/inbucket -curl -sL -o /opt/inbucket/themes/greeting.html "http://metadata.google.internal/computeMetadata/v1/instance/attributes/greeting" -H "Metadata-Flavor: Google" -systemctl enable inbucket.service -systemctl start inbucket - -# Fauxmailer -cd /opt -curl --location "$fauxmailer_url" | tar xzvf - -ln -s "$fauxmailer_pkg/" fauxmailer diff --git a/etc/gcloud/demo-greeting.html b/etc/gcloud/demo-greeting.html deleted file mode 100644 index 6a4e85b..0000000 --- a/etc/gcloud/demo-greeting.html +++ /dev/null @@ -1,15 +0,0 @@ -

Welcome to the Inbucket demonstration instance

- -

tl;dr - click Monitor above to get started.

- -

Inbucket is an email testing service; it will accept email for any email -address and make it available to view without a password.

- -

This instance is running on Google Compute Engine, and utilizes -fauxmailer to generate -a stream of fake email. Feel free to poke around, delete mails, or try out -the REST API.

- -

To protect our visitors, this instance does not accept external mail. You -will need to install your own copy of Inbucket to use it in your -environment.

From b4abdb66750005099d472e91bf747b23e190efec Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Wed, 28 Feb 2018 12:37:56 -0800 Subject: [PATCH 24/25] Change to trash glyph for delete mailbox --- themes/bootstrap/public/mailbox.js | 18 ++++++++---------- themes/bootstrap/templates/mailbox/index.html | 4 ++-- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/themes/bootstrap/public/mailbox.js b/themes/bootstrap/public/mailbox.js index e807b1c..5a624c6 100644 --- a/themes/bootstrap/public/mailbox.js +++ b/themes/bootstrap/public/mailbox.js @@ -22,17 +22,15 @@ function deleteMessage(id) { }) } -// Delete the mailbox -function deleteMailBox() { - cont = confirm("Are you sure you want delete the Mailbox?") - if (cont == false) { - return; +// deleteMailbox clears the mailbox +function deleteMailbox() { + if (confirm("Are you sure you want delete this mailbox?")) { + $.ajax({ + type: 'DELETE', + url: '/api/v1/mailbox/' + mailbox, + success: loadList + }) } - $.ajax({ - type: 'DELETE', - url: '/api/v1/mailbox/' + mailbox, - success: loadList - }) } // flashTooltip temporarily changes the text of a tooltip diff --git a/themes/bootstrap/templates/mailbox/index.html b/themes/bootstrap/templates/mailbox/index.html index 8103bac..d9a570c 100644 --- a/themes/bootstrap/templates/mailbox/index.html +++ b/themes/bootstrap/templates/mailbox/index.html @@ -62,8 +62,8 @@ $(document).ready(function() { data-toggle="tooltip" data-placement="top" title="Delete Mailbox" - onclick="deleteMailBox()"> - + onclick="deleteMailbox()"> +
From 1ff8ffe9bd80d046c0c150793b0aac34d135f894 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Wed, 28 Feb 2018 12:50:39 -0800 Subject: [PATCH 25/25] Release prep for 1.3.0 --- CHANGELOG.md | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d953fef..922be06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ Change Log All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). -## [Unreleased] +## [v1.3.0] - 2018-02-28 ### Added - Button to purge mailbox contents from the UI. @@ -13,12 +13,12 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Changed - Reverse message display sort order in the UI; now newest first. -## [1.2.0] - 2017-12-27 +## [v1.2.0] - 2017-12-27 ### Changed - No significant code changes from rc2 -## [1.2.0-rc2] - 2017-12-15 +## [v1.2.0-rc2] - 2017-12-15 ### Added - `rest/client` types `MessageHeader` and `Message` with convenience methods; @@ -33,7 +33,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). types - Fixed panic when `monitor.history` set to 0 -## [1.2.0-rc1] - 2017-01-29 +## [v1.2.0-rc1] - 2017-01-29 ### Added - Storage of `To:` header in messages (likely breaks existing datastores) @@ -59,7 +59,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Allow increased local-part length of 128 chars for Mailgun - RedHat and Ubuntu now use systemd instead of legacy init systems -## [1.1.0] - 2016-09-03 +## [v1.1.0] - 2016-09-03 ### Added - Homebrew inbucket.conf and formula (see README) @@ -67,7 +67,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Log and continue when unable to delete oldest message during cap enforcement -## [1.1.0-rc2] - 2016-03-06 +## [v1.1.0-rc2] - 2016-03-06 ### Added - Message Cap to status page @@ -77,7 +77,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Shutdown hang in retention scanner - Display empty subject as `(No Subject)` -## [1.1.0-rc1] - 2016-03-04 +## [v1.1.0-rc1] - 2016-03-04 ### Added - Inbucket now builds with Go 1.5 or 1.6 @@ -91,7 +91,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - RESTful API moved to `/api/v1` base URI - More graceful shutdown on Ctrl-C or when errors encountered -## [1.0] - 2014-04-14 +## [v1.0] - 2014-04-14 ### Added - Add new configuration option `mailbox.message.cap` to prevent individual @@ -99,23 +99,24 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Add Link button to messages, allows for directing another person to a specific message. -[Unreleased]: https://github.com/jhillyerd/inbucket/compare/master...develop -[1.2.0]: https://github.com/jhillyerd/inbucket/compare/1.2.0-rc2...1.2.0 -[1.2.0-rc2]: https://github.com/jhillyerd/inbucket/compare/1.2.0-rc1...1.2.0-rc2 -[1.2.0-rc1]: https://github.com/jhillyerd/inbucket/compare/1.1.0...1.2.0-rc1 -[1.1.0]: https://github.com/jhillyerd/inbucket/compare/1.1.0-rc2...1.1.0 -[1.1.0-rc2]: https://github.com/jhillyerd/inbucket/compare/1.1.0-rc1...1.1.0-rc2 -[1.1.0-rc1]: https://github.com/jhillyerd/inbucket/compare/1.0...1.1.0-rc1 -[1.0]: https://github.com/jhillyerd/inbucket/compare/1.0-rc1...1.0 +[Unreleased]: https://github.com/jhillyerd/inbucket/compare/master...develop +[v1.3.0]: https://github.com/jhillyerd/inbucket/compare/1.2.0...1.3.0 +[v1.2.0]: https://github.com/jhillyerd/inbucket/compare/1.2.0-rc2...1.2.0 +[v1.2.0-rc2]: https://github.com/jhillyerd/inbucket/compare/1.2.0-rc1...1.2.0-rc2 +[v1.2.0-rc1]: https://github.com/jhillyerd/inbucket/compare/1.1.0...1.2.0-rc1 +[v1.1.0]: https://github.com/jhillyerd/inbucket/compare/1.1.0-rc2...1.1.0 +[v1.1.0-rc2]: https://github.com/jhillyerd/inbucket/compare/1.1.0-rc1...1.1.0-rc2 +[v1.1.0-rc1]: https://github.com/jhillyerd/inbucket/compare/1.0...1.1.0-rc1 +[v1.0]: https://github.com/jhillyerd/inbucket/compare/1.0-rc1...1.0 ## Release Checklist 1. Create release branch: `git flow release start 1.x.0` 2. Update CHANGELOG.md: - - Ensure *Unreleased* section is up to date - - Rename *Unreleased* section to release name and date. - - Add new GitHub `/compare` link + - Ensure *Unreleased* section is up to date + - Rename *Unreleased* section to release name and date. + - Add new GitHub `/compare` link 3. Run tests 4. Test cross-compile: `goreleaser --snapshot` 5. Commit changes and merge release: `git flow release finish`