From 1d4bc5eb34d62cf471cb827a632a6c904522ce62 Mon Sep 17 00:00:00 2001 From: Chris Kuehl Date: Fri, 20 Sep 2024 21:44:22 -0500 Subject: [PATCH] Port the whole upload details page --- TODO | 2 + scss/details.scss | 2 +- server/assets/assets.go | 7 +--- server/config/config.go | 12 +++++- server/meta/meta.go | 21 ++++++++++ server/templates/upload-details-bk | 35 ---------------- server/templates/upload-details.html | 38 +++++++++++++++-- server/uploads/uploads.go | 46 ++++++++------------ server/uploads/uploads_test.go | 7 +++- server/uploads/uploads_x_test.go | 63 ---------------------------- server/utils/utils.go | 36 ++++++++++++++++ server/utils/utils_test.go | 63 ++++++++++++++++++++++++++++ server/views/upload.go | 26 +++++++++++- 13 files changed, 217 insertions(+), 141 deletions(-) delete mode 100644 server/templates/upload-details-bk diff --git a/TODO b/TODO index 4687980..15ecfae 100644 --- a/TODO +++ b/TODO @@ -1,2 +1,4 @@ +* Add some basic logging to storages, none of them have it. + * Also some timing information to the upload view + UploadObjects function * Test that tries to upload an HTML file and ensures it's not served as HTML. * Consolidate StoredFile and StoredHTML somehow. diff --git a/scss/details.scss b/scss/details.scss index 9eaf4b0..d049592 100644 --- a/scss/details.scss +++ b/scss/details.scss @@ -1,4 +1,4 @@ -.page-details { +.page-upload-details { .file-holder { display: flex; justify-content: center; diff --git a/server/assets/assets.go b/server/assets/assets.go index eefbe5f..a1c022f 100644 --- a/server/assets/assets.go +++ b/server/assets/assets.go @@ -13,13 +13,10 @@ import ( "github.com/chriskuehl/fluffy/server/config" ) -var mimeExtensions = []string{} - func LoadAssets(assetsFS *embed.FS) (*config.Assets, error) { assets := config.Assets{ - FS: assetsFS, - Hashes: map[string]string{}, - // MIMEExtensions is a set of all the mime extensions, without dot, e.g. "png", "jpg". + FS: assetsFS, + Hashes: map[string]string{}, MIMEExtensions: map[string]struct{}{}, } diff --git a/server/config/config.go b/server/config/config.go index 5fb6576..f7cf201 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -49,8 +49,10 @@ type StorageBackend interface { } type Assets struct { - FS *embed.FS - Hashes map[string]string + FS *embed.FS + // Hashes is a map of file paths to their SHA-256 hashes. + Hashes map[string]string + // MIMEExtensions is a set of all the mime extensions, without dot, e.g. "png", "jpg". MIMEExtensions map[string]struct{} } @@ -73,6 +75,9 @@ type Config struct { HomeURL *url.URL FileURLPattern *url.URL HTMLURLPattern *url.URL + // ForbiddenFileExtensions is the set of file extensions that are not allowed to be uploaded. + // The extensions should not start with a dot, but may contain one if trying to match multiple + // extensions, e.g. "tar.gz". ForbiddenFileExtensions map[string]struct{} Host string Port uint @@ -126,6 +131,9 @@ func (conf *Config) Validate() []string { if strings.HasPrefix(ext, ".") { errs = append(errs, "ForbiddenFileExtensions should not start with a dot: "+ext) } + if strings.ToLower(ext) != ext { + errs = append(errs, "ForbiddenFileExtensions should be lowercase: "+ext) + } } if conf.Version == "" { errs = append(errs, "Version must not be empty") diff --git a/server/meta/meta.go b/server/meta/meta.go index 6d6d69b..97dc553 100644 --- a/server/meta/meta.go +++ b/server/meta/meta.go @@ -10,6 +10,7 @@ import ( "github.com/chriskuehl/fluffy/server/assets" "github.com/chriskuehl/fluffy/server/config" "github.com/chriskuehl/fluffy/server/security" + "github.com/chriskuehl/fluffy/server/utils" ) type PageConfig struct { @@ -67,3 +68,23 @@ func (m Meta) AssetURL(path string) string { } return url } + +func (m Meta) MIMEIcon(filename string) string { + // Try "file.tar.gz" => "tar.gz" => "gz" => "unknown". + parts := strings.Split(filename, ".") + for i := 0; i < len(parts); i++ { + ext := strings.Join(parts[i:], ".") + if _, ok := m.Conf.Assets.MIMEExtensions[ext]; ok { + return ext + } + } + return "unknown" +} + +func (m Meta) MIMEIconSmallURL(iconName string) string { + return m.AssetURL("img/mime/small/" + iconName + ".png") +} + +func (m Meta) FormatBytes(bytes int64) string { + return utils.FormatBytes(bytes) +} diff --git a/server/templates/upload-details-bk b/server/templates/upload-details-bk deleted file mode 100644 index 3dc0cdd..0000000 --- a/server/templates/upload-details-bk +++ /dev/null @@ -1,35 +0,0 @@ -
- {% for (file, pb) in uploads %} -
-
-
- - {{file.human_name}} -
- - {% if file.is_image %} -
- - - -
- {% endif %} - -
-
- {{file.num_bytes|human_size}} -
- -
- Direct Link - {% if pb %} - View Text - {% endif %} -
- -
-
-
-
- {% endfor %} -
diff --git a/server/templates/upload-details.html b/server/templates/upload-details.html index a9e496f..09db489 100644 --- a/server/templates/upload-details.html +++ b/server/templates/upload-details.html @@ -1,10 +1,42 @@ {{define "extraHead"}}{{end}} {{define "content"}} -CONTENT HERE -{{end}} +
+ {{range .UploadedFiles}} +
+
+
+ + {{.Name}} +
+ + {{if .IsImage}} +
+ + + +
+ {{end}} + +
+
+ {{$.Meta.FormatBytes .Bytes}} +
-{{define "inlineJS"}} +
+ Direct Link + + View Text +
+ +
+
+
+
+ {{end}} +
{{end}} +{{define "inlineJS"}}{{end}} + {{template "base.html" .}} diff --git a/server/uploads/uploads.go b/server/uploads/uploads.go index 53cfe29..e988665 100644 --- a/server/uploads/uploads.go +++ b/server/uploads/uploads.go @@ -29,14 +29,6 @@ const ( var ( ErrForbiddenExtension = fmt.Errorf("forbidden extension") - // Extensions that traditionally wrap another file extension. - wrapperExtensions = map[string]struct{}{ - "bz2": {}, - "gz": {}, - "xz": {}, - "zst": {}, - } - // MIME types which are allowed to be presented as detected. // TODO: I think we actually only need to prevent text/html (and any HTML // variants like XHTML)? @@ -63,6 +55,14 @@ var ( "image/", "video/", } + imageMIMEAllowlist = map[string]struct{}{ + "image/gif": {}, + "image/jpeg": {}, + "image/png": {}, + "image/svg+xml": {}, + "image/tiff": {}, + "image/webp": {}, + } ) // GenUniqueObjectKey returns a random string for use as object key. @@ -81,23 +81,6 @@ func GenUniqueObjectKey() (string, error) { return s.String(), nil } -func extractExtension(name string) string { - fullExt := "" - for strings.Contains(name, ".") { - ext := filepath.Ext(name) - name = strings.TrimSuffix(name, ext) - if ext == "." { - // Don't add ".", but keep processing any additional extensions. - continue - } - fullExt = ext + fullExt - if _, ok := wrapperExtensions[strings.TrimPrefix(ext, ".")]; !ok { - return fullExt - } - } - return fullExt -} - type SanitizedKey struct { UniqueID string Extension string @@ -114,15 +97,15 @@ func SanitizeUploadName(name string, forbiddenExtensions map[string]struct{}) (* if err != nil { return nil, fmt.Errorf("generating unique object key: %w", err) } - ext := extractExtension(name) - for _, extPart := range strings.Split(ext, ".") { - if _, ok := forbiddenExtensions[extPart]; ok { + lowercaseName := strings.ToLower(name) + for ext := range forbiddenExtensions { + if strings.HasSuffix(lowercaseName, "."+ext) || strings.Contains(lowercaseName, "."+ext+".") { return nil, ErrForbiddenExtension } } return &SanitizedKey{ UniqueID: id, - Extension: ext, + Extension: utils.HumanFileExtension(name), }, nil } @@ -266,6 +249,11 @@ func isInlineDisplayMIME(mimeType string) bool { return false } +func IsImageMIME(mimeType string) bool { + _, ok := imageMIMEAllowlist[mimeType] + return ok +} + func DetermineContentDisposition(filename string, mimeType string, probablyText bool) string { renderType := "attachment" if probablyText || isInlineDisplayMIME(mimeType) { diff --git a/server/uploads/uploads_test.go b/server/uploads/uploads_test.go index f8d5a6d..06936b6 100644 --- a/server/uploads/uploads_test.go +++ b/server/uploads/uploads_test.go @@ -98,6 +98,11 @@ func TestSanitizeUploadName(t *testing.T) { in: "file.exe", wantErr: uploads.ErrForbiddenExtension, }, + { + name: "forbidden extension with caps", + in: "file.EXE", + wantErr: uploads.ErrForbiddenExtension, + }, { name: "forbidden extension before wrapped extension", in: "file.exe.gz", @@ -105,7 +110,7 @@ func TestSanitizeUploadName(t *testing.T) { }, { name: "forbidden extension before wrapped extension with ..", - in: "file.exe..gz", + in: "file.Exe..gz", wantErr: uploads.ErrForbiddenExtension, }, } diff --git a/server/uploads/uploads_x_test.go b/server/uploads/uploads_x_test.go index 591f96c..37d1ca9 100644 --- a/server/uploads/uploads_x_test.go +++ b/server/uploads/uploads_x_test.go @@ -15,69 +15,6 @@ func TestGetUniqueObjectKey(t *testing.T) { } } -func TestExtractExtension(t *testing.T) { - tests := []struct { - name string - in string - want string - wantErr error - }{ - { - name: "no extension", - in: "file", - want: "", - }, - { - name: "regular extension", - in: "file.txt", - want: ".txt", - }, - { - name: "wrapped extension only", - in: "file.gz", - want: ".gz", - }, - { - name: "wrapped extension after regular extension", - in: "file.tar.gz", - want: ".tar.gz", - }, - { - name: "multiple wrapped extensions", - in: "file.tar.gz.bz2", - want: ".tar.gz.bz2", - }, - { - name: "multiple wrapped extensions with a regular extension", - in: "file.txt.tar.gz.bz2", - want: ".tar.gz.bz2", - }, - { - // Kind of nonsense, just making sure it doesn't remove more than it should. - name: "wrapped extensions before regular extension", - in: "file.tar.gz.txt", - want: ".txt", - }, - { - name: ". only", - in: ".", - want: "", - }, - { - name: "multiple wrapped extensions with empty extensions", - in: "file.txt.tar.gz....bz2", - want: ".tar.gz.bz2", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := extractExtension(tt.in); got != tt.want { - t.Errorf("got extractExtension(%q) = %q, want %q", tt.in, got, tt.want) - } - }) - } -} - func TestIsAllowedMIMEType(t *testing.T) { tests := map[string]bool{ "application/javascript": true, diff --git a/server/utils/utils.go b/server/utils/utils.go index b257d25..8197a44 100644 --- a/server/utils/utils.go +++ b/server/utils/utils.go @@ -3,8 +3,44 @@ package utils import ( "fmt" "io" + "path/filepath" + "strings" ) +// Extensions that traditionally wrap another file extension. +var wrapperExtensions = map[string]struct{}{ + "bz2": {}, + "gz": {}, + "xz": {}, + "zst": {}, +} + +// HumanFileExtension returns the "human" file extension of a file name. +// +// This function tries to mimic what a human would consider the file extension rather than always +// returning just the last extension. For example, "file.tar.gz" would return ".tar.gz" instead of +// just ".gz". +// +// Files with no extension will return an empty string. +// +// This function should not be used for any kind of validation purposes. +func HumanFileExtension(filename string) string { + fullExt := "" + for strings.Contains(filename, ".") { + ext := filepath.Ext(filename) + filename = strings.TrimSuffix(filename, ext) + if ext == "." { + // Don't add ".", but keep processing any additional extensions. + continue + } + fullExt = ext + fullExt + if _, ok := wrapperExtensions[strings.TrimPrefix(ext, ".")]; !ok { + return fullExt + } + } + return fullExt +} + func Pluralize(s string, n int64) string { if n == 1 { return s diff --git a/server/utils/utils_test.go b/server/utils/utils_test.go index 3b2ec6c..ce7b6ec 100644 --- a/server/utils/utils_test.go +++ b/server/utils/utils_test.go @@ -7,6 +7,69 @@ import ( "github.com/chriskuehl/fluffy/server/utils" ) +func TestHumanFileExtension(t *testing.T) { + tests := []struct { + name string + in string + want string + wantErr error + }{ + { + name: "no extension", + in: "file", + want: "", + }, + { + name: "regular extension", + in: "file.txt", + want: ".txt", + }, + { + name: "wrapped extension only", + in: "file.gz", + want: ".gz", + }, + { + name: "wrapped extension after regular extension", + in: "file.tar.gz", + want: ".tar.gz", + }, + { + name: "multiple wrapped extensions", + in: "file.tar.gz.bz2", + want: ".tar.gz.bz2", + }, + { + name: "multiple wrapped extensions with a regular extension", + in: "file.txt.tar.gz.bz2", + want: ".tar.gz.bz2", + }, + { + // Kind of nonsense, just making sure it doesn't remove more than it should. + name: "wrapped extensions before regular extension", + in: "file.tar.gz.txt", + want: ".txt", + }, + { + name: ". only", + in: ".", + want: "", + }, + { + name: "multiple wrapped extensions with empty extensions", + in: "file.txt.tar.gz....bz2", + want: ".tar.gz.bz2", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := utils.HumanFileExtension(tt.in); got != tt.want { + t.Errorf("%q: got %q, want %q", tt.in, got, tt.want) + } + }) + } +} + func TestPluralize(t *testing.T) { tests := map[int64]string{ 0: "things", diff --git a/server/views/upload.go b/server/views/upload.go index acd9fa3..9713c6f 100644 --- a/server/views/upload.go +++ b/server/views/upload.go @@ -205,10 +205,32 @@ func HandleUpload(conf *config.Config, logger logging.Logger) http.HandlerFunc { } var uploadDetails bytes.Buffer + + type uploadedFileData struct { + config.StoredFile + IsImage bool + Bytes int64 + } uploadDetailsData := struct { - Meta *meta.Meta + Meta *meta.Meta + UploadedFiles []uploadedFileData }{ - Meta: uploadDetailsMeta, + Meta: uploadDetailsMeta, + UploadedFiles: make([]uploadedFileData, 0, len(files)), + } + for _, file := range files { + // TODO: this is ridiculous, we've done this a bajillion times now + bytes, err := utils.FileSizeBytes(file) + if err != nil { + logger.Error(r.Context(), "getting file size", "error", err) + userError{http.StatusInternalServerError, "Failed to get file size."}.output(w) + return + } + uploadDetailsData.UploadedFiles = append(uploadDetailsData.UploadedFiles, uploadedFileData{ + StoredFile: file, + IsImage: uploads.IsImageMIME(file.MIMEType()), + Bytes: bytes, + }) } if err := uploadDetailsTmpl.ExecuteTemplate(&uploadDetails, "upload-details.html", uploadDetailsData); err != nil { logger.Error(r.Context(), "executing template", "error", err)