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

URL-encode phone numbers so Lookup API can handle formats with spaces #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LukasRos
Copy link

The Lookup API can be used to parse and normalize phone numbers but it fails (with TypeError [ERR_UNESCAPED_CHARACTERS]: Request path contains unescaped characters) when the number contains spaces. Developers would have to URL-encode numbers, which is against the principle that an SDK should abstract HTTP details. This PR fixes that by URL-encoding within the SDK.

@LukasRos
Copy link
Author

This does not yet support formats like XXXX/YYYYYYYY (with slashes) which are commonly used in some countries. URL-encoding should do the trick so this is probably a server-side issue?!

@samwierema
Copy link
Contributor

@LukasRos thanks for submitting a pull request! We'll have a look at this next week.

As for the /. I do indeed think this is a server-side issue, but I'm unsure whether that is something we'll be supporting in the near future. I've put it towards the responsible team, and they'll have a look.

@epels
Copy link
Contributor

epels commented Jul 4, 2018

Hi @LukasRos, thanks again - and sorry it took a little longer to get back to you. I definitely agree encoding query parameters is something a client/SDK should take care off.

I'd just like to point out that this change is backwards incompatible: users who have ran into this same issue may already be encoding the phone numbers on their end. Merging this as-is would result in them ending up with double encoded, and thus malformed, strings.

A simple and effective solution for this would be to first decode the provided string, and then encoding that result (e.g. encodeURIComponent(decodeURIComponent(phoneNumber))). What do you think? Also, it would be great to have some tests for this.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants