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

Implement file upload #75

Merged
merged 2 commits into from
Feb 23, 2023
Merged

Implement file upload #75

merged 2 commits into from
Feb 23, 2023

Conversation

slifty
Copy link
Contributor

@slifty slifty commented Feb 13, 2023

This PR adds functionality for writing files to Permanent.

It has a few known limitations (which have been captured in issues already):

  1. It does not support empty file uploads (see Handle zero-sized files #79)
  2. It does not support mod time (see Supporting mod time #80)
  3. It does not support changes to existing files in permanent (see Supporting updates to files #81)

Resolves #48
Resolves #24

@slifty slifty force-pushed the 48-implement-file-upload branch 2 times, most recently from cb4367a to e0a96bc Compare February 13, 2023 18:59
@slifty slifty marked this pull request as draft February 13, 2023 19:00
@slifty slifty force-pushed the 48-implement-file-upload branch 3 times, most recently from 306a1e0 to aa815a7 Compare February 16, 2023 15:11
There were a few small typos / minor issues with debug output that have
no impact on functionality but are still worth correcting.
@slifty slifty force-pushed the 48-implement-file-upload branch 2 times, most recently from 5b93a15 to fc506c5 Compare February 22, 2023 16:28
@slifty slifty marked this pull request as ready for review February 22, 2023 16:28
@slifty slifty requested a review from nfebe February 22, 2023 16:29
@slifty slifty force-pushed the 48-implement-file-upload branch from fc506c5 to b5f1c16 Compare February 22, 2023 18:16
Copy link
Contributor

@nfebe nfebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, left two inline comments one of which you already resolved.

Tested by uploading a file to prod! Hurray!

Thanks @slifty

This allows an sftp client to upload data to the Permanent virtual
filesystem.  Unfortunately the way sftp works there is no way of knowing
how large a file is going to be until after the handle is closed, and
unfortunately permanent requires us to know how large the file is before
it will allow us to generate a presigned post.

This means our solution is to write the file into temporary storage.
This is a potential vulnerability and should be seen as a short term
solution.

Other limitations of this implementation include:

1. Lack of support for editing files
2. Lack of support for updating modtime

All of these will need to be improved in future developments.

Issue #48 Implement file open / write handlers
@slifty slifty force-pushed the 48-implement-file-upload branch from b5f1c16 to 7d5f4cd Compare February 23, 2023 15:57
@slifty slifty merged commit 62a1e9d into main Feb 23, 2023
@slifty slifty deleted the 48-implement-file-upload branch February 23, 2023 15:58
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.

Implement file open / write handlers Implement SSH_FXP_WRITE handler
2 participants