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

fix: invalid URL error #139

Merged
merged 3 commits into from
May 3, 2019
Merged

fix: invalid URL error #139

merged 3 commits into from
May 3, 2019

Conversation

krmax44
Copy link
Contributor

@krmax44 krmax44 commented May 2, 2019

e596812 introduced an issue by swapping deprecated url.parse() for the newer WHATWG api:

https://github.com/egoist/saber/blob/e596812f21aefc8b0206883f61af257bfa51b5a4/packages/saber/vue-renderer/lib/index.js#L420

req.url is a relative URL. Unfortunately, the new API only works with absolute URLs and throws an error whenever an invalid URL is passed - a relative URL is invalid. Thrown error means Saber crashes. See https://nodejs.org/api/url.html#url_constructor_new_url_input_base for more info.

@netlify
Copy link

netlify bot commented May 2, 2019

Deploy preview for saber ready!

Built with commit 68a4ae1

https://deploy-preview-139--saber.netlify.com

Copy link
Collaborator

@egoist egoist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget we can just use req.path

@krmax44
Copy link
Contributor Author

krmax44 commented May 2, 2019

I forget we can just use req.path

req.path also includes query parameters though. req.pathname could work, I don't know polka but standard Node HTTP supports that iirc.

@egoist
Copy link
Collaborator

egoist commented May 2, 2019

Are you sure about that? last time I checked it's only the pathname.

@krmax44
Copy link
Contributor Author

krmax44 commented May 2, 2019

I checked and it appears that path is really just the actual pathname. Mixed it up with the URL objects.

Copy link
Collaborator

@egoist egoist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we still need to decodeURI though

@egoist egoist merged commit 636cd72 into saberland:master May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants