Use :path: instead of {path}

{path} gets escaped in URL parsing and makes round-tripping awkward.
chriskuehl · Sep 8, 2024 · 871fde9 · 871fde9
1 parent 37fac6a
commit 871fde9
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 29 deletions.
diff --git a/go.mod b/go.mod
@@ -3,14 +3,14 @@ module github.com/chriskuehl/fluffy
 go 1.23.0
 
 require (
+	github.com/BurntSushi/toml v1.4.0
 	github.com/adrg/xdg v0.5.0
 	github.com/google/go-cmp v0.6.0
 	github.com/spf13/cobra v1.8.1
 	golang.org/x/term v0.23.0
 )
 
 require (
-	github.com/BurntSushi/toml v1.4.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	golang.org/x/sys v0.24.0 // indirect

diff --git a/server/assets/assets.go b/server/assets/assets.go
@@ -80,7 +80,7 @@ func AssetURL(conf *config.Config, path string) (string, error) {
 		return "", fmt.Errorf("asset not found: %s", path)
 	}
 	url := conf.ObjectURLPattern
-	url.Path = strings.Replace(url.Path, "{path}", assetObjectPath(path, hash), -1)
+	url.Path = strings.Replace(url.Path, ":path:", assetObjectPath(path, hash), -1)
 	return url.String(), nil
 }
 

