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

Prevent illegal ArangoDB Collection/Graph names #31

Open
jjnesbitt opened this issue May 20, 2021 · 3 comments
Open

Prevent illegal ArangoDB Collection/Graph names #31

jjnesbitt opened this issue May 20, 2021 · 3 comments

Comments

@jjnesbitt
Copy link
Member

When creating a table/network in Multinet, we don't currently check the supplied names for their validity against Arango's naming conventions. Due to this, it's currently possible to supply an invalid name, and cause a 500 error to occur when collection/graph creation is attempted.

We should instead enforce a regex constraint on these names. One way to do this would be at the serializer level, creating a custom RegexField for table and network names. Another is to create a RegexValidator, which can be attached to the name field on tables and networks. That would require calling full_clean before saving each object though, and might not lead to a great API surface, but would enforce these constraints before saving to the database. Perhaps both could be used.

@waxlamp
Copy link
Collaborator

waxlamp commented May 20, 2021

Why not enforce this at creation time instead of serialization (I may not be understanding the problem properly).

@jjnesbitt
Copy link
Member Author

Why not enforce this at creation time instead of serialization (I may not be understanding the problem properly).

In either case, creation with an invalid name can be done with the model directly, as calling model.full_clean before calling model.save is required to trigger the validation. When done at the serializer level, the serializer will prevent creation (because the request will immediately return), as well as provide a useful error message from the API. If validation is only done on the model itself, we wouldn't have a good error message to show the user.

I think in either case we should do validation in the serializer. However I'm not sure if it's worthwhile to also do this check upon model creation, as that would provide marginally better protection against this, but will probably also increase the complexity of creating a table/network.

@waxlamp
Copy link
Collaborator

waxlamp commented May 20, 2021

Thanks for the explainer. If you want to expound upon the tradeoffs, an RFC might be a good venue for it.

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

No branches or pull requests

2 participants