~emersion/soju-dev

Finalize file upload support v1 SUPERSEDED

delthas: 1
 Finalize file upload support

 7 files changed, 157 insertions(+), 52 deletions(-)
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~emersion/soju-dev/patches/47141/mbox | git am -3
Learn more about email & git

[PATCH v1] Finalize file upload support Export this patch

- Adds http-ingress to know which hostname to advertise in our
  ISUPPORT
- Migrates old upload tokens to the new uploader on config reload
- Parses the Content-Disposition header to optionally build the
  uploaded file path using the passed file name
- Adds HEAD for the file upload server in order to make it easier
  for clients to build previews
---
Based on file-upload branch.

Also deletes an obsolete function, and fixes a few typos.

 cmd/soju/main.go         | 30 ++++++++++++++++-----
 config/config.go         | 10 +++++--
 doc/soju.1.scd           |  5 ++++
 downstream.go            | 43 ++++++++++++++++++-----------
 fileupload/fileupload.go | 58 ++++++++++++++++++++++++++++------------
 fileupload/fs.go         | 32 +++++++++++++++++-----
 server.go                | 31 ++++++++++++++++++---
 7 files changed, 157 insertions(+), 52 deletions(-)

diff --git a/cmd/soju/main.go b/cmd/soju/main.go
index b7c6358..75f57c5 100644
--- a/cmd/soju/main.go
+++ b/cmd/soju/main.go
@@ -26,6 +26,7 @@ import (
	"git.sr.ht/~emersion/soju/auth"
	"git.sr.ht/~emersion/soju/config"
	"git.sr.ht/~emersion/soju/database"
	"git.sr.ht/~emersion/soju/fileupload"
	"git.sr.ht/~emersion/soju/identd"
)

@@ -81,6 +82,19 @@ func loadConfig() (*config.Server, *soju.Config, error) {
		return nil, nil, fmt.Errorf("failed to create authenticator: %v", err)
	}

	ingress := raw.HTTPIngress
	if ingress == "" {
		ingress = "https://" + raw.Hostname
	}

	var uploader *fileupload.Uploader
	if raw.FileUpload.Driver != "" {
		uploader, err = fileupload.New(raw.FileUpload.Driver, raw.FileUpload.Source)
		if err != nil {
			log.Fatalf("failed to open file uploader: %v", err)
		}
	}

	if raw.TLS != nil {
		cert, err := tls.LoadX509KeyPair(raw.TLS.CertPath, raw.TLS.KeyPath)
		if err != nil {
@@ -95,6 +109,7 @@ func loadConfig() (*config.Server, *soju.Config, error) {
		MsgStoreDriver:            raw.MsgStore.Driver,
		MsgStorePath:              raw.MsgStore.Source,
		HTTPOrigins:               raw.HTTPOrigins,
		HTTPIngress:               ingress,
		AcceptProxyIPs:            raw.AcceptProxyIPs,
		MaxUserNetworks:           raw.MaxUserNetworks,
		UpstreamUserIPs:           raw.UpstreamUserIPs,
@@ -102,6 +117,7 @@ func loadConfig() (*config.Server, *soju.Config, error) {
		EnableUsersOnAuth:         raw.EnableUsersOnAuth,
		MOTD:                      motd,
		Auth:                      auth,
		Uploader:                  uploader,
	}
	return raw, cfg, nil
}
@@ -145,13 +161,13 @@ func main() {
	srv.SetConfig(serverCfg)
	srv.Logger = soju.NewLogger(log.Writer(), debug)

	wsMux := http.NewServeMux()
	wsMux.Handle("/socket", srv)

	httpMux := http.NewServeMux()
	httpMux.Handle("/socket", srv)
	var fileUploadHandler http.Handler // TODO
	if fileUploadHandler != nil {
		httpMux.Handle("/uploads", fileUploadHandler)
		httpMux.Handle("/uploads/", fileUploadHandler)
	}
	httpMux.Handle("/uploads", srv)
	httpMux.Handle("/uploads/", srv)

	for _, listen := range cfg.Listen {
		listen := listen // copy
@@ -249,7 +265,7 @@ func main() {
			httpSrv := http.Server{
				Addr:      addr,
				TLSConfig: tlsCfg,
				Handler:   srv,
				Handler:   wsMux,
			}
			go func() {
				if err := httpSrv.ListenAndServeTLS("", ""); err != nil {
@@ -263,7 +279,7 @@ func main() {
			}
			httpSrv := http.Server{
				Addr:    addr,
				Handler: srv,
				Handler: wsMux,
			}
			go func() {
				if err := httpSrv.ListenAndServe(); err != nil {
diff --git a/config/config.go b/config/config.go
index 162ed8d..f8a84f8 100644
--- a/config/config.go
+++ b/config/config.go
@@ -81,9 +81,10 @@ type Server struct {
	DB         DB
	MsgStore   MsgStore
	Auth       Auth
	FileUpload *FileUpload
	FileUpload FileUpload

	HTTPOrigins    []string
	HTTPIngress    string
	AcceptProxyIPs IPSet

	MaxUserNetworks           int
@@ -109,6 +110,7 @@ func Defaults() *Server {
		Auth: Auth{
			Driver: "internal",
		},
		HTTPIngress:     "https://" + hostname,
		MaxUserNetworks: -1,
	}
}
@@ -186,7 +188,7 @@ func parse(cfg scfg.Block) (*Server, error) {
			}
			switch srv.FileUpload.Driver {
			case "fs":
				if err := d.ParseParams(nil, &srv.MsgStore.Source); err != nil {
				if err := d.ParseParams(nil, &srv.FileUpload.Source); err != nil {
					return nil, err
				}
			default:
@@ -194,6 +196,10 @@ func parse(cfg scfg.Block) (*Server, error) {
			}
		case "http-origin":
			srv.HTTPOrigins = d.Params
		case "http-ingress":
			if err := d.ParseParams(&srv.HTTPIngress); err != nil {
				return nil, err
			}
		case "accept-proxy-ip":
			srv.AcceptProxyIPs = nil
			for _, s := range d.Params {
diff --git a/doc/soju.1.scd b/doc/soju.1.scd
index 1a02564..4c1eb50 100644
--- a/doc/soju.1.scd
+++ b/doc/soju.1.scd
@@ -147,6 +147,11 @@ The following directives are supported:

	(_log_ is a deprecated alias for this directive.)

*http-ingress* <url>
	External URL on which soju HTTP/HTTPS listeners are exposed.

	By default, this is _https://<hostname>_.

*http-origin* <patterns...>
	List of allowed HTTP origins for WebSocket listeners. The parameters are
	interpreted as shell patterns, see *glob*(7).
diff --git a/downstream.go b/downstream.go
index 57bac33..d02d89b 100644
--- a/downstream.go
+++ b/downstream.go
@@ -8,6 +8,7 @@ import (
	"errors"
	"fmt"
	"io"
	"log"
	"net"
	"strconv"
	"strings"
@@ -19,6 +20,7 @@ import (

	"git.sr.ht/~emersion/soju/auth"
	"git.sr.ht/~emersion/soju/database"
	"git.sr.ht/~emersion/soju/fileupload"
	"git.sr.ht/~emersion/soju/msgstore"
	"git.sr.ht/~emersion/soju/xirc"
)
@@ -333,10 +335,11 @@ type downstreamConn struct {
	id uint64

	// These don't change after connection registration
	registered bool
	user       *user
	network    *network // can be nil
	clientName string
	registered  bool
	user        *user
	network     *network // can be nil
	clientName  string
	uploadToken *fileupload.UploadToken

	nick     string
	nickCM   string
@@ -438,16 +441,17 @@ func (dc *downstreamConn) upstreamForCommand(cmd string) (*upstreamConn, error)
	return dc.network.conn, nil
}

func isOurNick(net *network, nick string) bool {
	// TODO: this doesn't account for nick changes
	if net.conn != nil {
		return net.conn.isOurNick(nick)
func (dc *downstreamConn) Close() error {
	dc.lock.Lock()
	if dc.uploadToken != nil {
		if u := dc.srv.Config().Uploader; u != nil {
			u.DelToken(dc.uploadToken)
		}
		dc.uploadToken = nil
	}
	// We're not currently connected to the upstream connection, so we don't
	// know whether this name is our nickname. Best-effort: use the network's
	// configured nickname and hope it was the one being used when we were
	// connected.
	return net.casemap(nick) == net.casemap(database.GetNick(&net.user.User, &net.Network))
	dc.lock.Unlock()

	return dc.conn.Close()
}

func (dc *downstreamConn) ReadMessage() (*irc.Message, error) {
@@ -476,7 +480,7 @@ func (dc *downstreamConn) readMessages(ch chan<- event) error {

// SendMessage sends an outgoing message.
//
// This can only called from the user goroutine.
// This can only be called from the user goroutine.
func (dc *downstreamConn) SendMessage(ctx context.Context, msg *irc.Message) {
	if !dc.caps.IsEnabled("message-tags") {
		if msg.Command == "TAGMSG" {
@@ -1472,6 +1476,15 @@ func (dc *downstreamConn) welcome(ctx context.Context, user *user) error {
	if dc.caps.IsEnabled("soju.im/webpush") {
		isupport = append(isupport, "VAPID="+dc.srv.webPush.VAPIDKeys.Public)
	}
	if u := dc.srv.Config().Uploader; u != nil {
		var err error
		dc.uploadToken, err = u.GenToken(dc.user.Username)
		if err != nil {
			log.Printf("warning: unable to generate upload token for %q: %v", dc.user.Username, err)
		} else {
			isupport = append(isupport, "FILEHOST="+dc.srv.Config().HTTPIngress+dc.uploadToken.URL())
		}
	}

	if uc := dc.upstream(); uc != nil {
		// If upstream doesn't support message-tags, indicate that we'll drop
@@ -1620,7 +1633,7 @@ func (dc *downstreamConn) welcome(ctx context.Context, user *user) error {
}

// messageSupportsBacklog checks whether the provided message can be sent as
// part of an history batch.
// part of a history batch.
func (dc *downstreamConn) messageSupportsBacklog(msg *irc.Message) bool {
	// Don't replay all messages, because that would mess up client
	// state. For instance we just sent the list of users, sending
diff --git a/fileupload/fileupload.go b/fileupload/fileupload.go
index aced0c7..b78558f 100644
--- a/fileupload/fileupload.go
+++ b/fileupload/fileupload.go
@@ -7,6 +7,7 @@ import (
	"io"
	"mime"
	"net/http"
	"path"
	"strings"
	"sync"
)
@@ -17,7 +18,7 @@ type Uploader struct {
	fs *fs

	mutex  sync.Mutex
	tokens map[string]struct{}
	tokens map[string]string // token -> user
}

func New(driver, source string) (*Uploader, error) {
@@ -25,7 +26,7 @@ func New(driver, source string) (*Uploader, error) {
	case "fs":
		return &Uploader{
			fs:     &fs{source},
			tokens: make(map[string]struct{}),
			tokens: make(map[string]string),
		}, nil
	default:
		return nil, fmt.Errorf("unknown file upload driver %q", driver)
@@ -34,12 +35,25 @@ func New(driver, source string) (*Uploader, error) {

func (u *Uploader) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
	switch req.Method {
	case http.MethodGet:
	case http.MethodHead, http.MethodGet:
		u.fetch(resp, req)
	case http.MethodPost:
		u.store(resp, req)
	default:
		http.Error(resp, "only GET and POST are allowed", http.StatusMethodNotAllowed)
		http.Error(resp, "only HEAD, GET and POST are allowed", http.StatusMethodNotAllowed)
	}
}

func (u *Uploader) Migrate(old *Uploader) {
	if old == nil {
		return
	}
	old.mutex.Lock()
	defer old.mutex.Unlock()
	u.mutex.Lock()
	defer u.mutex.Unlock()
	for token, user := range old.tokens {
		u.tokens[token] = user
	}
}

@@ -63,11 +77,21 @@ func (u *Uploader) store(resp http.ResponseWriter, req *http.Request) {
	}

	token := req.URL.Query().Get("token")
	if !u.checkToken(token) {
	user, ok := u.checkToken(token)
	if !ok {
		http.Error(resp, "invalid token", http.StatusForbidden)
		return
	}

	var filename string
	if _, params, err := mime.ParseMediaType(req.Header.Get("Content-Disposition")); err == nil {
		filename = params["filename"]
		filename = path.Clean(filename)
		if i := strings.LastIndexByte(filename, '/'); i >= 0 {
			filename = filename[i+1:]
		}
	}

	var mimeType string
	if contentType := req.Header.Get("Content-Type"); contentType != "" {
		var (
@@ -93,7 +117,7 @@ func (u *Uploader) store(resp http.ResponseWriter, req *http.Request) {
	}

	r := &limitedReader{r: req.Body, n: maxSize}
	filename, err := u.fs.store(r, mimeType)
	filename, err := u.fs.store(r, user, filename, mimeType)
	if err != nil {
		http.Error(resp, "failed to write file", http.StatusInternalServerError)
		return
@@ -103,18 +127,18 @@ func (u *Uploader) store(resp http.ResponseWriter, req *http.Request) {
	resp.WriteHeader(http.StatusCreated)
}

func (u *Uploader) GenToken() (*UploadToken, error) {
func (u *Uploader) GenToken(user string) (*UploadToken, error) {
	u.mutex.Lock()
	defer u.mutex.Unlock()

	for i := 0; i < 100; i++ {
		tok, err := generateToken()
		tok, err := generateToken(16)
		if err != nil {
			return nil, fmt.Errorf("failed to generate upload token: %v", err)
		}

		if _, taken := u.tokens[tok]; !taken {
			u.tokens[tok] = struct{}{}
			u.tokens[tok] = user
			return &UploadToken{tok}, nil
		}
	}
@@ -128,11 +152,11 @@ func (u *Uploader) DelToken(tok *UploadToken) {
	u.mutex.Unlock()
}

func (u *Uploader) checkToken(token string) bool {
func (u *Uploader) checkToken(token string) (user string, ok bool) {
	u.mutex.Lock()
	_, ok := u.tokens[token]
	user, ok = u.tokens[token]
	u.mutex.Unlock()
	return ok
	return
}

type UploadToken struct {
@@ -140,15 +164,15 @@ type UploadToken struct {
}

func (tok *UploadToken) URL() string {
	return "/upload?token=" + tok.token
	return "/uploads?token=" + tok.token
}

func generateToken() (string, error) {
	var b [16]byte
	if _, err := rand.Read(b[:]); err != nil {
func generateToken(len int) (string, error) {
	b := make([]byte, len)
	if _, err := rand.Read(b); err != nil {
		return "", err
	}
	return base64.RawURLEncoding.EncodeToString(b[:]), nil
	return base64.RawURLEncoding.EncodeToString(b), nil
}

type limitedReader struct {
diff --git a/fileupload/fs.go b/fileupload/fs.go
index 5e19d83..168a5e0 100644
--- a/fileupload/fs.go
+++ b/fileupload/fs.go
@@ -17,7 +17,7 @@ func (fs *fs) fetch(resp http.ResponseWriter, req *http.Request, filename string
	http.ServeFile(resp, req, filepath.Join(fs.dir, filename))
}

func (fs *fs) store(r io.Reader, mimeType string) (filename string, err error) {
func (fs *fs) store(r io.Reader, user string, name string, mimeType string) (filename string, err error) {
	var ext string
	if mimeType != "" {
		exts, _ := mime.ExtensionsByType(mimeType)
@@ -26,18 +26,35 @@ func (fs *fs) store(r io.Reader, mimeType string) (filename string, err error) {
		}
	}

	base := filepath.Join(fs.dir, user)
	if err := os.MkdirAll(base, 0700); err != nil {
		return "", fmt.Errorf("failed to create user directory: %v", err)
	}

	var f *os.File
	for i := 0; i < 100; i++ {
		filebase, err := generateToken()
		if err != nil {
			return "", fmt.Errorf("failed to generate file base: %v", err)
		if name != "" {
			token, err := generateToken(4)
			if err != nil {
				return "", fmt.Errorf("failed to generate file base: %v", err)
			}
			filename = fmt.Sprintf("%s-%s", token, name)
		} else {
			filebase, err := generateToken(16)
			if err != nil {
				return "", fmt.Errorf("failed to generate file base: %v", err)
			}
			filename = filebase + ext
		}

		filename = filebase + ext
		f, err = os.OpenFile(filepath.Join(fs.dir, filename), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600)
		f, err = os.OpenFile(filepath.Join(base, filename), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600)
		if err != nil {
			if os.IsExist(err) {
				continue
			}
			return "", fmt.Errorf("failed to open file: %v", err)
		}
		break
	}
	if f == nil {
		return "", fmt.Errorf("failed to pick filename")
@@ -51,5 +68,6 @@ func (fs *fs) store(r io.Reader, mimeType string) (filename string, err error) {
		return "", fmt.Errorf("failed to close file: %v", err)
	}

	return filename, nil
	location := fmt.Sprintf("%s/%s", user, filename)
	return location, nil
}
diff --git a/server.go b/server.go
index 552104e..d4363a5 100644
--- a/server.go
+++ b/server.go
@@ -10,6 +10,7 @@ import (
	"net"
	"net/http"
	"runtime/debug"
	"strings"
	"sync"
	"sync/atomic"
	"time"
@@ -23,6 +24,7 @@ import (
	"git.sr.ht/~emersion/soju/auth"
	"git.sr.ht/~emersion/soju/config"
	"git.sr.ht/~emersion/soju/database"
	"git.sr.ht/~emersion/soju/fileupload"
	"git.sr.ht/~emersion/soju/identd"
)

@@ -142,6 +144,7 @@ type Config struct {
	MsgStoreDriver            string
	MsgStorePath              string
	HTTPOrigins               []string
	HTTPIngress               string
	AcceptProxyIPs            config.IPSet
	MaxUserNetworks           int
	MOTD                      string
@@ -149,6 +152,7 @@ type Config struct {
	DisableInactiveUsersDelay time.Duration
	EnableUsersOnAuth         bool
	Auth                      auth.Authenticator
	Uploader                  *fileupload.Uploader
}

type Server struct {
@@ -156,7 +160,9 @@ type Server struct {
	Identd          *identd.Identd        // can be nil
	MetricsRegistry prometheus.Registerer // can be nil

	config atomic.Value // *Config
	configLock sync.Mutex
	config     *Config

	db     database.Database
	stopWG sync.WaitGroup
	stopCh chan struct{}
@@ -190,7 +196,7 @@ func NewServer(db database.Database) *Server {
		users:     make(map[string]*user),
		stopCh:    make(chan struct{}),
	}
	srv.config.Store(&Config{
	srv.SetConfig(&Config{
		Hostname:        "localhost",
		MaxUserNetworks: -1,
		Auth:            auth.NewInternal(),
@@ -203,11 +209,19 @@ func (s *Server) prefix() *irc.Prefix {
}

func (s *Server) Config() *Config {
	return s.config.Load().(*Config)
	s.configLock.Lock()
	cfg := s.config
	s.configLock.Unlock()
	return cfg
}

func (s *Server) SetConfig(cfg *Config) {
	s.config.Store(cfg)
	s.configLock.Lock()
	if cfg.Uploader != nil {
		cfg.Uploader.Migrate(s.config.Uploader)
	}
	s.config = cfg
	s.configLock.Unlock()
}

func (s *Server) Start() error {
@@ -640,6 +654,15 @@ func (s *Server) Serve(ln net.Listener, handler func(ircConn)) error {
}

func (s *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) {
	if strings.HasPrefix(req.URL.Path, "/uploads") {
		if u := s.Config().Uploader; u != nil {
			u.ServeHTTP(w, req)
		} else {
			http.NotFoundHandler().ServeHTTP(w, req)
		}
		return
	}

	conn, err := websocket.Accept(w, req, &websocket.AcceptOptions{
		Subprotocols:   []string{"text.ircv3.net"}, // non-compliant, fight me
		OriginPatterns: s.Config().HTTPOrigins,

base-commit: 27a4ed96c66b2137321fd99d1ebf37b2c64410d4
-- 
2.42.0
Thanks for this work!

I've reworked this a bit and pushed a new version to the file-upload
branch (tested via Goguma's filehost branch). Summary of changes:

- Added our own specification to doc/ext/. I don't want to implement someone
  else's draft specification, because that might change anytime. I prefer to
  start with our own vendored extension. Thus, we no longer need to stick with
  the way proval's FILEHOST was written, we can diverge a bit.
- Clients now pass the IRC connection credentials via HTTP. This is easier to
  implement for us (no more token bookkeeping) and more secure (no more
  secrets in URLs).
- Added mandatory OPTIONS handling for servers, so that clients can query
  capabilities.
- Reworked wording and requirements for the specification.
- Set Content-Disposition.
- Protect against directory traversal attacks.
- Disallow directory listings.
- Introduce a fileupload.Handler type for easier integration with authenticators
  and databases.