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

Unable to delete rows in collection when using "+"characters in customId #1552

Closed
A2-NieR opened this issue Jan 9, 2023 · 5 comments
Closed

Comments

@A2-NieR
Copy link

A2-NieR commented Jan 9, 2023

When using special characters like + in a customId for a record one is later unable to delete the record via the API since trying to call this:

image

results in API calls like this:

image

Backend was hosted on Fly, the delete calls where submitted via the admin ui.
Let me know if you need more info 🙂

@A2-NieR
Copy link
Author

A2-NieR commented Jan 9, 2023

Hm weird, apparently %2B is the escaped + sign, still it results in a 404 🤔
(Creating and deleting a new record to the collection with auto id worked btw.)

@ganigeorgiev
Copy link
Member

This is caused by the url encoded characters because internally we try to match the record id with the c.PathParam("id") value, but for some reason instead of the decoded value the raw version is returned. I'm not sure whether this is an issue with echo or if it is related to golang/go#33596. I'll investigate it in more details and will add an integration test to verify the decoding.

@A2-NieR
Copy link
Author

A2-NieR commented Jan 9, 2023

Thanks, otherwise I would suggest to simply validate url safe characters:

https://stackoverflow.com/a/695469

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Jan 9, 2023

Validating just the characters is not enough since the above could happen with other user defined url parameters.

It will be fixed soon, but I'll need to first find out whether this is an issue with echo or it is just an implementation detail and we'll have to apply a fix in our code.

@ganigeorgiev
Copy link
Member

This is fixed in master and will be available with the next bugfix release sometime later this week.

It turn out that it is an implementation detail of echo but it is configurable so we just had to enable the option.

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