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

Canot call method 'split' of null #747

Closed
Rush opened this issue Nov 30, 2014 · 10 comments

Comments

Projects
None yet
4 participants
@Rush
Copy link
Contributor

commented Nov 30, 2014

After recent upgrade my tests break:

Uncaught TypeError: Cannot call method 'split' of null
    at Object.common.urlJoin (/code/http-master/node_modules/http-proxy/lib/http-proxy/common.js:142:23)
    at Object.common.setupOutgoing (/code/http-master/node_modules/http-proxy/lib/http-proxy/common.js:75:26)
    at Array.stream [as 3] (/code/http-master/node_modules/http-proxy/lib/http-proxy/passes/web-incoming.js:109:14)
    at ProxyServer.<anonymous> (/code/http-master/node_modules/http-proxy/lib/http-proxy/index.js:80:21)
    at Object.ProxyMiddleware.requestHandler (/code/http-master/modules/middleware/proxy.js:9:4607)
@jcrugzz

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2014

@Rush can you pinpoint whats being passed in? There should be a test here to prevent these types of failures and I would love to add one.

@Rush

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2014

proxy.web(req, res, options);
where options are:

{ target: 
   { protocol: 'http:',
     slashes: true,
     auth: null,
     host: '127.0.0.1:57572',
     port: '57572',
     hostname: '127.0.0.1',
     hash: null,
     search: null,
     query: null,
     pathname: '/foo/bar',
     path: '/foo/bar',
     href: 'http://127.0.0.1:57572/foo/bar' }

Target is preparsed

@jcrugzz

This comment has been minimized.

Copy link
Contributor

commented Dec 1, 2014

@Rush hmmm thats weird, could you turn this into a failing test case. That would be amazing <3

@Rush

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2014

Hmm, not really cause that's how I use node-http-proxy, as in I don't
want to pass a string as a target since I need to parse the url first
myself.

2014-12-01 22:23 GMT+01:00 Jarrett Cruger notifications@github.com:

@Rush https://github.com/Rush hmmm thats weird, could you turn this
into a failing test case. That would be amazing <3


Reply to this email directly or view it on GitHub
#747 (comment)
.

@jcrugzz

This comment has been minimized.

Copy link
Contributor

commented Dec 1, 2014

@Rush Im not saying that you should be passing a string as the common.setupOutgoing function accepts target as an object. It gets preparsed before this function. What I don't understand is how this object is causing your tests to fail because nothing should be null which is why I'd love for you to take this object and any other aspects of your test that are contextually relevant and produce a failing test so we can get to the bottom of this.

It seems to center around this logic which to me seems impossible to be null but I'd love to be proved wrong :).

@jcrugzz

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2014

@Rush are you still experiencing this? Im looking to make the data input validation a bit more robust but I'd really like to see the full test case here :)

damonmcminn added a commit to damonmcminn/node-http-proxy that referenced this issue Apr 1, 2015

@damonmcminn damonmcminn referenced this issue Apr 1, 2015

Merged

Fix #747 #798

@damonmcminn

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2015

I also experienced this. The problem was a result of rewriting req.url to an empty string:

/path/ => /
/path => ''

So url.parse('').path => null which means common.urlJoin was being passed null as it's last argument.

I have written a fix and submitted a pull request: #798

damonmcminn added a commit to damonmcminn/api-proxy that referenced this issue Apr 1, 2015

@jcrugzz jcrugzz closed this in ab37a22 Apr 1, 2015

jcrugzz added a commit that referenced this issue Apr 1, 2015

@efkan

This comment has been minimized.

Copy link

commented Apr 6, 2015

Hey @Rush ,

Did this fixing solve your problem?
Despite common.js file had fixed I get same error.

Is there any one in my case?

@damonmcminn

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2015

@efkan can you give more information?

@efkan

This comment has been minimized.

Copy link

commented Apr 6, 2015

Thanks @damonmcminn ,

I've opened a new issue;
#800

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.