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

JSON with a dot in it's key is not parsed correctly #1173

Closed
tomaskikutis opened this issue Jul 18, 2018 · 8 comments
Closed

JSON with a dot in it's key is not parsed correctly #1173

tomaskikutis opened this issue Jul 18, 2018 · 8 comments
Milestone

Comments

@tomaskikutis
Copy link

@tomaskikutis tomaskikutis commented Jul 18, 2018

When sending a request with JSON payload having a dot in the key, eve creates a sub dictionary:
{"hello.world": true} parses to {"hello": {"world": true}}

Environment: Ubuntu 16.04, Python 3.6.5, Eve 0.7.8

@nicolaiarocci
Copy link
Member

@nicolaiarocci nicolaiarocci commented Aug 7, 2018

You normally are not allowed to have dotted fields in the schema, Eve would raise SchemaException on startup, see https://github.com/pyeve/eve/blob/master/eve/flaskapp.py#L473

Can you add some more details on how to reproduce this issue? Thanks.

Loading

@tomaskikutis
Copy link
Author

@tomaskikutis tomaskikutis commented Aug 7, 2018

You can see the schema in use here. There is a field content which is a dict and inside that dict I'm putting the dotted keys.

To reproduce the issue try sending {"hello.world": true} to any Eve endpoint and you should get the same result I posted.

Looks like JSON spec allows dots and dollar signs, shouldn't Eve support that as well if it's a JSON based API?

Loading

@nicolaiarocci
Copy link
Member

@nicolaiarocci nicolaiarocci commented Aug 8, 2018

  • Mongo (pymongo) will raise bson.errors.InvalidDocument: key 'hello.world' must not contain '.' if you try to store such a document.
  • To circumvent that, we could escape the dot (hello\uff0eworld) but then, all queries should be executed against the escaped field name (for reference, see https://stackoverflow.com/a/16571347/323269). Don't think it would worth the penalty hit and the added complexity.
  • Are you on a data layer other than mongo?

Loading

@tomaskikutis
Copy link
Author

@tomaskikutis tomaskikutis commented Aug 8, 2018

I didn't try writing {"hello.world": true}. When I'd send {"hello.world": true} to an endpoint as a request payload, it would be received and written as {"hello": {"world": true}}. That explains why I didn't get any errors.

It may be a valid decision to not allow writing keys containing dots to a mongo database, but I'd rather have an error raised than my payload corrupted and written to the database.

Loading

@nicolaiarocci
Copy link
Member

@nicolaiarocci nicolaiarocci commented Aug 8, 2018

Well, Eve does its best to let you know (again, https://github.com/pyeve/eve/blob/master/eve/flaskapp.py#L473). In your specific use case the problem is that you define a dict with no schema, so there's no way for Eve to let you know beforehand.

One approach we could take is only 'normalize' dotted fields on PATCH requests (I should double-check but we're likely doing that to properly handle edit operations). This way, if you POST a dotted field in a case like yours, you get the Mongo exception.

Loading

@tomaskikutis
Copy link
Author

@tomaskikutis tomaskikutis commented Aug 8, 2018

In your specific use case the problem is that you define a dict with no schema

I have user generated sub-dicts, so can't have a hardcoded schema.

One approach we could take is only 'normalize' dotted fields on PATCH requests

Don't do that, it will cause a lot of headaches for many people. I don't like normalization at all, but I understand that it may be useful for others, so I think the best thing would be to have a config for normalization = yes | no | only-with-specific-header.

Loading

@nicolaiarocci nicolaiarocci added this to the 0.81 milestone Aug 8, 2018
@nicolaiarocci nicolaiarocci removed this from the 0.81 milestone Aug 8, 2018
@nicolaiarocci nicolaiarocci added this to the 0.8.1 milestone Aug 8, 2018
@nicolaiarocci
Copy link
Member

@nicolaiarocci nicolaiarocci commented Aug 8, 2018

Assume we add a NORMALIZE_DOTTED_FIELDS option which defaults to True to preserve current behavior. By setting it to False, you would also lose the ability to edit (sub)documents via 'dotted' notation, mongo-style ("location.city": "a new city"). Would that be ok with your use case?

A more fine-grained option would be NORMALIZE_DOTTED_FIELDS_ON_INSERTS, of course.

Loading

@tomaskikutis
Copy link
Author

@tomaskikutis tomaskikutis commented Aug 9, 2018

NORMALIZE_DOTTED_FIELDS would work, but as you mentioned that way you would lose the ability to use the "dotted notation". I'd prefer a granularity level of endpoint or request while not discriminating on operation type(update,create).

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants