FTP frontend should avoid caching plaintext of uploads #1083

Closed
opened 2010-06-15 23:26:59 +00:00 by davidsarah · 10 comments
davidsarah commented 2010-06-15 23:26:59 +00:00
Owner

The SFTP frontend was changed to avoid using a cleartext temporary file for the contents of files opened for writing (see [EncryptedTemporaryFile in sftpd.py]source:src/allmydata/frontends/sftpd.py?rev=4478#L294). A similar change should be made to the FTP frontend.

Perhaps EncryptedTemporaryFile should be moved to [fileutil.py]source:/src/allmydata/util/fileutil.py so that it can be used by both SFTP and FTP.

The SFTP frontend was changed to avoid using a cleartext temporary file for the contents of files opened for writing (see [EncryptedTemporaryFile in sftpd.py]source:src/allmydata/frontends/sftpd.py?rev=4478#L294). A similar change should be made to the FTP frontend. Perhaps `EncryptedTemporaryFile` should be moved to [fileutil.py]source:/src/allmydata/util/fileutil.py so that it can be used by both SFTP and FTP.
tahoe-lafs added the
code-frontend
major
defect
1.7β
labels 2010-06-15 23:26:59 +00:00
tahoe-lafs added this to the 1.8.0 milestone 2010-06-15 23:26:59 +00:00
davidsarah commented 2010-06-15 23:28:54 +00:00
Author
Owner

Note that EncryptedTemporaryFile is not tested directly, only via the SFTP tests. It probably should be tested directly.

Note that `EncryptedTemporaryFile` is not tested directly, only via the SFTP tests. It probably should be tested directly.
tahoe-lafs modified the milestone from 1.8.0 to 1.7.1 2010-06-21 01:53:19 +00:00
davidsarah commented 2010-07-11 21:55:06 +00:00
Author
Owner

The FTP frontend unfortunately has no tests (#512). On the other hand, EncryptedTemporaryFile is indirectly tested by the SFTP tests.

The FTP frontend unfortunately has no tests (#512). On the other hand, `EncryptedTemporaryFile` is indirectly tested by the SFTP tests.
zooko commented 2010-07-14 07:09:13 +00:00
Author
Owner

Perhaps this should be bumped from v1.7.1 to v1.8.0 simply due to it being a nice new feature instead of a bug fix, clean-up, unfinished-business, etc.?

Perhaps this should be bumped from v1.7.1 to v1.8.0 simply due to it being a nice new feature instead of a bug fix, clean-up, unfinished-business, etc.?
zooko commented 2010-07-14 07:10:44 +00:00
Author
Owner

Oh, it is sort of a bug-fix (potential security problem), clean-up (unify SFTP and FTP backends), and unfinished-business (SFTP got this feature in 1.7.0). So disregard comment:119670. :-)

Oh, it *is* sort of a bug-fix (potential security problem), clean-up (unify SFTP and FTP backends), and unfinished-business (SFTP got this feature in 1.7.0). So disregard [comment:119670](/tahoe-lafs/trac-2024-07-25/issues/1083#issuecomment-119670). :-)
davidsarah commented 2010-07-15 06:36:35 +00:00
Author
Owner

The current patch seems to have caused a regression in SFTP that results in the test_openFile_write test hanging (probably something simple, but I haven't had time to track it down). So I wouldn't object to this being bumped to 1.8. Whether we'll fit it into 1.7.1 depends on how much the schedule slips :-)

The current patch seems to have caused a regression in SFTP that results in the test_openFile_write test hanging (probably something simple, but I haven't had time to track it down). So I wouldn't object to this being bumped to 1.8. Whether we'll fit it into 1.7.1 depends on how much the schedule slips :-)
davidsarah commented 2010-07-15 20:14:52 +00:00
Author
Owner

Replying to davidsarah:

The current patch seems to have caused a regression in SFTP that results in the test_openFile_write test hanging...

I was mistaken; this was caused by sftp-no-trunc-files-opened-with-append.dpatch, not by this patch.

Replying to [davidsarah](/tahoe-lafs/trac-2024-07-25/issues/1083#issuecomment-119672): > The current patch seems to have caused a regression in SFTP that results in the test_openFile_write test hanging... I was mistaken; this was caused by [sftp-no-trunc-files-opened-with-append.dpatch](http://tahoe-lafs.org/trac/tahoe-lafs/attachment/ticket/1106/sftp-no-trunc-files-opened-with-append.dpatch), not by this patch.
davidsarah commented 2010-07-17 06:02:25 +00:00
Author
Owner

Attachment make-ftp-use-encrypted-temporary-file.dpatch (11005 bytes) added

Move EncryptedTemporaryFile from SFTP frontend to allmydata.util.fileutil, and make the FTP frontend also use it (fi xing #1083). Also, docstrings for non-obvious usage restrictions on methods of EncryptedTemporaryFile.

**Attachment** make-ftp-use-encrypted-temporary-file.dpatch (11005 bytes) added Move [EncryptedTemporaryFile](wiki/EncryptedTemporaryFile) from SFTP frontend to allmydata.util.fileutil, and make the FTP frontend also use it (fi xing #1083). Also, docstrings for non-obvious usage restrictions on methods of [EncryptedTemporaryFile](wiki/EncryptedTemporaryFile).
zooko commented 2010-07-18 02:17:01 +00:00
Author
Owner

I just reviewed the patch. +1! Thanks, David-Sarah!

Oh, it needs a NEWS snippet. I'll add that myself.

Should we remove the "test-needed" flag?

I just reviewed the patch. +1! Thanks, David-Sarah! Oh, it needs a NEWS snippet. I'll add that myself. Should we remove the "test-needed" flag?
davidsarah commented 2010-07-18 02:38:13 +00:00
Author
Owner

Replying to zooko:

Should we remove the "test-needed" flag?

That would be part of #512, so yes.

Replying to [zooko](/tahoe-lafs/trac-2024-07-25/issues/1083#issuecomment-119674): > Should we remove the "test-needed" flag? That would be part of #512, so yes.
zooko commented 2010-07-18 05:39:52 +00:00
Author
Owner

committed in changeset:05022dca36780b3b. Thanks!

committed in changeset:05022dca36780b3b. Thanks!
tahoe-lafs added the
fixed
label 2010-07-18 05:39:52 +00:00
zooko closed this issue 2010-07-18 05:39:52 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: tahoe-lafs/trac-2024-07-25#1083
No description provided.