Introduce virtual file system abstraction#30
Open
fknittel wants to merge 6 commits intomsoulier:masterfrom
Open
Introduce virtual file system abstraction#30fknittel wants to merge 6 commits intomsoulier:masterfrom
fknittel wants to merge 6 commits intomsoulier:masterfrom
Conversation
`self.dyn_file_func` is currently set twice: Once in the base class and once in the server child class. As it's only used in the non-server case, remove it from the base class.
The test currently fails, because the request path is improperly checked / sanitised.
This patch fixes the request path check. It makes sure that requested paths are _below_ the specified root directory.
This patch modifies the TFTP server code to depend on a virtual file system abstraction for all file-system access. The previous file system access code is moved to the TftpVfsNative class. The patch also provides a TftpVfsStack class, which allows multiple VFS classes to be combined into one name space. Includes unit tests.
Allows the ``--root`` option to be specified more than once. Any write requests are sent to the first root.
|
This feature would make what I am trying to do significantly easier and more powerful. |
ghost
assigned
Owner
|
If you're still interested in this, maybe we should chat. I've obviously neglected this for a long time so I'll understand if you're not interested. I'm trying to rectify that now, and I don't think I fully understand what you're doing here. |
msoulier
added
feedback needed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(Unfortunately, this is again based on the previous two pull requests / branches. I'll happily rebase if you're not interested in pulling them.)
This basically re-implements my older patch from the old fabians-patches branch. The basic idea is to abstract away all file system access, allowing various neat tricks.
Since my last implementation you've added a dyn_file_func call-back, but unfortunately this doesn't cleanly cover the intended use-cases:
The patches attempt to stay as compatible to the old interface as possible. If you find the approach generally acceptable, I'll continue along those lines to properly support tsize. (Right now tsize appears to be broken.)
The patches have the side-effect of accepting files starting with "/". BOOTP implementations appear to use this by default, so I see this as a feature, not a bug. :)
Please feel free to request smaller patches / reworked patches. I'm also willing to discuss a different approach.