Skip to content

Commit

Permalink
[fix] ensure path works on windows because path.join doesnt like URLs
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcrugzz
jcrugzz committed Sep 12, 2014
1 parent d5c656b commit ed73f06
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/http-proxy/common.js
View file
@@ -1,5 +1,4 @@
var common = exports,
path = require('path'),
url = require('url'),
extend = require('util')._extend;

Expand Down Expand Up @@ -72,7 +71,9 @@ common.setupOutgoing = function(outgoing, options, req, forward) {
? url.parse(req.url).path
: req.url;

outgoing.path = path.join(targetPath, outgoingPath);
outgoing.path = targetPath
? targetPath + '/' + outgoingPath
: outgoingPath;

return outgoing;
};
Expand Down

3 comments on commit ed73f06

@arnihermann
Copy link

Choose a reason for hiding this comment

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

This commit broke proxy-ing. I'm ending up with outgoing.path which has values like ///static/images/logout-icon.png because targetPath was already /.

@STRML
Copy link
Contributor

@STRML STRML commented on ed73f06 Sep 12, 2014

Choose a reason for hiding this comment

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

I am seeing the same, this has completely broken my app - good reason to use npm shrinkwrap in the future.

@STRML
Copy link
Contributor

@STRML STRML commented on ed73f06 Sep 12, 2014

Choose a reason for hiding this comment

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

I have had similar issues with using path.join on Windows when I intended to use it from urls. Unfortunately there is no url.join so I use the following utility:

module.exports = function urlJoin() {
  var args = Array.prototype.slice.call(arguments);
  return args.join('/').replace(/\/+/g, '/');
};

This is working fine in production in loopback-explorer. I have turned it into PR #699.

Please sign in to comment.