Let's sanitize windows path too, now we can run paperboy on MinGW-compile #13

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants

Let's sanitize windows path too, now we can run paperboy on MinGW-compiled NodeJS.

Owner

felixge commented May 9, 2011

I was under the impression that windows can use forward slashes. Is that not correct?

I tested on node 0.4.7 only path.join can support '/' .

Owner

felixge commented May 9, 2011

What I'm saying is that windows can take a '/' instead of a '' for all it's system calls. Is that not the case?

Owner

felixge commented May 9, 2011

Oh wait, I guess you are using a '' in your webroot? Maybe a better check is to look for either '' or '/' in that if statement since windows allows for both.

if (webroot[webroot.length - 1] !== '/') webroot += '/';

Webroot on windows is something like C:\\some\\path , after this line it will be C:\\some\\path/ , then it will fail on the next line testing:

if (fp.substr(0, webroot.length) != webroot)

felixge closed this May 12, 2011

Owner

felixge commented May 12, 2011

Ok, makes sense.

Merged it - if anybody has an issue with it they can always send me another pull request : ).

UnderCooled reopened this May 12, 2011

perhaps my case, looks pull #19

Contributor

fprijate commented Sep 4, 2011

Here is solution (working also on win32):

exports.filepath = function (webroot, url) {
  var pathSep=process.platform ==='win32' ? '\\' : '/';
  // Unescape URL to prevent security holes
  url = decodeURIComponent(url);
  // Append index.html if path ends with '/'
  fp = path.normalize(path.join(webroot, (url.match(/\/$/)=='/')  ? url+'index.html' : url));
  // Sanitize input, make sure people can't use .. to get above webroot
  if (webroot[webroot.length - 1] !== pathSep) webroot += pathSep;
  if (fp.substr(0, webroot.length) != webroot)
     return(['Permission Denied', null]);
  else
     return([null, fp.replace('/',pathSep)]);
};
Contributor

fprijate commented Sep 5, 2011

Hi

Today (5.9.2011 ) node-inspector works also on Windows XP (node v0.5.6-pre MSVC build).
It works only with change to filepath function.

Owner

felixge commented Sep 8, 2011

@fprijate Can you submit this as a proper patch / pull request?

GottZ commented Oct 20, 2011

with the fix from fprijate it runs fine for me in windows with 0.5.9

Collaborator

mcandre commented May 2, 2013

This seems to have been taken care of in 9355291#lib/paperboy.js and a18d4bc#lib/paperboy.js.

mcandre closed this May 2, 2013

Owner

felixge commented May 2, 2013

@mcandre thx for going through all these issues! 💖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment