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

SFTP: Add a NO_REALPATH option to put() #1137

Closed
wants to merge 4 commits into from
Closed

SFTP: Add a NO_REALPATH option to put() #1137

wants to merge 4 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 2, 2017

A third party we communicate with apparently requires us to use an initial double slash in the path when uploading files, for reasons unknown to me. Never the less, using just one slash does not appear to work.

Since the internal realpath() method strips all redundant slashes we need to bypass it.

@terrafrost
Copy link
Member

I think the changing of the constants from 1 to 0x1 was... unnecessary. I mean, I get why you're doing it - it could potentially make it more clear that it's serving as a bitmask.

But regardless, the changes otherwise look good. Well, it seems like get perhaps ought to have the same method, but I went ahead and took care of that - thanks!

@terrafrost terrafrost closed this Jun 5, 2017
@terrafrost
Copy link
Member

This has been merged BTW. I cherry-picked the commit into the 1.0 branch and merged it up to 2.0 and master:

terrafrost@5979ed5

@terrafrost terrafrost reopened this Jun 5, 2017
@terrafrost
Copy link
Member

terrafrost commented Jun 5, 2017

Actually, scratch the above. I did the merge on my local fork but hadn't yet pushed upstream... get doesn't have a $mode parameter. I could add one but it seems like this might be better if it was a global thing? eg. ->disableRealpath() and ->enableRealpath(), with the latter being the default option?

edit: what I've done thus far:

https://github.com/terrafrost/phpseclib/tree/no-realpath-1.0
https://github.com/terrafrost/phpseclib/tree/no-realpath-2.0
https://github.com/terrafrost/phpseclib/tree/no-realpath-master

@ghost
Copy link
Author

ghost commented Jun 7, 2017

Thank you for your review.

How does this look? I decided to go for "enablePathCanonicalization" instead of enableRealpath as I thought that to be a bit more explanatory, but that can be changed of course.

@terrafrost
Copy link
Member

I like that a lot better! Imma get this in the 1.0 and 2.0 as well. I'll try to have this done this evening.

Thanks!!

@terrafrost
Copy link
Member

I rebased your commits into one, changed the name of the first commit and then cherry-picked it from 1.0:

bbf467b

From there I merged into 2.0 and master.

@terrafrost terrafrost closed this Jun 13, 2017
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.

None yet

2 participants