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

When used on windows and node v0.10.26+ the isMalicious check always returns true #6

Closed
exocom opened this issue Apr 22, 2014 · 3 comments

Comments

@exocom
Copy link

exocom commented Apr 22, 2014

Awesome plugin! I have found a bug on windows.

On Windows when using node >= v0.10.26, node will parse ['c:\mypath'] into 'c:\mypath'.
Example:

var path = 'c:\windows\system';
console.log(path);  // 'c:\windows\system'
console.log([path]); // 'c:\\windows\\system' Node automatically adds \ delimiter

This results in your path.indexOf(docroot) to compare a delimited string with a non delimited string, thus always failing

'c:\\windows\\system'.indexOf('c:\windows\system');

I am going to do a pull request with this simple fix, create an array with docroot and get the string out of the array :

// array then index 0 is a windows filesytem fix please do not remove!
path.indexOf([docroot][0])
@fgnass
Copy link
Owner

fgnass commented Apr 22, 2014

In JavaScript \ is an escape character. When followed by a non-special character it doesn't become a literal basckslash. Instead, you have to escape the character itself:

var path = 'c:\\windows\\system';

See this MDN article for reference.

@fgnass fgnass closed this as completed Apr 22, 2014
@exocom
Copy link
Author

exocom commented Apr 22, 2014

Yes, I called "escape character" a "delimiter" by mistake, thanks for correcting my terminology.

However the bug still exists, were you able to take a look at my pull request? I ended up fixing the bug by replacing forward slashes with back slashes if the operating system is windows.

if(/^win/i.test(process.platform)){  // if windows operating system
    // fixes windows paths
    docroot = docroot.replace(/\//gi,"\\");
}

In my pull request I made 2 commits, after the first commit, I noticed the issues was still occurring when I specified a sub folder when using gateway directly with connect, by directly I mean not through a grunt task. So that is why I decided to change the approach to using find replace.

@fgnass
Copy link
Owner

fgnass commented Apr 22, 2014

Sorry, I didn't meant to correct your terminology. What I was trying to say is that things should already work if you escape the passed in string properly. If I understand your second commit correctly you want to be able to specify the docroot using a unix-like path (with forward slashes) on Windows too?

fgnass added a commit that referenced this issue Apr 22, 2014
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

No branches or pull requests

2 participants