mirror of
https://github.com/caddyserver/caddy.git
synced 2026-03-17 14:34:03 +00:00
core: Check whether @id is unique (#7002)
* caddy.go: Check whether @id is unique(#6991) * Alternate implementation, using Gemini 3.1 --------- Co-authored-by: Francis Lavoie <lavofr@gmail.com>
This commit is contained in:
Salent Olivick
23
caddy.go
23
caddy.go
@@ -227,8 +227,18 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force
|
||||
idx := make(map[string]string)
|
||||
err = indexConfigObjects(rawCfg[rawConfigKey], "/"+rawConfigKey, idx)
|
||||
if err != nil {
|
||||
if len(rawCfgJSON) > 0 {
|
||||
var oldCfg any
|
||||
err2 := json.Unmarshal(rawCfgJSON, &oldCfg)
|
||||
if err2 != nil {
|
||||
err = fmt.Errorf("%v; additionally, restoring old config: %v", err, err2)
|
||||
}
|
||||
rawCfg[rawConfigKey] = oldCfg
|
||||
} else {
|
||||
rawCfg[rawConfigKey] = nil
|
||||
}
|
||||
return APIError{
|
||||
HTTPStatus: http.StatusInternalServerError,
|
||||
HTTPStatus: http.StatusBadRequest,
|
||||
Err: fmt.Errorf("indexing config: %v", err),
|
||||
}
|
||||
}
|
||||
@@ -248,6 +258,8 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force
|
||||
err = fmt.Errorf("%v; additionally, restoring old config: %v", err, err2)
|
||||
}
|
||||
rawCfg[rawConfigKey] = oldCfg
|
||||
} else {
|
||||
rawCfg[rawConfigKey] = nil
|
||||
}
|
||||
|
||||
return fmt.Errorf("loading new config: %v", err)
|
||||
@@ -281,14 +293,19 @@ func indexConfigObjects(ptr any, configPath string, index map[string]string) err
|
||||
case map[string]any:
|
||||
for k, v := range val {
|
||||
if k == idKey {
|
||||
var idStr string
|
||||
switch idVal := v.(type) {
|
||||
case string:
|
||||
index[idVal] = configPath
|
||||
idStr = idVal
|
||||
case float64: // all JSON numbers decode as float64
|
||||
index[fmt.Sprintf("%v", idVal)] = configPath
|
||||
idStr = fmt.Sprintf("%v", idVal)
|
||||
default:
|
||||
return fmt.Errorf("%s: %s field must be a string or number", configPath, idKey)
|
||||
}
|
||||
if existingPath, ok := index[idStr]; ok {
|
||||
return fmt.Errorf("duplicate ID '%s' found at %s and %s", idStr, existingPath, configPath)
|
||||
}
|
||||
index[idStr] = configPath
|
||||
continue
|
||||
}
|
||||
// traverse this object property recursively
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package caddytest
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -126,3 +127,118 @@ 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"}]}]}`)
|
||||
|
||||
// PUT to an existing ID should fail with a 409 conflict
|
||||
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"}`+"\n")
|
||||
|
||||
// POST replaces the object fully
|
||||
tester.AssertPostResponseBody(
|
||||
"http://localhost:2999/id/s_server",
|
||||
headers,
|
||||
bytes.NewBuffer(sServer1),
|
||||
200,
|
||||
"")
|
||||
|
||||
// Verify the server is running the new route
|
||||
tester.AssertGetResponse(
|
||||
"http://localhost:9080/",
|
||||
200,
|
||||
"Hello 2")
|
||||
|
||||
// Update the existing route to ensure IDs are handled correctly when replaced
|
||||
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/*"]}]}]}`)
|
||||
|
||||
// Identical patch should succeed and return 200 (config is unchanged branch)
|
||||
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/*"]}]}`)
|
||||
|
||||
// Put a new route2 object before the route1 object due to the path of /id/route1
|
||||
// Being translated to: /config/apps/http/servers/s_server/routes/0
|
||||
tester.AssertPutResponseBody(
|
||||
"http://localhost:2999/id/route1",
|
||||
headers,
|
||||
bytes.NewBuffer(route2),
|
||||
200,
|
||||
"")
|
||||
|
||||
// Verify that the whole config looks correct, now containing both route1 and route2
|
||||
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/*"]}]}]}}}}}`+"\n")
|
||||
|
||||
// Try to add another copy of route2 using POST to test duplicate ID handling
|
||||
// Since the first route2 ended up at array index 0, and we are appending to the array, the index for the new element would be 2
|
||||
tester.AssertPostResponseBody(
|
||||
"http://localhost:2999/id/route2",
|
||||
headers,
|
||||
bytes.NewBuffer(route2),
|
||||
400,
|
||||
`{"error":"indexing config: duplicate ID 'route2' found at /config/apps/http/servers/s_server/routes/0 and /config/apps/http/servers/s_server/routes/2"}`+"\n")
|
||||
|
||||
// Use PATCH to modify an existing object successfully
|
||||
tester.AssertPatchResponseBody(
|
||||
"http://localhost:2999/id/route1",
|
||||
headers,
|
||||
bytes.NewBuffer([]byte(`{"@id":"route1","handle":[{"handler":"static_response","body":"route1"}],"match":[{"path":["/route_1/*"]}]}`)),
|
||||
200,
|
||||
"")
|
||||
|
||||
// Verify the PATCH updated the server state
|
||||
tester.AssertGetResponse(
|
||||
"http://localhost:9080/route_1/",
|
||||
200,
|
||||
"route1")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user