Skip to content

Improve getFilenameFromUrl to work with proxy requests #80

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Sep 16, 2016
Merged

Improve getFilenameFromUrl to work with proxy requests #80

Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4bdb28e
fix req.path is undefined issue
ijse Mar 24, 2016
544b5be
fix when url.hostname is null
Mar 24, 2016
056fa01
fix when request hostname is null
ijse Mar 24, 2016
f33e53e
Merge branch 'ijse-patch-1' of https://github.com/ijse/webpack-dev-mi…
Jun 1, 2016
cf7e3fa
Merge pull request #1 from webpack/master
ijse Jun 1, 2016
4d8c4af
merge with new updates
Jun 1, 2016
172b5d3
Merge branch 'master' of github.com:webpack/webpack-dev-middleware
ijse Sep 16, 2016
8849bf7
restore tests
ijse Sep 16, 2016
3ac5946
fix test
ijse Sep 16, 2016
4bce806
fix unexpected error with RegExp
ijse Sep 16, 2016
1ad584b
add test for url from proxy
ijse Sep 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions lib/GetFilenameFromUrl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,36 @@
var pathJoin = require("./PathJoin");
var urlParse = require("url").parse;

function getFilenameFromUrl(publicPath, outputPath, url) {
// publicPrefix is the folder our bundle should be in
var localPrefix = publicPath || "/";
if(url.indexOf(localPrefix) !== 0) {
if(/^(https?:)?\/\//.test(localPrefix)) {
localPrefix = "/" + localPrefix.replace(/^(https?:)?\/\/[^\/]+\//, "");
// fast exit if another directory requested
if(url.indexOf(localPrefix) !== 0) return false;
} else return false;
var filename;

// localPrefix is the folder our bundle should be in
var localPrefix = urlParse(publicPath || "/");
var urlObject = urlParse(url);

// publicPath has the hostname that is not the same as request url's, should fail
if(localPrefix.hostname !== null && urlObject.hostname !== null &&
localPrefix.hostname !== urlObject.hostname) {
return false;
}

// publicPath is not in url, so it should fail
if(publicPath && localPrefix.hostname === urlObject.hostname && !new RegExp('^' + publicPath).test(url)) {
Copy link
Member

Choose a reason for hiding this comment

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

Inserting publicPath into RegExp can lead to unexpected errors, since the string will be interpreted as regex.

Maybe use lastIndexOf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lastIndexOf or indexOf ?

{
    url: "/more/complex/path.js",
    outputPath: "/a",
    publicPath: "/complex"
}

Is it expected to be false?

Copy link
Member

Choose a reason for hiding this comment

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

I think lastIndexOf, see this question.

Yes, the example you've given is expected to be false, because publicPath is not in the url. Would be nice if you add your example to the tests.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, just saw your changes with indexOf, that's also ok.

return false;
}
// get filename from request
var filename = url.substr(localPrefix.length);
if(filename.indexOf("?") >= 0) {
filename = filename.substr(0, filename.indexOf("?"));

// strip localPrefix from the start of url
if(urlObject.pathname.indexOf(localPrefix.pathname) === 0) {
filename = urlObject.pathname.substr(localPrefix.pathname.length);
}

if(!urlObject.hostname && localPrefix.hostname &&
!new RegExp('^' + localPrefix.path).test(url)) {
Copy link
Member

Choose a reason for hiding this comment

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

Same with the RegExp as above.

return false;
}
// and if not match, use outputPath as filename
return filename ? pathJoin(outputPath, filename) : outputPath;

}

module.exports = getFilenameFromUrl;