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

Host is stripped from protocol-relative urls #76

Closed
44px opened this issue Jul 24, 2018 · 2 comments
Closed

Host is stripped from protocol-relative urls #76

44px opened this issue Jul 24, 2018 · 2 comments
Assignees
Labels
bug 🐞 Something isn't working

Comments

@44px
Copy link

44px commented Jul 24, 2018

How to reproduce:

  1. Have app on http://my-app.dev
  2. Make request to protocol-relative url //api.dev/posts/1

Expected behavior
Polly proxies this request to api.dev and writes response.

Actual behavior
Polly strips host from //api.dev/posts/1 making it into /posts/1 and then trying to send request to http://my-app.dev/posts/1 instead of original url.

I found two places in code where url is modified. Here is one which caused desribed behavior:

/*
If the url is relative, setup the parsed url to reflect just that
by removing the host. By default URL sets the host via window.location if
it does not exist.
*/
if (!isAbsoluteUrl(value)) {
removeHostFromUrl(url);
}

Seems like is-absolute-url check here can be swapped with custom implementation supporting protocol-relative urls

@offirgolan offirgolan added the bug 🐞 Something isn't working label Jul 24, 2018
offirgolan added a commit that referenced this issue Jul 24, 2018
- Create a shared parseUrl utility that will make sure to match the provided URL string to a url-parse instance
- Support for protocol-relative URLs (Resolves #76)
@offirgolan offirgolan self-assigned this Jul 25, 2018
offirgolan added a commit that referenced this issue Jul 25, 2018
- Create a shared parseUrl utility that will make sure to match the provided URL string to a url-parse instance
- Support for protocol-relative URLs (Resolves #76)
@offirgolan
Copy link
Collaborator

@44px thanks for the detailed issue! A fix has been published with v1.1.0 of @pollyjs/core.

@44px
Copy link
Author

44px commented Jul 30, 2018

@offirgolan, great, thank you for quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants