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

JsonType url filter is incomplete #2555

Closed
dogawaf opened this issue Nov 18, 2015 · 5 comments
Closed

JsonType url filter is incomplete #2555

dogawaf opened this issue Nov 18, 2015 · 5 comments

Comments

@dogawaf
Copy link

@dogawaf dogawaf commented Nov 18, 2015

The filter urlin JsonType fails to validate correctly url with a query string like http://example.com?foo=bar.

Here is the actual regex used: /^(https?:\/\/)?([\da-z\.-]+)\.([a-z\.]{2,6})([\/\w \.-]*)*\/?$/.
I suggest to use php filter_var builtin function, with the flag FILTER_VALIDATE_URL.

What do you think?

@dogawaf
Copy link
Author

@dogawaf dogawaf commented Nov 19, 2015

It seems that filter_var and FILTER_VALIDATE_URL validate only absolute url. So we need a custom regex to validate both relative and absolute url.

@DavertMik
Copy link
Member

@DavertMik DavertMik commented Nov 19, 2015

Url is expected to be absolute in case of JSON response. At least I'd suppose it to be this way

@dogawaf
Copy link
Author

@dogawaf dogawaf commented Nov 19, 2015

I am good (for my testcases) to validate only with absolute url, but it would be a breaking change.

@DavertMik
Copy link
Member

@DavertMik DavertMik commented Nov 19, 2015

...or a bug fixes :) Anyway, please add your change, I think it is ok. If anyone needs to validate relative url they can add their own filter :uri, for instance, and use it. Filters are really easy to add

@DavertMik DavertMik closed this in b3936fb Nov 26, 2015
@dogawaf
Copy link
Author

@dogawaf dogawaf commented Nov 27, 2015

Nice, you were faster than me :)

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
You can’t perform that action at this time.