braintree / sanitize-url

MIT License
307 stars 35 forks source link

Is it expected that sanitize-url replaces Cyrillic characters? #31

Closed levvsha closed 3 years ago

levvsha commented 3 years ago

I faced with a problem, that sanitize-url replaces Cyrillic characters. So it is impossible to sanitize Cyrillic domains (like https://лот.рф) or urls with Cyrillic characters in search params (like https://yandex.ru/search/?text=шишки).

console.log(sanitizeUrl('https://лот.рф')); // ==> 'https://.'
console.log(sanitizeUrl('https://yandex.ru/search/?text=шишки')); // ==> 'https://yandex.ru/search/?text='

Check this example in runkit.

Is this behaviour intentional or maybe I can open a PR to fix it?

crookedneighbor commented 3 years ago

Feel free to open a PR. As long as ctrl characters are successfully removed to prevent javascript urls from being allowed, it should be fine.

akirchmyer commented 3 years ago

~@crookedneighbor I have a fix ready for this but can't push a branch. Can you give me permission? Thank you~

Nevermind permissions, I figured out how to open a PR form a fork.

PR is ready

crookedneighbor commented 3 years ago

This is fixed (thanks to @akirchmyer) and released in v5.0.1