diff --git a/server/config/config.go b/server/config/config.go
@@ -52,11 +52,11 @@ func (conf *Config) Validate() []string {
 	if strings.HasSuffix(conf.HomeURL.Path, "/") {
 		errs = append(errs, "HomeURL must not end with a slash")
 	}
-	if !strings.Contains(conf.ObjectURLPattern.Path, "{path}") {
-		errs = append(errs, "ObjectURLPattern must contain a '{path}' placeholder")
+	if !strings.Contains(conf.ObjectURLPattern.Path, ":path:") {
+		errs = append(errs, "ObjectURLPattern must contain a ':path:' placeholder")
 	}
-	if !strings.Contains(conf.HTMLURLPattern.Path, "{path}") {
-		errs = append(errs, "HTMLURLPattern must contain a '{path}' placeholder")
+	if !strings.Contains(conf.HTMLURLPattern.Path, ":path:") {
+		errs = append(errs, "HTMLURLPattern must contain a ':path:' placeholder")
 	}
 	if conf.ForbiddenFileExtensions == nil {
 		errs = append(errs, "ForbiddenFileExtensions must not be nil")

diff --git a/server/config/loader/loader_test.go b/server/config/loader/loader_test.go
@@ -12,6 +12,24 @@ import (
 	"github.com/chriskuehl/fluffy/testfunc"
 )
 
+var configWithEverything = []byte(`
+branding = "foo"
+custom_footer_html = "<p>foo</p>"
+abuse_contact_email = "abuse@foo.com"
+max_upload_bytes = 123
+max_multipart_memory_bytes = 456
+home_url = "http://foo.com"
+object_url_pattern = "http://i.foo.com/o/:path:"
+html_url_pattern = "http://i.foo.com/h/:path:"
+forbidden_file_extensions = ["foo", "bar"]
+host = "192.168.1.100"
+port = 5555
+
+[filesystem_storage_backend]
+object_root = "/tmp/objects"
+html_root = "/tmp/html"
+`)
+
 func TestLoadConfigTOMLEmptyFile(t *testing.T) {
 	configPath := t.TempDir() + "/config.toml"
 	if err := os.WriteFile(configPath, []byte(""), 0644); err != nil {
@@ -30,31 +48,13 @@ func TestLoadConfigTOMLEmptyFile(t *testing.T) {
 
 func TestLoadConfigTOMLWithEverything(t *testing.T) {
 	configPath := t.TempDir() + "/config.toml"
-	if err := os.WriteFile(configPath, []byte(`
-branding = "foo"
-custom_footer_html = "<p>foo</p>"
-abuse_contact_email = "abuse@foo.com"
-max_upload_bytes = 123
-max_multipart_memory_bytes = 456
-home_url = "http://foo.com"
-object_url_pattern = "http://i.foo.com/o/{path}"
-html_url_pattern = "http://i.foo.com/h/{path}"
-forbidden_file_extensions = ["foo", "bar"]
-host = "192.168.1.100"
-port = 5555
-
-[filesystem_storage_backend]
-object_root = "/tmp/objects"
-html_root = "/tmp/html"
-`), 0644); err != nil {
+	if err := os.WriteFile(configPath, configWithEverything, 0644); err != nil {
 		t.Fatalf("failed to write file: %v", err)
 	}
-
 	conf := testfunc.NewConfig()
 	if err := LoadConfigTOML(conf, configPath); err != nil {
 		t.Fatalf("failed to load config: %v", err)
 	}
-
 	errs := conf.Validate()
 	if len(errs) != 0 {
 		t.Fatalf("config validation failed: %v", errs)
@@ -67,8 +67,8 @@ html_root = "/tmp/html"
 		MaxUploadBytes:          123,
 		MaxMultipartMemoryBytes: 456,
 		HomeURL:                 url.URL{Scheme: "http", Host: "foo.com"},
-		ObjectURLPattern:        url.URL{Scheme: "http", Host: "i.foo.com", Path: "/o/{path}", RawPath: "/o/{path}"},
-		HTMLURLPattern:          url.URL{Scheme: "http", Host: "i.foo.com", Path: "/h/{path}", RawPath: "/h/{path}"},
+		ObjectURLPattern:        url.URL{Scheme: "http", Host: "i.foo.com", Path: "/o/:path:"},
+		HTMLURLPattern:          url.URL{Scheme: "http", Host: "i.foo.com", Path: "/h/:path:"},
 		ForbiddenFileExtensions: map[string]struct{}{"foo": {}, "bar": {}},
 		Host:                    "192.168.1.100",
 		Port:                    5555,
@@ -82,3 +82,40 @@ html_root = "/tmp/html"
 		t.Fatalf("config mismatch (-want +got):\n%s", diff)
 	}
 }
+
+func TestRoundtripDumpLoadConfigTOML(t *testing.T) {
+	configPath := t.TempDir() + "/config.toml"
+	if err := os.WriteFile(configPath, configWithEverything, 0644); err != nil {
+		t.Fatalf("failed to write file: %v", err)
+	}
+	conf := testfunc.NewConfig()
+	if err := LoadConfigTOML(conf, configPath); err != nil {
+		t.Fatalf("failed to load config: %v", err)
+	}
+	errs := conf.Validate()
+	if len(errs) != 0 {
+		t.Fatalf("config validation failed: %v", errs)
+	}
+
+	newConfigPath := t.TempDir() + "/new_config.toml"
+	configText, err := DumpConfigTOML(conf)
+	if err != nil {
+		t.Fatalf("failed to dump config: %v", err)
+	}
+	if err := os.WriteFile(newConfigPath, []byte(configText), 0644); err != nil {
+		t.Fatalf("failed to write file: %v", err)
+	}
+
+	newConf := testfunc.NewConfig()
+	if err := LoadConfigTOML(newConf, newConfigPath); err != nil {
+		t.Fatalf("failed to load config: %v", err)
+	}
+	errs = newConf.Validate()
+	if len(errs) != 0 {
+		t.Fatalf("config validation failed: %v", errs)
+	}
+
+	if diff := cmp.Diff(conf, newConf); diff != "" {
+		t.Fatalf("config mismatch (-want +got):\n%s", diff)
+	}
+}
diff --git a/server/server.go b/server/server.go
@@ -29,8 +29,8 @@ func NewConfig() *config.Config {
 		MaxUploadBytes:          1024 * 1024 * 10, // 10 MiB
 		MaxMultipartMemoryBytes: 1024 * 1024 * 10, // 10 MiB
 		HomeURL:                 url.URL{Scheme: "http", Host: "localhost:8080"},
-		ObjectURLPattern:        url.URL{Scheme: "http", Host: "localhost:8080", Path: "/dev/object/{path}"},
-		HTMLURLPattern:          url.URL{Scheme: "http", Host: "localhost:8080", Path: "/dev/html/{path}"},
+		ObjectURLPattern:        url.URL{Scheme: "http", Host: "localhost:8080", Path: "/dev/object/:path:"},
+		HTMLURLPattern:          url.URL{Scheme: "http", Host: "localhost:8080", Path: "/dev/html/:path:"},
 		ForbiddenFileExtensions: make(map[string]struct{}),
 		Host:                    "127.0.0.1",
 		Port:                    8080,