Skip to content

WIP: HttpsFS #16

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

WIP: HttpsFS #16

wants to merge 8 commits into from

Conversation

der-b
Copy link

@der-b der-b commented Feb 10, 2021

Hi,

I did a proof of concept implementation of a HttpsFS. This FS allows to access over HTTPS. The whole implementation is work in progress and not ready for a pull. I use the pull request to announce this idea and hope i get some feedback. Would this be something, which you like to pull?

For a fix overview how the API works, see the https_fs_server.rs and https_fs.rs in the example directory,

I have a ToDo list, which can be found in src/impl/https.rs, but here are some higlights:

  • Add user authentication
  • Prepare the HttpsFS for WebAssembly
  • Do a version check on session start (There are no sessions yet)
  • Since this is a POC, the code is a mess. For example, all Errors are converted to VfsError::Other in the current implementation. This have to be fixed.

I noticed some oddities of the FileSystem trait:

  • The exists() method is the only method, which does not return a VfsResult. This is a problem for the HttpsFS, since the connection to the HttpsFSServer may fail. In this case, it is not clear, whether a file exist or not. Therefor i would prefer to return an VfsError instead of a boolean.
  • u64 is used for position in Files, but the traits std::io::Read and std::io::Write use usize for positions. (std::io::Seek uses u64 too, but this also is strange)

What is your opinion to all this?

Kind regards,

PS: I'm new to Rust and this is my first "project" after "Hello World!". There is probably a lot stuff, which can be improved.

@manuel-woelker
Copy link
Owner

manuel-woelker commented Feb 11, 2021

Hi, great to see this!

  • Regarding exists() not returning a VfsResult, you are totally right, that is quite inconsistent and should be fixed. I added VfsPath::exists() should return VfsResult<bool> instead of plain bool #17.

  • u64 is only used in the in-memory implementation mostly due to the fact that SeekFrom uses 64 bit addresses as well. It could probably be usize, but it would mean type casts either way.

  • Regarding the PR, I think the best course of action for a HTTPS implementation would be to split it into a separate crate. This would allow users to pick and choose those implementations they require, and only pull in those dependencies. For now it would also be best to host this as a separate repository as well.

@der-b der-b mentioned this pull request Mar 13, 2021
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.

2 participants