search

balena-io / pinejs

Generate rest APIs from natural language models
Apache License 2.0
63 stars 11 forks source link

OData query params URI decoding issues on "complex URLs" #693

Closed shaunco closed 1 year ago

shaunco commented 1 year ago

Most http libraries automatically handle URI encoding of query parameters. It would be expected that PineJS is able to accept URI encoded query parameters and decode them properly, however there seem to be some weird inconsistencies about when things need to be encoded and when they don't. For example, in open-balena-api, this request gets a Malformed url response:

/v6/device_type?%24filter=device_type_alias%2Fany(dta:dta/is_referenced_by__alias+eq+%27generic-amd64%27)&%24orderby=name+asc&%24select=id&%24top=1

whereas this works fine:

/v6/device_type?%24filter=device_type_alias/any(dta:dta/is_referenced_by__alias+eq+%27generic-amd64%27)&%24orderby=name+asc&%24select=id&%24top=1

The difference between the two is the / in device_type_alias/any being encoded as %2f. In fact, encoding any of /, (, ), or : in the filter value results in a Malformed url response but ' can be encoded as %27 and spaces as + or %20 without issue (or even not encoded), for example all of these fail:

/v6/device_type?%24filter=device_type_alias%2Fany%28dta%3Adta%2Fis_referenced_by__alias+eq+%27generic-amd64%27%29&%24orderby=name+asc&%24select=id&%24top=1
/v6/device_type?%24filter=device_type_alias/any%28dta%3Adta/is_referenced_by__alias+eq+%27generic-amd64%27%29&%24orderby=name+asc&%24select=id&%24top=1
/v6/device_type?%24filter=device_type_alias/any(dta%3Adta/is_referenced_by__alias+eq+%27generic-amd64%27)&%24orderby=name+asc&%24select=id&%24top=1
/v6/device_type?%24filter=device_type_alias/any%28dta:dta/is_referenced_by__alias+eq+%27generic-amd64%27%29&%24orderby=name+asc&%24select=id&%24top=1

I could be wrong, but the issue seems to be that this complex url check happens BEFORE a call to decodeURIComponent: https://github.com/balena-io/pinejs/blob/ca446d988dc0f6fb4e55f0bd23b545ec3b5ab618/src/sbvr-api/uri-parser.ts#L100

This means that when the /, (, ), or : are URI encoded, it skips straight to line 137 and calls parseOdata without ever calling decodeURIComponent: https://github.com/balena-io/pinejs/blob/ca446d988dc0f6fb4e55f0bd23b545ec3b5ab618/src/sbvr-api/uri-parser.ts#L137

On the other path, inside the if statement, OData parsing is done with the URI decoded params: https://github.com/balena-io/pinejs/blob/ca446d988dc0f6fb4e55f0bd23b545ec3b5ab618/src/sbvr-api/uri-parser.ts#L119-L125

shaunco commented 1 year ago

Closing in favor of https://github.com/balena-io-modules/odata-parser/issues/73