PostgREST / postgrest

REST API for any Postgres database
https://postgrest.org
MIT License
23.2k stars 1.02k forks source link

The `@` symbol is not accepted as a json key #3054

Closed steve-chavez closed 9 months ago

steve-chavez commented 10 months ago

Problem

See:

curl 'localhost:3000/bets?data_json->@type=eq.4'

{"code":"PGRST100","details":"unexpected \"@\" expecting \"-\", digit or field name (* or [a..z0..9_$])","hint":null,"message":"\"failed to parse tree path (data_json->@type)\" (line 1, column 12)"}

And @type is a valid json key:

select ('{"a": {"@type": 2}}'::jsonb)->'a'->'@type';
 ?column? 
----------
 2
(1 row)

Solution

Accept @. The relevant lines:

https://github.com/PostgREST/postgrest/blob/a8387e0ad3860e0c6d956fdae9554745e4453f6d/src/PostgREST/ApiRequest/QueryParams.hs#L407-L408

https://github.com/PostgREST/postgrest/blob/a8387e0ad3860e0c6d956fdae9554745e4453f6d/src/PostgREST/ApiRequest/QueryParams.hs#L349-L358

https://github.com/PostgREST/postgrest/blob/a8387e0ad3860e0c6d956fdae9554745e4453f6d/src/PostgREST/ApiRequest/QueryParams.hs#L799-L803

The json path parser is using pIdentifier for json keys , which restrics the chars to a postgres identifier. It shouldn't use that.

runeksvendsen commented 9 months ago

Thanks for carving this out. Looks like fun first issue.

The json path parser is using pIdentifier for json keys , which restrics the chars to a postgres identifier. It shouldn't use that.

So it shouldn't use pIdentifier, but what should it use?

There seems to be some restrictions on the pIdentifier parser — e.g. pIdentifier `sepBy1` dash is used, so pIdentifier can't also parse dashes.
Or should dashes be supported provided they're escaped somehow? If so, how is escaping performed?

In short, which part of the stack defines which characters should be supported in a JSON key? The JSON spec, Postgrest, PostgreSQL, or something else?

laurenceisla commented 9 months ago

So it shouldn't use pIdentifier, but what should it use?

I think it should be a new one (needs to be created).

[...] so pIdentifier can't also parse dashes. Or should dashes be supported provided they're escaped somehow? If so, how is escaping performed?

Yeah, it cannot, but it's allowed in the pFieldName (it adds it back with T.intercalate "-"). In the doctest, you can see that it's allowed even without escaping.

https://github.com/PostgREST/postgrest/blob/a8387e0ad3860e0c6d956fdae9554745e4453f6d/src/PostgREST/ApiRequest/QueryParams.hs#L324-L325

In short, which part of the stack defines which characters should be supported in a JSON key? The JSON spec, Postgrest, PostgreSQL, or something else?

We shoud go with the PostgeSQL spec, it mentions that it separates itself from the RFC 7159 many times. But i didn't find exactly what is or isn't allowed in keys (didn't look too thoroughly though), maybe we should allow anyChar? Doing something like this works, for instance:

select '{"-}{/.?>!@_a-#ñµ": 2, "b": 4}'::json->'-}{/.?>!@_a-#ñµ' as col;
col
---
2