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

Allow square braces in URL validation #1487

Merged
merged 2 commits into from
Jun 29, 2018
Merged

Allow square braces in URL validation #1487

merged 2 commits into from
Jun 29, 2018

Conversation

BolajiOlajide
Copy link
Contributor

What does this PR do

Allow square braces in URL validation

Description of Task to be completed

Update regex to accept square braces

How should this be manually tested

const Joi = require('./');

const schema = Joi.object().keys({
    url: Joi.string().uri()
});

const resOne = Joi.validate({url: 'https://example.com?abc[]=123&abc[]=456'}, schema);

the snippet above wouldn't result in error because it's a valid URL with query.

Any background context you want to add

Fixes Issue: 1461

Screenshots

@BolajiOlajide BolajiOlajide changed the title allow square braces in URL validation Allow square braces in URL validation May 1, 2018
@WesTyler
Copy link
Contributor

WesTyler commented May 1, 2018

Can you please add a test or two? Just to show the previous failing URI and that it is now passing. :)

@BolajiOlajide
Copy link
Contributor Author

Done @WesTyler

@Marsup
Copy link
Collaborator

Marsup commented May 2, 2018

I don't think you can do it this way. Brackets need to go last and by pair, this PR is allowing any number of brackets anywhere, this looks invalid to me.

@BolajiOlajide
Copy link
Contributor Author

I'll correct the PR to ensure the brackets come in pairs but isn't the point of the PR to also allow any number of paired brackets.

For example: https://example.com?abc[]=123&abc[]=456 contains two paired brackets.

@Marsup
Copy link
Collaborator

Marsup commented May 2, 2018

The point is to validate an URL is valid. Those brackets are clearly invalid regarding RFC3986, but since then we have the whatwg one (https://url.spec.whatwg.org/#query-state), and I'm not sure for example https://example.com?a]b[[[c]=123&]]]a[[b[c]]=456 should be considered valid, still reading and trying to make sense of it.

@elliotblackburn
Copy link

From what I can see of the RFC's it is clearly invalid state given by the URL producer, but it also says it should still be transformed and understood. As there is nothing about this particular case (as it's invalid), I feel like the best course of action would be to stay as strict as possible and later on become more lenient as/if required.

That's of course an opinion as this isn't something really by the specs (for obvious reasons).

Really appreciate that someone picked this up before I had a chance, thanks for the work being done to write and review it!

@Marsup Marsup self-assigned this Jun 29, 2018
@Marsup Marsup added the feature New functionality or improvement label Jun 29, 2018
@Marsup Marsup added this to the 13.5.0 milestone Jun 29, 2018
@Marsup
Copy link
Collaborator

Marsup commented Jun 29, 2018

I'm going to allow it as an option defaulting to false. This kind of URI is very common nowadays, but I still want to adhere to RFC3986 by default.

@Marsup Marsup merged commit 7aa0df0 into hapijs:master Jun 29, 2018
Marsup added a commit that referenced this pull request Jun 29, 2018
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URI validator rejects query param square brackets
4 participants