mirror of
https://github.com/zap-proto/zip.git
synced 2026-10-19 15:38:57 +00:00
zooqueenandHanzo Dev
d96b45b06b
Remote.Invoke percent-encodes every path argument (urlEscape) precisely so that
what a caller passes as a value cannot become part of the path. fasthttp then
undid it: its default serialisation is appendQuotedPath(u.Path()), and Path()
has already decoded %2F and resolved "..".
Invoke(GET /v1/platform/apps/:app, {"app": "../../../v1/iam/users"})
→ GET /v1/iam/users
One operation's ARGUMENT addressing a different operation entirely, on whatever
credential the caller attached. Every decision made above this line — which
command was named, whether that command may run, whether it mutates — is made
about a request that is not the one sent.
Two settings, because two things have to be told and only one of them is read
per transport:
Remote.do sets it on the REQUEST, so RequestURI() serialises PathOriginal() —
the bytes Invoke built. This is what carries the property over ZAP (the
default transport) and anything RegisterTransport adds.
transport.go sets it on the http and https HostClients, because
HostClient.doNonNilReqResp ASSIGNS req.URI().DisablePathNormalizing from its
own field (fasthttp client.go:1631). On those two transports the request's own
setting is overwritten, and the client's is the only one that survives. The
request-level line alone measurably does not hold — that is how this was found.
Pinned on a live server across the cases that matter: a plain id arrives
unchanged, "../../.." arrives escaped as one segment, a bare "/" arrives as
%2F, a pre-encoded %2F arrives as %252F rather than collapsing a level per hop,
and a bodyless op's flags still ride the query string untouched.
Co-authored-by: Hanzo Dev <[email protected]>
75 lines
2.2 KiB
Go
75 lines
2.2 KiB
Go
package zip_test
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/zap-proto/zip"
|
|
)
|
|
|
|
// A COMMAND ADDRESSES ONE OPERATION, and its arguments may not change which.
|
|
//
|
|
// Invoke percent-encodes every path argument for exactly this reason, but the
|
|
// encoding is only worth as much as the transport that carries it: fasthttp's
|
|
// default serialisation re-reads the path through Path(), which decodes %2F and
|
|
// resolves "..", so an argument could retarget the request at any route the
|
|
// method reached. do() disables that normalising; this is the pin.
|
|
func TestRemoteInvoke_ArgumentsCannotRetargetThePath(t *testing.T) {
|
|
var got string
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
got = r.URL.EscapedPath()
|
|
_, _ = w.Write([]byte(`{}`))
|
|
}))
|
|
defer srv.Close()
|
|
|
|
cmd := zip.Command{
|
|
Service: "platform", Name: "apps-get",
|
|
Method: http.MethodGet, Path: "/v1/platform/apps/:app",
|
|
Args: []zip.Arg{{Name: "app"}},
|
|
}
|
|
remote := zip.Remote{Base: srv.URL}
|
|
|
|
cases := []struct {
|
|
arg string
|
|
want string
|
|
}{
|
|
{"web", "/v1/platform/apps/web"},
|
|
{"../../../v1/iam/users", "/v1/platform/apps/..%2F..%2F..%2Fv1%2Fiam%2Fusers"},
|
|
{"a/b", "/v1/platform/apps/a%2Fb"},
|
|
{"a%2Fb", "/v1/platform/apps/a%252Fb"},
|
|
{"..", "/v1/platform/apps/.."},
|
|
}
|
|
for _, c := range cases {
|
|
if _, err := remote.Invoke(context.Background(), cmd, map[string]string{"app": c.arg}, nil); err != nil {
|
|
t.Fatalf("arg %q: %v", c.arg, err)
|
|
}
|
|
if got != c.want {
|
|
t.Errorf("arg %q reached %q, want %q", c.arg, got, c.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
// The query half is unaffected: a bodyless op's flags still ride the URL.
|
|
func TestRemoteInvoke_QuerySurvives(t *testing.T) {
|
|
var got string
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
got = r.URL.EscapedPath() + "?" + r.URL.RawQuery
|
|
_, _ = w.Write([]byte(`{}`))
|
|
}))
|
|
defer srv.Close()
|
|
|
|
cmd := zip.Command{
|
|
Service: "o11y", Name: "logs-list",
|
|
Method: http.MethodGet, Path: "/v1/o11y/logs",
|
|
}
|
|
remote := zip.Remote{Base: srv.URL}
|
|
if _, err := remote.Invoke(context.Background(), cmd, nil, []byte(`{"limit":5}`)); err != nil {
|
|
t.Fatalf("invoke: %v", err)
|
|
}
|
|
if got != "/v1/o11y/logs?limit=5" {
|
|
t.Fatalf("reached %q", got)
|
|
}
|
|
}
|