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

string().uri() not working with accented characters #2889

Closed
mlarcher opened this issue Dec 10, 2022 · 15 comments · Fixed by #3027
Closed

string().uri() not working with accented characters #2889

mlarcher opened this issue Dec 10, 2022 · 15 comments · Fixed by #3027
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@mlarcher
Copy link

mlarcher commented Dec 10, 2022

Support plan

  • is this issue currently blocking your project? (yes/no): yes
  • is this issue affecting a production system? (yes/no): yes

Context

  • node version: 18.10.0
  • module version with issue: 17.1.1
  • last module version without issue:
  • environment (e.g. node, browser, native): node
  • used with (e.g. hapi application, another framework, standalone, ...):
  • any other relevant information:

What are you trying to achieve or the steps to reproduce?

const { error } = Joi.string().uri().validate('https://linkedin.com/in/aïssa/');
console.log(error); // there should be no error

What was the result you got?

"value" must be a valid uri] {
  _original: 'https://linkedin.com/in/aïssa/',
  details: [
    {
      message: '"value" must be a valid uri',
      path: [],
      type: 'string.uri',
      context: [Object]
    }
  ]
}

What result did you expect?

no error

Same check with a i instead of the ï in aïssa in the url works as expected

@mlarcher mlarcher added the support Questions, discussions, and general support label Dec 10, 2022
@geeksilva97
Copy link

geeksilva97 commented Dec 16, 2022

Hello, @mlarcher. I think if you do something like

const encodedUsername = encodeURIComponent('aïssa');

const { error } = Joi.string().uri().validate(`https://linkedin.com/in/${encodedUsername}`);
console.log(error); // undefined

it might work.

However, I also think it could be handled by the lib.

@Marsup
Copy link
Collaborator

Marsup commented Dec 16, 2022

It is rejected because it is invalid. There's a well-written answer on SO.

@mlarcher
Copy link
Author

mlarcher commented Dec 16, 2022

I understand the rationale behind it, but don't you think that we could have a setting to allow urls that are not RFC compliant but widely used and accepted in all major browsers ?

For instance, if a user copy/pastes a LinkedIn url from her browser, I would expect Joi to provide a way to recognize it as valid, be it through an option.

@Marsup
Copy link
Collaborator

Marsup commented Mar 30, 2023

Browsers accept about anything, they just run encodeURI on it before use but show you the decoded version. Using both Chrome and Firefox, when I copy your URL, I get the encoded version in the clipboard. Do you actually have a use case where this would happen?

@mlarcher
Copy link
Author

The linkedin url used in our example came into our subscription form. We want to validate it is a correct/usable url. Joi says it isn't even though to us it is...

@Marsup
Copy link
Collaborator

Marsup commented Mar 30, 2023

This sounds a lot like raw text copy-pasting, a browser wouldn't provide that. I'm not going to change the validation part (@hapi/address), but I guess joi itself could accept an option (false by default) that would apply encodeURI before attempting the validation if the convert flag is also true. Do you want to try a PR implementing that?

@mlarcher
Copy link
Author

I'd be glad to, but I don't have enough spare time at the moment to handle that I'm afraid

@HYOSITIVE
Copy link

Can I try this issue?

@Marsup
Copy link
Collaborator

Marsup commented Mar 28, 2024

Sure! Be careful to target v17 branch, the main branch is currently in progress.

@HYOSITIVE
Copy link

I submitted a PR for this. Please take a look when you have a chance. Thanks!

@Marsup Marsup linked a pull request Apr 23, 2024 that will close this issue
@Marsup Marsup added this to the 17.13.0 milestone Apr 23, 2024
@Marsup Marsup added feature New functionality or improvement and removed support Questions, discussions, and general support labels Apr 23, 2024
@Marsup Marsup self-assigned this Apr 23, 2024
@Marsup Marsup closed this as completed Apr 23, 2024
@KaKi87
Copy link

KaKi87 commented May 21, 2024

Hi,
It would be nice not to need a parameter in the validate method call to make this work for situations where said method is called by third-party code.
Thanks

@Marsup
Copy link
Collaborator

Marsup commented May 22, 2024

Do you mean an extension? That could have some unintended consequences. Depending on how it's implemented, you can maybe force this, but I'd rather have extension providers offer that option if relevant.

@KaKi87
Copy link

KaKi87 commented May 22, 2024

What I mean is, if I pass Joi.string().uri({ encodeUri: true }), I would like it to work alone, with validate(), without needing validate({ convert: true }), which can be complicated to pass when validate is called by third-party code while I'm just passing the schema.

Thanks

@Marsup
Copy link
Collaborator

Marsup commented May 22, 2024

convert: true is the default, I'm still not sure what you mean.

@KaKi87
Copy link

KaKi87 commented May 23, 2024

Oh, I'm sorry, I wasted your time.

I was experiencing an error saying I couldn't use the encodeUri option, so I thought it was because of the convert option being required, but turns out I just didn't notice how recently this feature was added, as I only saw how long ago it was requested, so I didn't doubt whether the version I was using would already include it. Therefore, simply upgrading did the trick.

Thanks

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

Successfully merging a pull request may close this issue.

5 participants