Skip to content

Conversation

fabiengb
Copy link

Hi,

This is the pull request you requested to fix bug #39, bad path for Windows host.

I have tested this under Windows 7 & RedHat, it seems to work OK.

Fab

@sstur
Copy link
Collaborator

sstur commented Mar 24, 2015

Thanks @fabiengb. These changes look good to me, but they have broken some tests. We need to determine if the tests are wrong or if this is actually a breaking change.

@sstur
Copy link
Collaborator

sstur commented Mar 24, 2015

This appears to break path resolution. There are test cases where a path should resolve relative to this.root but instead it resolves relative to / which is dangerous.

@fabiengb
Copy link
Author

Hi,
You have a point here.
Can you check the new version of my repo ?

I have implemented actual virtual root for users, preventing them to go upper from the directory they are allowed to.

Plus, i have changed pathEscape function so the path showed to the client is an abstract FTP path and not the or a part of the actual FS path.

@impaler
Copy link

impaler commented Sep 25, 2015

Thanks @fabiengb your changes work for me @sstur would be great to have this in a release for windoze ;)

@ikb42
Copy link

ikb42 commented Dec 5, 2015

@sstur will this be merged? Thanks

@sstur
Copy link
Collaborator

sstur commented Dec 31, 2015

There are some problems with this code. Apart from the stylistic issues (why are there added spaces to the end of lines all over?) I don't think the code will work in a robust way (although we will need some tests to verify this).

@sstur
Copy link
Collaborator

sstur commented Dec 31, 2015

Regarding the style issues, I will add eslint to this project and have it run as part of the tests so we don't have to discuss this on future PRs.
EDIT: I have added ESLint to this project so please pull from master and run npm test before submitting a PR and it will catch style issues. Thanks!

@iongion
Copy link

iongion commented Jan 24, 2017

Can this be applied ?

@mk-pmb
Copy link
Contributor

mk-pmb commented Jan 24, 2017

I'm confused. I see travis claim success, but the diff view still shows several occurrences of whitespace at end of line.

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

Successfully merging this pull request may close these issues.

6 participants