From b6490d25edfe80390c3111e4a6f93f3185f8adc4 Mon Sep 17 00:00:00 2001 From: yuxf Date: Thu, 15 May 2025 10:42:29 +0800 Subject: [PATCH] caddy.go: Check whether @id is unique(#6991) --- caddy.go | 40 +++++++++++++++++--- caddytest/caddytest_test.go | 75 +++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/caddy.go b/caddy.go index d6a2ae0b3..b14097531 100644 --- a/caddy.go +++ b/caddy.go @@ -224,6 +224,10 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force idx := make(map[string]string) err = indexConfigObjects(rawCfg[rawConfigKey], "/"+rawConfigKey, idx) if err != nil { + err2 := rollbackRawCfg() + if err2 != nil { + err = errors.Join(err, err2) + } return APIError{ HTTPStatus: http.StatusInternalServerError, Err: fmt.Errorf("indexing config: %v", err), @@ -239,12 +243,10 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force // with what caddy is still running; we need to // unmarshal it again because it's likely that // pointers deep in our rawCfg map were modified - var oldCfg any - err2 := json.Unmarshal(rawCfgJSON, &oldCfg) + err2 := rollbackRawCfg() if err2 != nil { err = fmt.Errorf("%v; additionally, restoring old config: %v", err, err2) } - rawCfg[rawConfigKey] = oldCfg } return fmt.Errorf("loading new config: %v", err) @@ -261,6 +263,20 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force return nil } +func rollbackRawCfg() error { + var oldCfg any + if len(rawCfgJSON) != 0 { + err := json.Unmarshal(rawCfgJSON, &oldCfg) + if err != nil { + return err + } + rawCfg[rawConfigKey] = oldCfg + } else { + rawCfg[rawConfigKey] = nil + } + return nil +} + // readConfig traverses the current config to path // and writes its JSON encoding to out. func readConfig(path string, out io.Writer) error { @@ -268,6 +284,14 @@ func readConfig(path string, out io.Writer) error { defer rawCfgMu.RUnlock() return unsyncedConfigAccess(http.MethodGet, path, nil, out) } +func writeToIdIndex(index map[string]string, key, val string) error { + _, found := index[key] + if found { + return errors.New("multiple keys found: " + key) + } + index[key] = val + return nil +} // indexConfigObjects recursively searches ptr for object fields named // "@id" and maps that ID value to the full configPath in the index. @@ -280,9 +304,15 @@ func indexConfigObjects(ptr any, configPath string, index map[string]string) err if k == idKey { switch idVal := v.(type) { case string: - index[idVal] = configPath + err := writeToIdIndex(index, idVal, configPath) + if err != nil { + return err + } case float64: // all JSON numbers decode as float64 - index[fmt.Sprintf("%v", idVal)] = configPath + err := writeToIdIndex(index, fmt.Sprintf("%v", idVal), configPath) + if err != nil { + return err + } default: return fmt.Errorf("%s: %s field must be a string or number", configPath, idKey) } diff --git a/caddytest/caddytest_test.go b/caddytest/caddytest_test.go index a9d5da936..6a8d4483d 100644 --- a/caddytest/caddytest_test.go +++ b/caddytest/caddytest_test.go @@ -1,6 +1,7 @@ package caddytest import ( + "bytes" "net/http" "strings" "testing" @@ -126,3 +127,77 @@ func TestLoadUnorderedJSON(t *testing.T) { } tester.AssertResponseCode(req, 200) } + +func TestCheckID(t *testing.T) { + tester := NewTester(t) + tester.InitServer(`{ + "admin": { + "listen": "localhost:2999" + }, + "apps": { + "http": { + "http_port": 9080, + "servers": { + "s_server": { + "@id": "s_server", + "listen": [ + ":9080" + ], + "routes": [ + { + "handle": [ + { + "handler": "static_response", + "body": "Hello" + } + ] + } + ] + } + } + } + } + } + `, "json") + headers := []string{"Content-Type:application/json"} + sServer1 := []byte( + `{"@id":"s_server","listen":[":9080"],"routes":[{"@id":"route1","handle":[{"handler":"static_response","body":"Hello 2"}]}]}`) + tester.AssertPutResponseBody( + "http://localhost:2999/id/s_server", + headers, bytes.NewBuffer(sServer1), 409, `{"error":"[/config/apps/http/servers/s_server] key already exists: s_server"} +`) + tester.AssertPostResponseBody("http://localhost:2999/id/s_server", headers, bytes.NewBuffer(sServer1), 200, "") + + tester.AssertGetResponse("http://localhost:9080/", 200, "Hello 2") + tester.AssertPostResponseBody( + "http://localhost:2999/id/s_server", + headers, bytes.NewBuffer([]byte( + `{"@id":"s_server","listen":[":9080"],"routes":[{"@id":"route1","handle":[ + {"handler":"static_response","body":"Hello2"}],"match":[{"path":["/route_1/*"]}]}]}`)), 200, "") + + sServer2 := []byte(` +{"@id":"s_server","listen":[":9080"],"routes":[{"@id":"route1","handle":[ + {"handler":"static_response","body":"Hello2"}],"match":[{"path":["/route_1/*"]}]}]}`) + tester.AssertPatchResponseBody( + "http://localhost:2999/id/s_server", + headers, bytes.NewBuffer(sServer2), 200, "") + route2 := []byte( + `{"@id":"route2","handle": [{"handler": "static_response","body": "route2"}],"match":[{"path":["/route_2/*"]}]}`) + tester.AssertPutResponseBody( + "http://localhost:2999/id/route1", + headers, bytes.NewBuffer(route2), 200, "") + tester.AssertGetResponse("http://localhost:2999/config", 200, + `{"admin":{"listen":"localhost:2999"},"apps":{"http":{"http_port":9080,"servers":{"s_server":{"@id":"s_server","listen":[":9080"],"routes":[{"@id":"route2","handle":[{"body":"route2","handler":"static_response"}],"match":[{"path":["/route_2/*"]}]},{"@id":"route1","handle":[{"body":"Hello2","handler":"static_response"}],"match":[{"path":["/route_1/*"]}]}]}}}}} +`) + // use post or put to add one + tester.AssertPostResponseBody( + "http://localhost:2999/id/route2", + headers, bytes.NewBuffer(route2), 500, `{"error":"indexing config: multiple keys found: route2"} +`) + // use patch to update + tester.AssertPatchResponseBody( + "http://localhost:2999/id/route1", + headers, bytes.NewBuffer([]byte(`{"@id":"route1","handle":[{"handler":"static_response","body":"route1"}],"match":[{"path":["/route_1/*"]}]}`)), + 200, "") + tester.AssertGetResponse("http://localhost:9080/route_1/", 200, "route1") +}