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

Unclear URL matching rules with param encoding #63

Open
dobriai opened this issue May 8, 2018 · 3 comments
Open

Unclear URL matching rules with param encoding #63

dobriai opened this issue May 8, 2018 · 3 comments

Comments

@dobriai
Copy link

dobriai commented May 8, 2018

I figured it out, after some debugging, but it should be either changed or documented better.

EXAMPLE:
I have a myUrl with some search params like this: https://developer.api.acmecorp.com/content/v1/contentsSearch?filter[contents][libraryId]=urn:acme.content:library:a0d5b7b4-85cb-46f1-8df5-294f16993702&filter[contents][entityType]=content&filter[contents][searchSkipCache]=True&filter[contents][query]=title:Ellipse&page[size]=50&page[number]=1
(Pardon the extra length, it could probably be trimmed a bit for a demo - I know!)

Then I prepare to receive the call like so:

    mock.get(myUrl, (req, res) => {
...
    });

And expect a call xhr.open('get', myUrl) somewhere in the code to be handled by the mock. Instead I get an Error like this:

console.error node_modules\xhr-mock\lib\MockXMLHttpRequest.js:671
xhr-mock: No handler returned a response for the request.

GET https://developer.api.acmecorp.com/content/v1/contentsSearch?filter%5Bcontents%5D%5BlibraryId%5D=urn%3Aacme.content%3Alibrary%3Aa0d5b7b4-85cb-46f1-8df5-294f16993702&filter%5Bcontents%5D%5BentityType%5D=content&filter%5Bcontents%5D%5BsearchSkipCache%5D=True&filter%5Bcontents%5D%5Bquery%5D=title%3AEllipse&page%5Bsize%5D=50&page%5Bnumber%5D=1 HTTP/1.1
content-type: application/vnd.api+json
accept: application/vnd.api+json

Of course, stuff has been encoded! But how exactly?! Something like encodeURI(myUrl) gets me oh-so-close, but no thanks! It leaves the ':'-s intact, unlike the path displayed in the error above.

After some digging, it seems that xhr-mock's internally compares to something like this, during the lookup phase:

const encodedUrl = url.format( {...(url.parse(myUrl, true)), search: undefined} );
mock.get(encodedUrl, (req, res) => {
...

At least this works in my case!

I am not sure if this is a bug or not, but as a minimum it should be documented, if not fixed. The ideal fix being "One can use the same URLs for xhr.open() and mock.xxx()."

Very nice package - many thanks for the effort!

@jameslnewell
Copy link
Owner

👍Thanks for the report.

"One can use the same URLs for xhr.open() and mock.xxx()."

Yeah that sounds ideal. Are you able to submit a PR for the fix?

Very nice package - many thanks for the effort!

Thanks!

@dobriai
Copy link
Author

dobriai commented May 25, 2018

OK, I will try to think of a reasonable fix. The thing is I don't know that much about URL encoding and all of its facets, so I'll have to educate myself a bit more.

Do you have any opinions on what the fix would look like? At first glance I can imagine:

  1. An aggressive approach: Modify mock.get() et al to transform the URL appropriately. This would be swell, but might break something that I am not aware of

  2. A passive approach: Add a separate utility that encodes the URL, to be called by the user. This is quite safe, but requires the user to ... well, use that new API :-)

  3. Then there are probably other options - maybe add some optional arg somewhere, which tells whether the passed URL should be encoded or left "as is".

I will think about it ... when I have some time. But let me know if you have any preferences or thought on the subject!

@jameslnewell
Copy link
Owner

Yeah I'd prefer to match URLs the same as passed to xhr.open() without having to encode them for xhr-mock. I'd consider having to encode the URL for xhr-mock (and not xhr.open()) a bug... hopefully that's not affecting too many people as it hasn't been reported before.

dwido added a commit to outbrain/Leonardo that referenced this issue Aug 4, 2020
jameslnewell/xhr-mock#63
since the url is encoded when calling request.url().toString() it is failing to compare some url that have commas and such  this will help to solve
until the above issue is fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants