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 #747 #798

Merged
merged 2 commits into from
Apr 1, 2015
Merged

Fix #747 #798

merged 2 commits into from
Apr 1, 2015

Conversation

damonmcminn
Copy link
Contributor

#747

If req.url is an empty string, url.parse(req.url).path is null and common.urlJoin will raise a TypeError.

@jcrugzz
Copy link
Contributor

jcrugzz commented Apr 1, 2015

@damonmcminn whats the use case of making req.url an empty string out of curiosity? Thanks for the fix!

jcrugzz added a commit that referenced this pull request Apr 1, 2015
@jcrugzz jcrugzz merged commit aa8f3e9 into http-party:master Apr 1, 2015
@jcrugzz
Copy link
Contributor

jcrugzz commented Apr 1, 2015

fix in 1.9.1

@damonmcminn
Copy link
Contributor Author

@jcrugzz No problem! I don't believe there is a use case for an empty string... It was an accidental discovery when I changed a regex that is used to strip the first path from req.url:
https://github.com/damonmcminn/api-proxy/commit/7dda2413535c5d5c1be0f574dde8ee69cd827fa7

The stripped path is used to identify what server to proxy the request to (so I can serve multiple APIs off a single validated domain i.e. api.damonmcminn.com) e.g.:
api.damonmcminn.com/nutrition/green-turtle => localhost:50000/green-turtle

The original regex only matched when a trailing slash was present, so req.url would be rewritten thus '/path/' => '/' but as I wanted to match regardless of trailing slash, I was erroneously rewriting '/path' => ''.

I am no longer rewriting req.url to an empty string but provided the fix in case someone else inadvertently does (or wants to).

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