Verbose variant of 'target' in config

8 files changed, 273 insertions, 111 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d1e0df2..2a4f280 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Added
+- Verbose variant of 'target' in config: Do not hassle with urlencoded passwords anymore!
 
 ## [0.3.1] - 2020-05-12
 - Docker Setup
diff --git a/config.yml.example b/config.yml.example
index 14f413e..0e0e304 100644
--- a/config.yml.example
+++ b/config.yml.example
@@ -1,14 +1,24 @@
 # Example configuration. Each configuration option presented shows its default, so sensible.
 
 ## Target
-# The IMAP URI where to put emails, including credentials. Should start with imap:// for IMAP and imaps:// for IMAPS.
-# URI must be properly escaped, thus '@' as part of the user must be written as %40.
-# NB: This also applies for the password!
-# Format: scheme://user:password@host/PATH
-# Host can include a port; PATH denotes the root of the hierarchy for all feeds. Probably should start with INBOX.
-# Allowed delimiters in PATH are '/' -- they will be replaced by the correct delimiter. If you know yours, you can
-# also use that.
-target: imap://test%40example.com:passw0rd@mail.example.com:143/INBOX/Feeds
+target:
+  # scheme: either imap or imaps; if omitted, it is deduced from the port
+  scheme: imap
+  # user
+  user: test@example.com
+  # password
+  password: passw0rd
+  # host, without the port
+  host: mail.example.com
+  # port; optional if scheme is given
+  port: 143
+  # root denotes the root of the hierarchy for all feeds. Probably should start with INBOX.
+  # Allowed delimiters in PATH are '/' -- they will be replaced by the correct delimiter. If you know yours, you can
+  # also use that.
+  root: INBOX/Feeds
+# Instead of the verbose target, specifiying a URI is also legitimate. Be sure to properly url-encode user and password.
+# The example from above would read:
+# target: imap://test%40example.com:passw0rd@mail.example.com:143/INBOX/Feeds
 
 ## Global Options
 # Timeout in seconds for fetching feeds.
diff --git a/internal/imap/imap.go b/internal/imap/imap.go
index 63684a8..369d4b4 100644
--- a/internal/imap/imap.go
+++ b/internal/imap/imap.go
@@ -2,27 +2,27 @@ package imap
 
 import (
 	"fmt"
-	"net/url"
 	"strings"
 
 	imapClient "github.com/emersion/go-imap/client"
 
+	"github.com/Necoro/feed2imap-go/pkg/config"
 	"github.com/Necoro/feed2imap-go/pkg/log"
 )
 
-func newImapClient(url *URL, forceTls bool) (*imapClient.Client, error) {
+func newImapClient(url config.Url) (*imapClient.Client, error) {
 	var (
 		c   *imapClient.Client
 		err error
 	)
 
-	if forceTls {
-		if c, err = imapClient.DialTLS(url.Host, nil); err != nil {
+	if url.ForceTLS() {
+		if c, err = imapClient.DialTLS(url.HostPort(), nil); err != nil {
 			return nil, fmt.Errorf("connecting (TLS) to %s: %w", url.Host, err)
 		}
-		log.Print("Connected to ", url.Host, " (TLS)")
+		log.Print("Connected to ", url.HostPort(), " (TLS)")
 	} else {
-		if c, err = imapClient.Dial(url.Host); err != nil {
+		if c, err = imapClient.Dial(url.HostPort()); err != nil {
 			return nil, fmt.Errorf("connecting to %s: %w", url.Host, err)
 		}
 	}
@@ -30,32 +30,29 @@ func newImapClient(url *URL, forceTls bool) (*imapClient.Client, error) {
 	return c, nil
 }
 
-func (cl *Client) connect(url *URL, forceTls bool) (*connection, error) {
-	c, err := newImapClient(url, forceTls)
+func (cl *Client) connect(url config.Url) (*connection, error) {
+	c, err := newImapClient(url)
 	if err != nil {
 		return nil, err
 	}
 
 	conn := cl.createConnection(c)
 
-	if !forceTls {
+	if !url.ForceTLS() {
 		if err = conn.startTls(); err != nil {
 			return nil, err
 		}
 	}
 
-	pwd, _ := url.User.Password()
-	if err = c.Login(url.User.Username(), pwd); err != nil {
+	if err = c.Login(url.User, url.Password); err != nil {
 		return nil, fmt.Errorf("login to %s: %w", url.Host, err)
 	}
 
 	return conn, nil
 }
 
-func Connect(_url *url.URL) (*Client, error) {
+func Connect(url config.Url) (*Client, error) {
 	var err error
-	url := NewUrl(_url)
-	forceTls := url.ForceTLS()
 
 	client := NewClient()
 	client.host = url.Host
@@ -66,7 +63,7 @@ func Connect(_url *url.URL) (*Client, error) {
 	}()
 
 	var conn *connection // the main connection
-	if conn, err = client.connect(url, forceTls); err != nil {
+	if conn, err = client.connect(url); err != nil {
 		return nil, err
 	}
 
@@ -76,7 +73,7 @@ func Connect(_url *url.URL) (*Client, error) {
 	}
 	client.delimiter = delim
 
-	toplevel := url.Path
+	toplevel := url.Root
 	if toplevel[0] == '/' {
 		toplevel = toplevel[1:]
 	}
@@ -90,7 +87,7 @@ func Connect(_url *url.URL) (*Client, error) {
 
 	// the other connections
 	for i := 1; i < len(client.connections); i++ {
-		if _, err := client.connect(url, forceTls); err != nil { // explicitly new var 'err', b/c these are now harmless
+		if _, err := client.connect(url); err != nil { // explicitly new var 'err', b/c these are now harmless
 			log.Warnf("connecting #%d: %s", i, err)
 		}
 	}
diff --git a/internal/imap/url.go b/internal/imap/url.go
deleted file mode 100644
index 90c34e6..0000000
--- a/internal/imap/url.go
+++ /dev/null
@@ -1,76 +0,0 @@
-package imap
-
-import (
-	"net"
-	"net/url"
-
-	"github.com/Necoro/feed2imap-go/pkg/log"
-)
-
-// Our own convenience wrapper
-type URL struct {
-	*url.URL
-	// url.URL has no port field and splits it everytime from Host
-	port *string
-}
-
-const (
-	imapsPort   = "993"
-	imapPort    = "143"
-	imapsSchema = "imaps"
-	imapSchema  = "imap"
-)
-
-func (url *URL) Port() string {
-	if url.port == nil {
-		port := url.URL.Port()
-		url.port = &port
-	}
-	return *url.port
-}
-
-func (url *URL) ForceTLS() bool {
-	return url.Scheme == imapsSchema || url.Port() == imapsPort
-}
-
-func (url *URL) setDefaultScheme() {
-	switch url.Scheme {
-	case imapSchema, imapsSchema:
-		return
-	default:
-		oldScheme := url.Scheme
-		if url.Port() == imapsPort {
-			url.Scheme = imapsSchema
-		} else {
-			url.Scheme = imapSchema
-		}
-
-		if oldScheme != "" {
-			log.Warnf("Unknown scheme '%s', defaulting to '%s'", oldScheme, url.Scheme)
-		}
-	}
-}
-
-func (url *URL) setDefaultPort() {
-	if url.Port() == "" {
-		var port string
-		if url.Scheme == imapsSchema {
-			port = imapsPort
-		} else {
-			port = imapPort
-		}
-		url.port = &port
-		url.Host = net.JoinHostPort(url.Host, port)
-	}
-}
-
-func (url *URL) sanitizeUrl() {
-	url.setDefaultScheme()
-	url.setDefaultPort()
-}
-
-func NewUrl(url *url.URL) *URL {
-	u := URL{URL: url}
-	u.sanitizeUrl()
-	return &u
-}
diff --git a/main.go b/main.go
index e6faa68..f2a2447 100644
--- a/main.go
+++ b/main.go
@@ -3,7 +3,6 @@ package main
 import (
 	"flag"
 	"fmt"
-	"net/url"
 	"os"
 
 	"github.com/Necoro/feed2imap-go/internal/feed"
@@ -107,14 +106,9 @@ func run() error {
 
 	state.Filter()
 
-	imapUrl, err := url.Parse(cfg.Target)
-	if err != nil {
-		return fmt.Errorf("parsing 'target': %w", err)
-	}
-
 	var c *imap.Client
 	if !dryRun && !buildCache {
-		if c, err = imap.Connect(imapUrl); err != nil {
+		if c, err = imap.Connect(cfg.Target); err != nil {
 			return err
 		}
 
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 8e2aff6..377365f 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -19,7 +19,7 @@ type Map map[string]interface{}
 type GlobalOptions struct {
 	Timeout      int      `yaml:"timeout"`
 	DefaultEmail string   `yaml:"default-email"`
-	Target       string   `yaml:"target"`
+	Target       Url      `yaml:"target"`
 	Parts        []string `yaml:"parts"`
 	MaxFailures  int      `yaml:"max-failures"`
 }
@@ -29,7 +29,7 @@ var DefaultGlobalOptions = GlobalOptions{
 	Timeout:      30,
 	MaxFailures:  10,
 	DefaultEmail: username() + "@" + Hostname(),
-	Target:       "",
+	Target:       Url{},
 	Parts:        []string{"text", "html"},
 }
 
@@ -77,7 +77,7 @@ func WithDefault() *Config {
 
 // Validates the configuration against common mistakes
 func (cfg *Config) Validate() error {
-	if cfg.Target == "" {
+	if cfg.Target.Empty() {
 		return fmt.Errorf("No target set!")
 	}
 
diff --git a/pkg/config/url.go b/pkg/config/url.go
new file mode 100644
index 0000000..403f787
--- /dev/null
+++ b/pkg/config/url.go
@@ -0,0 +1,127 @@
+package config
+
+import (
+	"fmt"
+	"net"
+	"net/url"
+
+	"gopkg.in/yaml.v3"
+)
+
+type Url struct {
+	Scheme   string `yaml:"scheme"`
+	User     string `yaml:"user"`
+	Password string `yaml:"password"`
+	Host     string `yaml:"host"`
+	Port     string `yaml:"port"`
+	Root     string `yaml:"root"`
+}
+
+func (u *Url) Empty() bool {
+	return u.Host == ""
+}
+
+func (u *Url) UnmarshalYAML(value *yaml.Node) (err error) {
+	if value.ShortTag() == strTag {
+		var val string
+		var rawUrl *url.URL
+
+		if err = value.Decode(&val); err != nil {
+			return err
+		}
+		if rawUrl, err = url.Parse(val); err != nil {
+			return err
+		}
+
+		u.Scheme = rawUrl.Scheme
+		u.User = rawUrl.User.Username()
+		u.Password, _ = rawUrl.User.Password()
+		u.Host = rawUrl.Hostname()
+		u.Port = rawUrl.Port()
+		u.Root = rawUrl.Path
+	} else {
+		type _url Url // avoid recursion
+		wrapped := (*_url)(u)
+		if err = value.Decode(wrapped); err != nil {
+			return err
+		}
+	}
+
+	u.sanitize()
+
+	if errors := u.validate(); len(errors) > 0 {
+		errs := make([]string, len(errors)+1)
+		copy(errs[1:], errors)
+		errs[0] = fmt.Sprintf("line %d: Invalid target:", value.Line)
+		return &yaml.TypeError{Errors: errs}
+	}
+
+	return nil
+}
+
+func (u *Url) String() string {
+	var pwd string
+	if u.Password != "" {
+		pwd = ":******"
+	}
+
+	return fmt.Sprintf("%s://%s%s@%s%s", u.Scheme, u.User, pwd, u.HostPort(), u.Root)
+}
+
+func (u *Url) HostPort() string {
+	if u.Port != "" {
+		return net.JoinHostPort(u.Host, u.Port)
+	}
+	return u.Host
+}
+
+const (
+	imapsPort   = "993"
+	imapPort    = "143"
+	imapsSchema = "imaps"
+	imapSchema  = "imap"
+)
+
+func (u *Url) ForceTLS() bool {
+	return u.Scheme == imapsSchema || u.Port == imapsPort
+}
+
+func (u *Url) setDefaultScheme() {
+	if u.Scheme == "" {
+		if u.Port == imapsPort {
+			u.Scheme = imapsSchema
+		} else {
+			u.Scheme = imapSchema
+		}
+	}
+}
+
+func (u *Url) setDefaultPort() {
+	if u.Port == "" {
+		if u.Scheme == imapsSchema {
+			u.Port = imapsPort
+		} else {
+			u.Port = imapPort
+		}
+	}
+}
+
+func (u *Url) sanitize() {
+	u.setDefaultScheme()
+	u.setDefaultPort()
+}
+
+func (u *Url) validate() (errors []string) {
+	if u.Scheme != imapSchema && u.Scheme != imapsSchema {
+		errors = append(errors, fmt.Sprintf("Unknown scheme %q", u.Scheme))
+	}
+
+	if u.Host == "" {
+		errors = append(errors, "Host not set")
+	}
+
+	if u.Root == "" || u.Root == "/" {
+		errors = append(errors, "Root path not set")
+	}
+	return
+}
diff --git a/pkg/config/url_test.go b/pkg/config/url_test.go
new file mode 100644
index 0000000..d345690
--- /dev/null
+++ b/pkg/config/url_test.go
@@ -0,0 +1,108 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"gopkg.in/yaml.v3"
+)
+
+func TestUrl_Unmarshal(t *testing.T) {
+
+	tests := []struct {
+		name    string
+		inp     string
+		url     Url
+		wantErr bool
+	}{
+		{name: "Empty", inp: `url: ""`, wantErr: true},
+		{name: "Simple String", inp: `url: "imap://user:pass@example.net:143/INBOX"`, url: Url{
+			Scheme:   "imap",
+			User:     "user",
+			Password: "pass",
+			Host:     "example.net",
+			Port:     "143",
+			Root:     "/INBOX",
+		}},
+		{name: "Simple String with @", inp: `url: "imaps://user@example:pass@example.net:143/INBOX"`, url: Url{
+			Scheme:   "imaps",
+			User:     "user@example",
+			Password: "pass",
+			Host:     "example.net",
+			Port:     "143",
+			Root:     "/INBOX",
+		}},
+		{name: "Simple String with %40", inp: `url: "imap://user%40example:pass@example.net:4711/INBOX"`, url: Url{
+			Scheme:   "imap",
+			User:     "user@example",
+			Password: "pass",
+			Host:     "example.net",
+			Port:     "4711",
+			Root:     "/INBOX",
+		}},
+		{name: "Err: Inv scheme", inp: `url: "smtp://user%40example:pass@example.net:4711/INBOX"`, wantErr: true},
+		{name: "Err: No Host", inp: `url: "imap://user%40example:pass/INBOX"`, wantErr: true},
+		{name: "Err: Scheme Only", inp: `url: "imap://"`, wantErr: true},
+		{name: "Err: No Root", inp: `url: "imap://user:pass@example.net:143"`, wantErr: true},
+		{name: "Err: No Root: Slash", inp: `url: "imap://user:pass@example.net:143/"`, wantErr: true},
+		{name: "Full", inp: `url:
+  scheme: imap
+  host: example.net
+  user: user
+  password: p4ss
+  port: 143
+  root: INBOX
+`, url: Url{
+			Scheme:   "imap",
+			User:     "user",
+			Password: "p4ss",
+			Host:     "example.net",
+			Port:     "143",
+			Root:     "INBOX",
+		}},
+		{name: "Default Port", inp: `url:
+  scheme: imap
+  host: example.net
+  user: user
+  password: p4ss
+  root: INBOX
+`, url: Url{
+			Scheme:   "imap",
+			User:     "user",
+			Password: "p4ss",
+			Host:     "example.net",
+			Port:     "143",
+			Root:     "INBOX",
+		}},
+		{name: "Default Scheme", inp: `url:
+  host: example.net
+  user: user
+  password: p4ss
+  port: 993
+  root: INBOX
+`, url: Url{
+			Scheme:   "imaps",
+			User:     "user",
+			Password: "p4ss",
+			Host:     "example.net",
+			Port:     "993",
+			Root:     "INBOX",
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var u struct {
+				Url Url `yaml:"url"`
+			}
+			err := yaml.Unmarshal([]byte(tt.inp), &u)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Unmarshal() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if diff := cmp.Diff(u.Url, tt.url); err == nil && diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}