This should fix a path enumeration issue #70
Conversation
|
Great! I can look into why the changes are making the tests fail. |
|
The change also seems to make pretty much every single test fail on Windows. |
Looks like the main reason for the Windows failures is because |
|
Ahh, gotcha. Sorry I didn't run the tests; I set up the test suite now and am having another go at it. |
|
No problem :) I just haven't yet gotten to a machine to be able to look into anything yet. Your effort is greatly appreciated! |
|
Oh, and I think if I were to give the fix a goal, it would be that "at no point, should the path ever descend below the |
|
Ok, I got back and I have a perfect solution :)! Basically, just normalize the rhs first, then there is a simple check for a leading up directive. |
|
Oh man! That's probably simpler than what I ended up doing, but the tests pass with my latest commit |
| root = normalize(root + sep) | ||
|
|
||
| // check for exiting current dir | ||
| var traversalCheck = rawPath.split(sep); |
There was a problem hiding this comment.
This still doesn't work on Windows ;) sep === '\\' on there, and path will just contain forward slashes.
There was a problem hiding this comment.
I think eventually in the far distant future we'll finally have path.split in Node.js core, haha.
|
Ok, @dxa4481 @DinisCruz if you guys want to confirm the current |
|
Great stuff, I'll take a look and its great to see such a fast fix. I will try to also add a unit test that checks for the edge cases discovered |
|
Awesome, @DinisCruz :) The final commit does have one unit test (98a5b89), but if you want to add more, that would be great :) |
|
So far |
I got rid of normilize here because join normilizes, and I added this to resolve the issue found here TeamMentor/TM_4_0_Design#206