From 430ae68dd1143ff1af2970ca518532c65272c6dc Mon Sep 17 00:00:00 2001 From: drduh Date: Sun, 21 Dec 2025 14:43:46 -0800 Subject: [PATCH 1/4] split and test handler utils --- .gitignore | 1 + Makefile | 12 +++++-- handlers/form_test.go | 6 ++++ handlers/json.go | 30 ++++++++++++++++ handlers/param.go | 20 +++++++++++ handlers/param_test.go | 70 +++++++++++++++++++++++++++++++++++++ handlers/theme.go | 40 +++++++++++++++++++++ handlers/util.go | 79 +++--------------------------------------- 8 files changed, 181 insertions(+), 77 deletions(-) create mode 100644 handlers/json.go create mode 100644 handlers/param.go create mode 100644 handlers/param_test.go create mode 100644 handlers/theme.go diff --git a/.gitignore b/.gitignore index 51012ed..bf5fc9c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .DS_Store *.log +main release testCoverage* diff --git a/Makefile b/Makefile index a3e5b09..aa7275b 100644 --- a/Makefile +++ b/Makefile @@ -32,8 +32,10 @@ BUILDFLAG = \ -X "$(BUILDPKG).Version=$(VERSION)" BUILDCMD = $(GO) build -ldflags '-s -w $(BUILDFLAG)' BINNAME = $(APPNAME)-$(BUILDOS)-$(BUILDARCH)-$(VERSION) -GOBUILD = GOOS=$(BUILDOS) GOARCH=$(BUILDARCH) \ - $(BUILDCMD) -o $(OUT)/$(BINNAME) $(SRC) +GOBUILD = GOOS=$(BUILDOS) GOARCH=$(BUILDARCH) $(BUILDCMD) \ + -o "$(OUT)/$(BINNAME)" "$(SRC)" +GORACE = GOOS=$(BUILDOS) GOARCH=$(BUILDARCH) $(BUILDCMD) \ + -race -o "$(OUT)/$(BINNAME)-race" "$(SRC)" SERVICE = $(APPNAME).service @@ -123,6 +125,12 @@ lint: lint-verbose: @$(GOLINT) run -v ./... +build-race: prep + @$(GORACE) + +race: build-race + @$(OUT)/$(BINNAME)-race -debug + cover: test-cover @$(GO) tool cover -html=$(TESTCOVER) -o $(TESTCOVER).html @printf "cover: %s\n" "$$(file $(TESTCOVER).html)" diff --git a/handlers/form_test.go b/handlers/form_test.go index 74bc460..56f68b9 100644 --- a/handlers/form_test.go +++ b/handlers/form_test.go @@ -82,10 +82,16 @@ func TestParseFormDuration(t *testing.T) { def, def, maximum}, {"fraction", "/?duration=1.5h", "duration", def, 90 * time.Minute, maximum}, + {"no-unit", "/?duration=3333", "duration", + def, def, maximum}, + {"bad-unit", "/duration=8h", "duration", + def, def, maximum}, {"large", "/?duration=9999h", "duration", def, maximum, maximum}, {"xlarge", "/?duration=99999999999h", "duration", def, def, maximum}, // overflows int64 + {"encoded", "/?duration=%32%34%68", "duration", + def, 24 * time.Hour, maximum}, // "24h" encoded } for _, tc := range tests { tc := tc diff --git a/handlers/json.go b/handlers/json.go new file mode 100644 index 0000000..951b558 --- /dev/null +++ b/handlers/json.go @@ -0,0 +1,30 @@ +package handlers + +import ( + "encoding/json" + "net/http" +) + +// deny serves a JSON response for disallowed requests. +func deny(w http.ResponseWriter, code int, reason string, r *Request) { + writeJSON(w, code, errorJSON(reason)) +} + +// errorJSON returns an error string map containing the string. +func errorJSON(s string) map[string]string { + return map[string]string{ + "error": s, + } +} + +// writeJSON serves a JSON response with data. +func writeJSON(w http.ResponseWriter, code int, data interface{}) { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.WriteHeader(code) + err := json.NewEncoder(w).Encode(data) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(errorJSON(err.Error())) + return + } +} diff --git a/handlers/param.go b/handlers/param.go new file mode 100644 index 0000000..f37845e --- /dev/null +++ b/handlers/param.go @@ -0,0 +1,20 @@ +package handlers + +import "net/http" + +// getRequestParameter returns a parameter from the request +// URL or a form value. +func getRequestParameter( + r *http.Request, pathLen int, fieldName string) string { + if pathLen > len(r.URL.Path) { + return "" + } + p := r.URL.Path[pathLen:] + if p != "" { + return p + } + if queryValue := r.URL.Query().Get(fieldName); queryValue != "" { + return queryValue + } + return r.FormValue(fieldName) +} diff --git a/handlers/param_test.go b/handlers/param_test.go new file mode 100644 index 0000000..8ec25cd --- /dev/null +++ b/handlers/param_test.go @@ -0,0 +1,70 @@ +package handlers + +import ( + "net/http" + "net/url" + "testing" +) + +// TestGetRequestParameter validates the parameter value +// is read from the URL or form. +func TestGetRequestParameter(t *testing.T) { + tests := []struct { + name string + request *http.Request + pathLen int + field string + want string + }{ + { + name: "URL parameter", + request: &http.Request{ + URL: &url.URL{Path: "/download/id1"}, + }, + pathLen: 10, + field: "", + want: "id1", + }, + { + name: "Query parameter", + request: &http.Request{ + URL: &url.URL{ + Path: "/download/", + RawQuery: "field=id2", + }, + }, + pathLen: 10, + field: "field", + want: "id2", + }, + { + name: "Form parameter", + request: &http.Request{ + Body: http.NoBody, + Form: url.Values{"field": {"id3"}}, + URL: &url.URL{Path: "/download/"}, + }, + pathLen: 10, + field: "field", + want: "id3", + }, + { + name: "No parameter", + request: &http.Request{ + Body: http.NoBody, + URL: &url.URL{Path: "/download/"}, + }, + pathLen: 10, + field: "", + want: "", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := getRequestParameter(test.request, test.pathLen, test.field) + if result != test.want { + t.Fatalf("%s: expect '%q'; got '%q'", test.name, test.want, result) + } + }) + } +} diff --git a/handlers/theme.go b/handlers/theme.go new file mode 100644 index 0000000..311b66c --- /dev/null +++ b/handlers/theme.go @@ -0,0 +1,40 @@ +package handlers + +import ( + "net/http" + "slices" + "time" + + "github.com/drduh/gone/auth" + "github.com/drduh/gone/util" +) + +const autoTheme = "auto" + +// getDefaultTheme returns a default theme, based on +// the current time if set to automatically theme. +func getDefaultTheme(theme string) string { + if theme != autoTheme { + return theme + } + if util.IsDaytime() { + return "light" + } + return "dark" +} + +// getTheme returns the CSS theme based on cookie preference, +// setting the cookie value if none exists, or is invalid. +func getTheme(w http.ResponseWriter, r *http.Request, + defaultTheme, id string, t time.Duration, themes []string) string { + formContent := r.FormValue(formFieldTheme) + if formContent != "" { + theme := formContent + if !slices.Contains(themes, theme) { + theme = getDefaultTheme(autoTheme) + } + http.SetCookie(w, auth.NewCookie(theme, id, t)) + return theme + } + return auth.GetCookie(w, r, defaultTheme, id, t) +} diff --git a/handlers/util.go b/handlers/util.go index f605cb6..2618998 100644 --- a/handlers/util.go +++ b/handlers/util.go @@ -1,26 +1,14 @@ package handlers import ( - "encoding/json" "net" "net/http" - "slices" - "time" "github.com/drduh/gone/auth" "github.com/drduh/gone/config" "github.com/drduh/gone/util" ) -const autoTheme = "auto" - -// deny serves a JSON response for disallowed requests. -func deny(w http.ResponseWriter, httpCode int, reason string, - app *config.App, r *Request) { - writeJSON(w, httpCode, errorJSON(reason)) - app.Log.Error(reason, "user", r) -} - // toRoot redirects an HTTP request to the root/index handler. func toRoot(w http.ResponseWriter, r *http.Request, rootPath string) { http.Redirect(w, r, rootPath, http.StatusSeeOther) @@ -54,25 +42,6 @@ func isAuthenticated(app *config.App, r *http.Request) bool { return auth.Basic(app.Basic.Field, app.Basic.Token, r) } -// errorJSON returns an error string map containing the string. -func errorJSON(s string) map[string]string { - return map[string]string{ - "error": s, - } -} - -// getDefaultTheme returns a default theme, based on -// the current time if set to automatically theme. -func getDefaultTheme(theme string) string { - if theme != autoTheme { - return theme - } - if util.IsDaytime() { - return "light" - } - return "dark" -} - // parseRequest returns a Request with masked address. func parseRequest(r *http.Request) *Request { address, _, err := net.SplitHostPort(r.RemoteAddr) @@ -93,54 +62,14 @@ func authRequest(w http.ResponseWriter, r *http.Request, app *config.App) *Request { req := parseRequest(r) if !isAuthenticated(app, r) { - deny(w, http.StatusForbidden, app.Deny, app, req) + deny(w, http.StatusForbidden, app.Deny, req) + app.Log.Error(app.Deny, "user", r) return nil } if !app.Authorize(app.ReqsPerMinute) { - deny(w, http.StatusTooManyRequests, app.RateLimit, app, req) + deny(w, http.StatusTooManyRequests, app.RateLimit, req) + app.Log.Error(app.RateLimit, "user", r) return nil } return req } - -// writeJSON serves a JSON response with data. -func writeJSON(w http.ResponseWriter, code int, data interface{}) { - w.Header().Set("Content-Type", "application/json; charset=utf-8") - w.WriteHeader(code) - err := json.NewEncoder(w).Encode(data) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - _ = json.NewEncoder(w).Encode(errorJSON(err.Error())) - return - } -} - -// getRequestParameter returns a request parameter from the -// URL or a form value. -func getRequestParameter(r *http.Request, - pathLen int, fieldName string) string { - p := r.URL.Path[pathLen:] - if p == "" { - p = r.URL.Query().Get(fieldName) - } - if p == "" { - p = r.FormValue(fieldName) - } - return p -} - -// getTheme returns the CSS theme based on cookie preference, -// setting the cookie value if none exists, or is invalid. -func getTheme(w http.ResponseWriter, r *http.Request, - defaultTheme, id string, t time.Duration, themes []string) string { - formContent := r.FormValue(formFieldTheme) - if formContent != "" { - theme := formContent - if !slices.Contains(themes, theme) { - theme = getDefaultTheme(autoTheme) - } - http.SetCookie(w, auth.NewCookie(theme, id, t)) - return theme - } - return auth.GetCookie(w, r, defaultTheme, id, t) -} From ff5fccd36e0ff154494ce52a4018d54922b16bdb Mon Sep 17 00:00:00 2001 From: drduh Date: Sun, 21 Dec 2025 14:48:03 -0800 Subject: [PATCH 2/4] more sanitize tests, lock multi-uploads, allow filename spaces --- handlers/message.go | 5 +--- handlers/upload.go | 5 +++- handlers/wall.go | 2 +- settings/defaultSettings.json | 2 +- storage/sanitize.go | 7 +++++- storage/sanitize_test.go | 44 ++++++++++++++++++++++++----------- templates/data/list.tmpl | 6 ++--- 7 files changed, 47 insertions(+), 24 deletions(-) diff --git a/handlers/message.go b/handlers/message.go index f3e43e3..d320db8 100644 --- a/handlers/message.go +++ b/handlers/message.go @@ -41,9 +41,6 @@ func Message(app *config.App) http.HandlerFunc { if formContent != "" { message.Count++ message.Data = formContent - app.Log.Debug("adding message", - "count", message.Count, - "content", message.Data, "user", req) app.Messages[message.Count] = &message app.Log.Info("added message", "user", req) } @@ -52,7 +49,7 @@ func Message(app *config.App) http.HandlerFunc { } if r.URL.Query().Get("download") == "all" { - app.Log.Debug("serving all messages", + app.Log.Debug("dowloading messages", "count", app.NumMessages, "user", req) app.ServeMessages(w) return diff --git a/handlers/upload.go b/handlers/upload.go index 90475ed..95a857b 100644 --- a/handlers/upload.go +++ b/handlers/upload.go @@ -46,6 +46,7 @@ func Upload(app *config.App) http.HandlerFunc { var upload storage.File var uploads []storage.File + var mu sync.Mutex var wg sync.WaitGroup formFileContent := r.MultipartForm.File["file"] @@ -59,6 +60,8 @@ func Upload(app *config.App) http.HandlerFunc { for _, fileHeader := range formFileContent { go func(fileHeader *multipart.FileHeader) { defer wg.Done() + mu.Lock() + defer mu.Unlock() file, err := fileHeader.Open() if err != nil { app.Log.Error(app.Copy, "error", err.Error(), "user", req) @@ -78,7 +81,7 @@ func Upload(app *config.App) http.HandlerFunc { } filename := storage.SanitizeName(fileHeader.Filename, - app.MaxSizeName, app.FilenameExtraChars) + app.FilenameExtraChars, app.MaxSizeName) f := &storage.File{ Name: filename, Data: buf.Bytes(), diff --git a/handlers/wall.go b/handlers/wall.go index 19fb626..32a30fe 100644 --- a/handlers/wall.go +++ b/handlers/wall.go @@ -36,8 +36,8 @@ func Wall(app *config.App) http.HandlerFunc { } if r.URL.Query().Get("download") == "all" { - app.Log.Debug("serving wall content", "user", req) app.ServeWall(w) + app.Log.Info("downloaded wall", "user", req) return } diff --git a/settings/defaultSettings.json b/settings/defaultSettings.json index 6051b07..384311b 100644 --- a/settings/defaultSettings.json +++ b/settings/defaultSettings.json @@ -74,7 +74,7 @@ }, "limit": { "expiryTicker": "30s", - "filenameChars": "_.-", + "filenameChars": "_.- ", "maxDownloads": 100, "maxDuration": "192h", "maxSizeMsg": 128, diff --git a/storage/sanitize.go b/storage/sanitize.go index a374eab..df83257 100644 --- a/storage/sanitize.go +++ b/storage/sanitize.go @@ -1,6 +1,7 @@ package storage import ( + "net/url" "path/filepath" "strings" "unicode" @@ -9,7 +10,11 @@ import ( const defaultName = "default" // SanitizeName validates strings for use as filename. -func SanitizeName(input string, maxLength int, extraChars string) string { +func SanitizeName(input, extraChars string, maxLength int) string { + input, err := url.QueryUnescape(input) + if err != nil { + return defaultName + } f := filepath.Base(input) f = removeInvalidChars(f, extraChars) ext := filepath.Ext(f) diff --git a/storage/sanitize_test.go b/storage/sanitize_test.go index a2d061f..3efe475 100644 --- a/storage/sanitize_test.go +++ b/storage/sanitize_test.go @@ -1,8 +1,11 @@ package storage -import "testing" +import ( + "strings" + "testing" +) -const extraChars = "_.-" +const extraChars = "_.- " // TestRemoveInvalidChars validates invalid characters // are removed from filename strings. @@ -65,23 +68,38 @@ func TestTruncateName(t *testing.T) { func TestSanitizeName(t *testing.T) { tests := []struct { input string - maxLength int extraChars string + maxLength int want string }{ - {"my@invalid#name?.txt", 20, extraChars, "myinvalidname.txt"}, - {"averylongfilename.png", 15, extraChars, "averylongfi.png"}, - {"my.file.name.txt", 20, extraChars, "my.file.name.txt"}, - {".hiddenfile", 15, extraChars, defaultName + ".hidd"}, - {"myfilename.png", 20, extraChars, "myfilename.png"}, - {"/path/to/file.txt", 20, extraChars, "file.txt"}, - {"myfilenames", 10, extraChars, "myfilename"}, - {".@#$%^&*.png", 15, extraChars, "..png"}, - {"@#$%^&*", 10, extraChars, defaultName}, + {"my@invalid#name?.txt", extraChars, 20, "myinvalidname.txt"}, + {"averylongfilename.png", extraChars, 15, "averylongfi.png"}, + {"my.file.name.txt", extraChars, 20, "my.file.name.txt"}, + {".hiddenfile", extraChars, 15, defaultName + ".hidd"}, + {"myfilename.png", extraChars, 20, "myfilename.png"}, + {"/path/to/file.txt", extraChars, 20, "file.txt"}, + {"myfilenames", extraChars, 10, "myfilename"}, + {"@#$%^&*.png", extraChars, 20, defaultName}, + {"!@#$%^&*()[]{}<>", extraChars, 20, defaultName}, + {"filename.", extraChars, 15, "filename."}, + {"/etc/passwd", extraChars, 15, "passwd"}, + {"name\u0000.txt", extraChars, 20, "name.txt"}, + {"control\ttest.txt", extraChars, 20, "controltest.txt"}, + {"/path/../file.txt", extraChars, 20, "file.txt"}, + {"", extraChars, 25, "script"}, + {"percent%encoded%name.doc", extraChars, 20, defaultName}, + {"filename%20with%20spaces.txt", extraChars, 20, + "filename with sp.txt"}, + {"filename with spaces.txt", extraChars, 25, + "filename with spaces.txt"}, + {"my%2Fcool%2Bdoc%26about%2Cstuff.md", extraChars + "/+&,", 40, + "cool+doc&about,stuff.md"}, + {strings.Repeat("a", 1000) + ".txt", extraChars, 50, + strings.Repeat("a", 46) + ".txt"}, } for _, test := range tests { t.Run(test.input, func(t *testing.T) { - result := SanitizeName(test.input, test.maxLength, test.extraChars) + result := SanitizeName(test.input, test.extraChars, test.maxLength) if result != test.want { t.Fatalf("length: %d, special: '%s', got: '%s', want: '%s'", test.maxLength, test.extraChars, result, test.want) diff --git a/templates/data/list.tmpl b/templates/data/list.tmpl index 1792e00..ed29d54 100644 --- a/templates/data/list.tmpl +++ b/templates/data/list.tmpl @@ -20,10 +20,10 @@ - + - + @@ -31,7 +31,7 @@ {{ range .Storage.Files }}
NameFile Size TypeOwnerUser Time
- + Date: Sun, 21 Dec 2025 15:44:02 -0800 Subject: [PATCH 3/4] trim extensions too --- handlers/list.go | 1 - storage/list.go | 1 + storage/sanitize.go | 15 +++++++++++---- storage/sanitize_test.go | 10 ++++++++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/handlers/list.go b/handlers/list.go index cddc90e..bc82477 100644 --- a/handlers/list.go +++ b/handlers/list.go @@ -13,7 +13,6 @@ func List(app *config.App) http.HandlerFunc { if req == nil { return } - app.UpdateTimeRemaining() files := app.ListFiles() app.Log.Info("serving file list", "files", len(files), "user", req) diff --git a/storage/list.go b/storage/list.go index efc44a6..80131c6 100644 --- a/storage/list.go +++ b/storage/list.go @@ -11,6 +11,7 @@ func (s *Storage) ListFiles() []File { break } + s.UpdateTimeRemaining() f := File{ Id: file.Id, Name: file.Name, diff --git a/storage/sanitize.go b/storage/sanitize.go index df83257..ae46726 100644 --- a/storage/sanitize.go +++ b/storage/sanitize.go @@ -7,10 +7,16 @@ import ( "unicode" ) -const defaultName = "default" +const ( + defaultName = "default" + maxExtLength = 5 +) // SanitizeName validates strings for use as filename. func SanitizeName(input, extraChars string, maxLength int) string { + if strings.TrimSpace(input) == "" { + return defaultName + } input, err := url.QueryUnescape(input) if err != nil { return defaultName @@ -41,10 +47,11 @@ func removeInvalidChars(filename string, allowed string) string { // truncateName trims a filename string to max size, // preserving reasonably-sized original file extensions. func truncateName(base string, ext string, maxLength int) string { - const maxExtensionLength = 5 - if len(ext) > maxExtensionLength { - ext = ext[:maxExtensionLength] + ext = strings.ReplaceAll(ext, " ", "") + if len(ext) > maxExtLength { + ext = ext[:maxExtLength] } + base = strings.TrimSpace(base) totalLength := len(base) + len(ext) if totalLength <= maxLength { return base + ext diff --git a/storage/sanitize_test.go b/storage/sanitize_test.go index 3efe475..5bca527 100644 --- a/storage/sanitize_test.go +++ b/storage/sanitize_test.go @@ -47,8 +47,10 @@ func TestTruncateName(t *testing.T) { {"base", ".longextension", 12, "base.long"}, {"exactfit", ".jpeg", 13, "exactfit.jpeg"}, {"longfilename", ".txt", 10, "longfi.txt"}, + {"file", ". . . long", 12, "file...lo"}, {"truncatebase", ".png", 8, "trun.png"}, {"onlybase", ".toolong", 8, "onl.tool"}, + {"test", "test test", 18, "testtestt"}, {"short", ".dat", 9, "short.dat"}, {"example", "", 5, "examp"}, {"", ".zip", 5, ".zip"}, @@ -81,12 +83,16 @@ func TestSanitizeName(t *testing.T) { {"myfilenames", extraChars, 10, "myfilename"}, {"@#$%^&*.png", extraChars, 20, defaultName}, {"!@#$%^&*()[]{}<>", extraChars, 20, defaultName}, + {"!@# a copy.alongext", extraChars, 20, "a copy.alon"}, + {"<>$ a copy.m", extraChars, 10, "a copy.m"}, + {"a copy.my copy", extraChars, 15, "a copy.myco"}, {"filename.", extraChars, 15, "filename."}, {"/etc/passwd", extraChars, 15, "passwd"}, {"name\u0000.txt", extraChars, 20, "name.txt"}, {"control\ttest.txt", extraChars, 20, "controltest.txt"}, {"/path/../file.txt", extraChars, 20, "file.txt"}, {"", extraChars, 25, "script"}, + {" ", extraChars, 15, "default"}, {"percent%encoded%name.doc", extraChars, 20, defaultName}, {"filename%20with%20spaces.txt", extraChars, 20, "filename with sp.txt"}, @@ -94,8 +100,12 @@ func TestSanitizeName(t *testing.T) { "filename with spaces.txt"}, {"my%2Fcool%2Bdoc%26about%2Cstuff.md", extraChars + "/+&,", 40, "cool+doc&about,stuff.md"}, + {"example." + strings.Repeat("x", 1000), extraChars, 20, + "example.xxxx"}, {strings.Repeat("a", 1000) + ".txt", extraChars, 50, strings.Repeat("a", 46) + ".txt"}, + {strings.Repeat(".", 100) + strings.Repeat(".", 100), extraChars, 80, + strings.Repeat(".", 79) + "."}, } for _, test := range tests { t.Run(test.input, func(t *testing.T) { From 5aa2006e6bad1ca766a8fcd1ab1d3f98c7243ac2 Mon Sep 17 00:00:00 2001 From: drduh Date: Sun, 21 Dec 2025 20:43:01 -0800 Subject: [PATCH 4/4] fix typo --- handlers/message.go | 2 +- storage/sanitize_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/handlers/message.go b/handlers/message.go index d320db8..fe00116 100644 --- a/handlers/message.go +++ b/handlers/message.go @@ -49,7 +49,7 @@ func Message(app *config.App) http.HandlerFunc { } if r.URL.Query().Get("download") == "all" { - app.Log.Debug("dowloading messages", + app.Log.Debug("downloading messages", "count", app.NumMessages, "user", req) app.ServeMessages(w) return diff --git a/storage/sanitize_test.go b/storage/sanitize_test.go index 5bca527..8ab5f5f 100644 --- a/storage/sanitize_test.go +++ b/storage/sanitize_test.go @@ -104,8 +104,8 @@ func TestSanitizeName(t *testing.T) { "example.xxxx"}, {strings.Repeat("a", 1000) + ".txt", extraChars, 50, strings.Repeat("a", 46) + ".txt"}, - {strings.Repeat(".", 100) + strings.Repeat(".", 100), extraChars, 80, - strings.Repeat(".", 79) + "."}, + {strings.Repeat(".", 200), extraChars, 80, + strings.Repeat(".", 80)}, } for _, test := range tests { t.Run(test.input, func(t *testing.T) {