Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: query/path params only allow string #3049

Merged
merged 1 commit into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions backend/controller/ingress/ingress_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ func TestHttpIngress(t *testing.T) {
assert.True(t, ok, "good_stuff is not a string: %s", repr.String(resp.JsonBody))
assert.Equal(t, "This is good stuff", goodStuff)
})},
in.SubTest{Name: "GetWithNumericQueryParams", Action: in.HttpCall(http.MethodGet, "/getquery?userId=123&postId=456", nil, nil, func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"Header from FTL"}, resp.Headers["Get"])
expectContentType(t, resp, "application/json;charset=utf-8")

message, ok := resp.JsonBody["msg"].(string)
assert.True(t, ok, "msg is not a string: %s", repr.String(resp.JsonBody))
assert.Equal(t, "UserID: 123, PostID: 456", message)

nested, ok := resp.JsonBody["nested"].(map[string]any)
assert.True(t, ok, "nested is not a map: %s", repr.String(resp.JsonBody))
goodStuff, ok := nested["good_stuff"].(string)
assert.True(t, ok, "good_stuff is not a string: %s", repr.String(resp.JsonBody))
assert.Equal(t, "This is good stuff", goodStuff)
})},
in.SubTest{Name: "PostUsers", Action: in.HttpCall(http.MethodPost, "/users", nil, in.JsonData(t, in.Obj{"userId": 123, "postId": 345}), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 201, resp.Status)
assert.Equal(t, []string{"Header from FTL"}, resp.Headers["Post"])
Expand Down
12 changes: 6 additions & 6 deletions backend/controller/ingress/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,13 @@ func TestParseQueryParams(t *testing.T) {
err string
}{
{query: "", request: obj{}},
{query: "int=1", request: obj{"int": "1"}},
{query: "float=2.2", request: obj{"float": "2.2"}},
{query: "int=1", request: obj{"int": int64(1)}},
{query: "float=2.2", request: obj{"float": float64(2.2)}},
{query: "string=test", request: obj{"string": "test"}},
{query: "bool=true", request: obj{"bool": "true"}},
{query: "array=2", request: obj{"array": []string{"2"}}},
{query: "array=10&array=11", request: obj{"array": []string{"10", "11"}}},
{query: "int=10&array=11&array=12", request: obj{"int": "10", "array": []string{"11", "12"}}},
{query: "bool=true", request: obj{"bool": true}},
{query: "array=2", request: obj{"array": []any{int64(2)}}},
{query: "array=10&array=11", request: obj{"array": []any{int64(10), int64(11)}}},
{query: "int=10&array=11&array=12", request: obj{"int": int64(10), "array": []any{int64(11), int64(12)}}},
{query: "int=1&int=2", request: nil, err: `failed to parse query parameter "int": multiple values are not supported`},
{query: "[a,b]=c", request: nil, err: "complex key \"[a,b]\" is not supported, use '@json=' instead"},
{query: "array=[1,2]", request: nil, err: `failed to parse query parameter "array": complex value "[1,2]" is not supported, use '@json=' instead`},
Expand Down
141 changes: 102 additions & 39 deletions backend/controller/ingress/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,45 +134,55 @@ func manglePathParameters(params map[string]string, ref *schema.Ref, sch *schema
return nil, err
}

switch paramsField.Type.(type) {
case *schema.Ref, *schema.Map:
switch m := paramsField.Type.(type) {
case *schema.Map:
ret := map[string]any{}
for k, v := range params {
ret[k] = v
}
return ret, nil
case *schema.Ref:
data := &schema.Data{}
err := sch.ResolveToType(m, data)
if err != nil {
return nil, fmt.Errorf("failed to resolve path parameter type: %w", err)
}
return parsePathParams(params, data)
default:
}
// This is a scalar, there should only be a single param
// This is validated by the schema, we don't need extra validation here
for _, val := range params {

switch paramsField.Type.(type) {
case *schema.String:
return val, nil
case *schema.Int:
parsed, err := strconv.ParseInt(val, 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse int from path parameter: %w", err)
}
return parsed, nil
case *schema.Float:
float, err := strconv.ParseFloat(val, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse float from path parameter: %w", err)
}
return float, nil
case *schema.Bool:
// TODO: is anything else considered truthy?
return val == "true", nil
default:
return nil, fmt.Errorf("unsupported path parameter type %T", paramsField.Type)
}
return parseScalar(paramsField.Type, val)
}
// Empty map
return map[string]any{}, nil
}

func parseScalar(paramsField schema.Type, val string) (any, error) {
switch paramsField.(type) {
case *schema.String:
return val, nil
case *schema.Int:
parsed, err := strconv.ParseInt(val, 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse int from path parameter: %w", err)
}
return parsed, nil
case *schema.Float:
float, err := strconv.ParseFloat(val, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse float from path parameter: %w", err)
}
return float, nil
case *schema.Bool:
// TODO: is anything else considered truthy?
return val == "true", nil
default:
return nil, fmt.Errorf("unsupported path parameter type %T", paramsField)
}
}

// Takes the map of query parameters and transforms them into the appropriate type
func mangleQueryParameters(params map[string]any, underlying map[string][]string, ref *schema.Ref, sch *schema.Schema) (any, error) {

Expand All @@ -181,19 +191,33 @@ func mangleQueryParameters(params map[string]any, underlying map[string][]string
return nil, err
}

if m, ok := paramsField.Type.(*schema.Map); ok {
switch m := paramsField.Type.(type) {
case *schema.Map:
if _, ok := m.Value.(*schema.Array); ok {
return params, nil
} else if _, ok := m.Value.(*schema.String); ok {
// We need to turn them into straight strings
newParams := map[string]any{}
for k, v := range underlying {
if len(v) > 0 {
newParams[k] = v[0]
}
}
return newParams, nil
}
}
// We need to turn them into straight strings
newParams := map[string]any{}
for k, v := range underlying {
if len(v) > 0 {
newParams[k] = v[0]
case *schema.Ref:
data := &schema.Data{}
err := sch.ResolveToType(m, data)
if err != nil {
return nil, fmt.Errorf("failed to resolve query parameter type: %w", err)
}
return parseQueryParams(underlying, data)
case *schema.Unit:
return params, nil
default:
}
return newParams, nil

return nil, fmt.Errorf("unsupported query parameter type %v", paramsField.Type)
}

func valueForData(typ schema.Type, data []byte) (any, error) {
Expand Down Expand Up @@ -292,7 +316,7 @@ func buildRequestMap(r *http.Request) (map[string]any, error) {
}
}

func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, error) {
func parseQueryParams(values map[string][]string, data *schema.Data) (map[string]any, error) {
if jsonStr, ok := values["@json"]; ok {
if len(values) > 1 {
return nil, fmt.Errorf("only '@json' parameter is allowed, but other parameters were found")
Expand All @@ -309,7 +333,6 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err
if hasInvalidQueryChars(key) {
return nil, fmt.Errorf("complex key %q is not supported, use '@json=' instead", key)
}

var field *schema.Field
for _, f := range data.Fields {
if jsonAlias, ok := f.Alias(schema.AliasKindJson).Get(); (ok && jsonAlias == key) || f.Name == key {
Expand All @@ -334,15 +357,40 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err
if err != nil {
return nil, fmt.Errorf("failed to parse query parameter %q: %w", key, err)
}

if v, ok := val.Get(); ok {
queryMap[key] = v
}
}

return queryMap, nil
}

func parsePathParams(values map[string]string, data *schema.Data) (map[string]any, error) {
// This is annoyingly close to the query params logic, but just disimilar enough to not be easily refactored
pathMap := make(map[string]any)
for key, value := range values {
var field *schema.Field
for _, f := range data.Fields {
if jsonAlias, ok := f.Alias(schema.AliasKindJson).Get(); (ok && jsonAlias == key) || f.Name == key {
field = f
}
}

if field == nil {
pathMap[key] = value
continue
}

val, err := valueForField(field.Type, []string{value})
if err != nil {
return nil, fmt.Errorf("failed to parse path parameter %q: %w", key, err)
}
if v, ok := val.Get(); ok {
pathMap[key] = v
}
}
return pathMap, nil
}

func valueForField(typ schema.Type, value []string) (optional.Option[any], error) {
switch t := typ.(type) {
case *schema.Bytes, *schema.Map, *schema.Time,
Expand All @@ -355,23 +403,38 @@ func valueForField(typ schema.Type, value []string) (optional.Option[any], error
if hasInvalidQueryChars(value[0]) {
return optional.None[any](), fmt.Errorf("complex value %q is not supported, use '@json=' instead", value[0])
}
return optional.Some[any](value[0]), nil

parsed, err := parseScalar(typ, value[0])
if err != nil {
return optional.None[any](), fmt.Errorf("failed to parse int from path parameter: %w", err)
}
return optional.Some[any](parsed), nil

case *schema.Array:
for _, v := range value {
if hasInvalidQueryChars(v) {
return optional.None[any](), fmt.Errorf("complex value %q is not supported, use '@json=' instead", v)
}
}
return optional.Some[any](value), nil
ret := []any{}
for _, v := range value {
field, err := valueForField(t.Element, []string{v})
if err != nil {
return optional.None[any](), fmt.Errorf("failed to parse array element: %w", err)
}
if unwrapped, ok := field.Get(); ok {
ret = append(ret, unwrapped)
}
}
return optional.Some[any](ret), nil

case *schema.Optional:
if len(value) > 0 {
return valueForField(t.Type, value)
}

default:
panic(fmt.Sprintf("unsupported type %T", typ))
return optional.None[any](), fmt.Errorf("unsupported type %T", typ)
}

return optional.Some[any](value), nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
)

type GetRequest struct {
UserID string `json:"userId,omitempty"`
PostID string `json:"postId,something,else"`
UserID int `json:"userId,omitempty"`
PostID int `json:"postId,something,else"`
}

type Nested struct {
Expand Down Expand Up @@ -42,7 +42,20 @@ func Get(ctx context.Context, req builtin.HttpRequest[ftl.Unit, GetRequest, ftl.
return builtin.HttpResponse[GetResponse, string]{
Headers: map[string][]string{"Get": {"Header from FTL"}},
Body: ftl.Some(GetResponse{
Message: fmt.Sprintf("UserID: %s, PostID: %s", req.PathParameters.UserID, req.PathParameters.PostID),
Message: fmt.Sprintf("UserID: %d, PostID: %d", req.PathParameters.UserID, req.PathParameters.PostID),
Nested: Nested{
GoodStuff: "This is good stuff",
},
}),
}, nil
}

//ftl:ingress http GET /getquery
func GetQuery(ctx context.Context, req builtin.HttpRequest[ftl.Unit, ftl.Unit, GetRequest]) (builtin.HttpResponse[GetResponse, string], error) {
return builtin.HttpResponse[GetResponse, string]{
Headers: map[string][]string{"Get": {"Header from FTL"}},
Body: ftl.Some(GetResponse{
Message: fmt.Sprintf("UserID: %d, PostID: %d", req.Query.UserID, req.Query.PostID),
Nested: Nested{
GoodStuff: "This is good stuff",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,17 @@ public class TestHTTP {
@GET
@Path("/users/{userId}/posts/{postId}")
@ResponseHeader(name = "Get", value = "Header from FTL")
public GetResponse get(@RestPath String userId, @RestPath String postId) {
public GetResponse get(@RestPath int userId, @RestPath int postId) {
return new GetResponse()
.setMsg(String.format("UserID: %s, PostID: %s", userId, postId))
.setNested(new Nested().setGoodStuff("This is good stuff"));
}


@GET
@Path("/getquery")
@ResponseHeader(name = "Get", value = "Header from FTL")
public GetResponse getquery(@RestQuery int userId, @RestQuery int postId) {
return new GetResponse()
.setMsg(String.format("UserID: %s, PostID: %s", userId, postId))
.setNested(new Nested().setGoodStuff("This is good stuff"));
Expand Down
23 changes: 20 additions & 3 deletions internal/schema/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,11 @@ func requireUnitPayloadType(n Node, r Type, v *Verb, reqOrResp string) error {

func validatePathParamsPayloadType(n Node, r Type, v *Verb, reqOrResp string) error {
switch t := n.(type) {
case *String, *Data, *Unit, *Float, *Int, *Bool, *Map: // Valid HTTP param payload types.
case *String, *Data, *Unit, *Float, *Int, *Bool: // Valid HTTP param payload types.
case *Map:
if _, ok := t.Value.(*String); !ok {
return errorf(r, "ingress verb %s: %s type %s path params can only support maps of strings, not %v", v.Name, reqOrResp, r, n)
}
case *TypeAlias:
// allow aliases of external types
for _, m := range t.Metadata {
Expand All @@ -833,7 +837,20 @@ func validatePathParamsPayloadType(n Node, r Type, v *Verb, reqOrResp string) er

func validateQueryParamsPayloadType(n Node, r Type, v *Verb, reqOrResp string) error {
switch t := n.(type) {
case *Data, *Unit, *Map: // Valid HTTP query payload types.
case *Data, *Unit: // Valid HTTP query payload types.
return nil
case *Map:
switch val := t.Value.(type) {
case *String:
// Valid HTTP query payload type
return nil
case *Array:
if _, ok := val.Element.(*String); ok {
return nil
}
default:
return errorf(r, "ingress verb %s: %s type %s query params can only support maps of strings or string arrays, not %v", v.Name, reqOrResp, r, n)
}
case *TypeAlias:
// allow aliases of external types
for _, m := range t.Metadata {
Expand All @@ -845,7 +862,7 @@ func validateQueryParamsPayloadType(n Node, r Type, v *Verb, reqOrResp string) e
default:
return errorf(r, "ingress verb %s: %s type %s must have a param of data structure, unit or map not %s", v.Name, reqOrResp, r, n)
}
return nil
return errorf(r, "ingress verb %s: %s type %s query params can only support maps of strings or string arrays, or data types not %v", v.Name, reqOrResp, r, n)
}

func validateVerbSubscriptions(module *Module, v *Verb, md *MetadataSubscriber, scopes Scopes, schema optional.Option[*Schema]) (merr []error) {
Expand Down
Loading