Fix for mutable files with FTP #680

New issue

Closed

opened 2009-04-10 22:11:30 +00:00 by frozenfire · 34 comments

frozenfire commented

2009-04-10 22:11:30 +00:00

Owner

FTP hangs when a directory list contains mutable files, as their filesize is the string "?", and Twisted's ftpd expects an integer.
This patch by warner should fix it.

For a different reason (comment:371477), mutable files cannot then be downloaded successfully over FTP.

FTP hangs when a directory list contains mutable files, as their filesize is the string "?", and Twisted's ftpd expects an integer. This patch by warner should fix it. For a different reason ([comment:371477](/tahoe-lafs/trac/issues/680#issuecomment-371477)), mutable files cannot then be downloaded successfully over FTP.

tahoe-lafs added the

labels 2009-04-10 22:11:30 +00:00

tahoe-lafs added this to the undecided milestone 2009-04-10 22:11:30 +00:00

warner was assigned by tahoe-lafs

2009-04-10 22:11:30 +00:00

frozenfire commented

2009-04-10 22:34:34 +00:00

Author

Owner

Attachment ftp.patch (503 bytes) added

The patch, in diff/patch format

**Attachment** ftp.patch (503 bytes) added The patch, in diff/patch format

ftp.patch

503 B

zooko commented

2009-04-11 14:00:00 +00:00

If only the code changed by this patch were covered by automated tests, I would consider including this patch in Tahoe-1.4.0. But in the interests of stability I don't want to apply patches to untested code just before the 1.4.0 release.

Actually it is the mutable files whose size shows up as "?". (This is because their size could change, and it would be expensive to examine each child to find out its current size when listing the parent directory.) If only the code changed by this patch were covered by automated tests, I would consider including this patch in Tahoe-1.4.0. But in the interests of stability I don't want to apply patches to untested code just before the 1.4.0 release.

zooko changed title from ~~Fix for immutable files with FTP~~ to Fix for mutable files with FTP

2009-04-11 14:00:00 +00:00

warner commented

2009-04-12 00:23:27 +00:00

I feel pretty comfortable about this patch. I'll try to apply it tonight. If 1.4.0 happens before then, I'd be ok with that too.

frozenfire commented

2009-04-12 10:05:49 +00:00

Author

Owner

Hmm, this seems to misbehave with curlftpfs, will test with normal FTP client too.
With curlftpfs it simply sees them as files of zero length, as the FTP server says. So they're unusable there.

Hmm, this seems to misbehave with curlftpfs, will test with normal FTP client too. With curlftpfs it simply sees them as files of zero length, as the FTP server says. So they're unusable there.

frozenfire commented

2009-04-12 10:16:28 +00:00

Author

Owner

Unusable in normal FTP client too D=

warner commented

2009-04-12 20:27:35 +00:00

Anyways, now I'm thinking we should not apply this patch before 1.4.0 .. better to keep the broken behavior the same, rather than switch it to a different broken behavior.

Drat.. maybe the client is stubbornly remembering the "0" size, and refuses to download more than 0 bytes of it later on. A simple test of this would be to change that "if not a number, return 0" to "if not a number, return 1", and then see what the client does when you try to download the file. Anyways, now I'm thinking we should *not* apply this patch before 1.4.0 .. better to keep the broken behavior the same, rather than switch it to a different broken behavior.

warner modified the milestone from undecided to 1.4.1

2009-04-12 20:27:35 +00:00

zooko commented

2009-04-12 22:58:12 +00:00

Should we add it to source:docs/known_issues.txt, with a suggested workaround saying "Don't use mutable files."?

daira commented

2009-11-01 06:08:46 +00:00

In any case, the relevant part of RFC 959 seems to be section 3.4, which says

The following transmission modes are defined in FTP:

3.4.1. STREAM MODE

If the structure is a file structure, the EOF is indicated by the sending host closing the data connection and all bytes are data bytes.

It is very unlikely that the FTP server library is using anything other than STREAM transmission mode with a file structure ( http://cr.yp.to/ftp/type.html says the other options are all obsolete). If so, then the tested clients are nonconformant in treating anything other than a close of the data connection as end-of-file. http://cr.yp.to/ftp/browsing.html seems to suggest that browser ftp clients do actually look for connection close to signal EOF -- so I'm stumped.

FTP is a horrible protocol, and grokking its specification (RFC 959) is not helped by the fact that it has never been streamlined and updated for a world where bytes are always 8 bits and files are flat. In any case, the relevant part of RFC 959 seems to be section 3.4, which says > The following transmission modes are defined in FTP: > 3.4.1. STREAM MODE > If the structure is a file structure, the EOF is indicated by the sending host closing the data connection and all bytes are data bytes. It is very unlikely that the FTP server library is using anything other than STREAM transmission mode with a file structure ( <http://cr.yp.to/ftp/type.html> says the other options are all obsolete). If so, then the tested clients are nonconformant in treating anything other than a close of the data connection as end-of-file. <http://cr.yp.to/ftp/browsing.html> seems to suggest that browser ftp clients do actually look for connection close to signal EOF -- so I'm stumped.

zooko modified the milestone from 1.6.0 to eventually

2010-01-26 15:39:23 +00:00

daira commented

2010-02-07 18:21:56 +00:00

SFTP has a similar problem -- just writing up a new ticket about that. I think blackmatch fuse also gets this wrong.

daira modified the milestone from eventually to 1.7.0

2010-02-07 18:21:56 +00:00

daira commented

2010-03-17 00:54:12 +00:00

Replying to zooko:

Should we add it to source:docs/known_issues.txt, with a suggested workaround saying "Don't use mutable files."?

Yes (more precisely, don't use mutable files with FTP). We should leave this ticket open since it might be fixable, even though I don't currently see how to do so reliably.

Replying to [zooko](/tahoe-lafs/trac/issues/680#issuecomment-371452): > Should we add it to source:docs/known_issues.txt, with a suggested workaround saying "Don't use mutable files."? Yes (more precisely, don't use mutable files with FTP). We should leave this ticket open since it *might* be fixable, even though I don't currently see how to do so reliably.

zooko commented

2010-05-16 23:41:50 +00:00

I guess this doesn't rise to the level of a Known Issue to be added to source:docs/known_issues.txt. (That file is more for things which might badly burn you if you don't know about them.) Let's add a note about this issue to the documentation of the (S)FTP interfaces.

zooko commented

2010-06-16 19:04:10 +00:00

Mutable files are not supported by the FTP frontend.

(from source:docs/frontends/FTP-and-SFTP.txt@4439#L201)
Hm, this documentation seems incomplete. What happens if I put a mutable file into a directory with tahoe put or tahoe ln or the wui, and then I list that directory using my FTP client?

Do we need to document this problem with mutable files in FTP? I don't think it is serious enough to deserve an entry in source:docs/known_issues.txt, but maybe in source:docs/frontends/FTP-and-SFTP.txt? Oh, I see there is already a note in there: ``` Mutable files are not supported by the FTP frontend. ``` (from source:docs/frontends/FTP-and-SFTP.txt@4439#L201) Hm, this documentation seems incomplete. What happens if I put a mutable file into a directory with `tahoe put` or `tahoe ln` or the wui, and then I list that directory using my FTP client?

daira commented

2010-06-16 20:30:09 +00:00

Replying to zooko:

Do we need to document this problem with mutable files in FTP? I don't think it is serious enough to deserve an entry in source:docs/known_issues.txt, but maybe in source:docs/frontends/FTP-and-SFTP.txt? Oh, I see there is already a note in there:
Mutable files are not supported by the FTP frontend.
(from source:docs/frontends/FTP-and-SFTP.txt@4439#L201)
Hm, this documentation seems incomplete. What happens if I put a mutable file into a directory with tahoe put or tahoe ln or the wui, and then I list that directory using my FTP client?

Well, according to the bug description it hangs (it's not clear whether the client hangs or the FTP server, probably the former).

The patch has not been applied because of the problem in comment:371448. On the other hand, hanging on a directory listing is a more serious problem than failing to download the file correctly in some (broken) clients, so perhaps the patch should be applied anyway. Not for 1.7.0, though.

Replying to [zooko](/tahoe-lafs/trac/issues/680#issuecomment-371459): > Do we need to document this problem with mutable files in FTP? I don't think it is serious enough to deserve an entry in source:docs/known_issues.txt, but maybe in source:docs/frontends/FTP-and-SFTP.txt? Oh, I see there is already a note in there: > ``` > Mutable files are not supported by the FTP frontend. > ``` > (from source:docs/frontends/FTP-and-SFTP.txt@4439#L201) > Hm, this documentation seems incomplete. What happens if I put a mutable file into a directory with `tahoe put` or `tahoe ln` or the wui, and then I list that directory using my FTP client? Well, according to the bug description it hangs (it's not clear whether the client hangs or the FTP server, probably the former). The patch has not been applied because of the problem in [comment:371448](/tahoe-lafs/trac/issues/680#issuecomment-371448). On the other hand, hanging on a directory listing is a more serious problem than failing to download the file correctly in some (broken) clients, so perhaps the patch should be applied anyway. Not for 1.7.0, though.

daira modified the milestone from 1.7.0 to 1.7.1

2010-06-16 20:30:09 +00:00

zooko commented

2010-07-17 21:54:00 +00:00

This would be appropriate for 1.7.1 with the addition of a NEWS snippet and a patch to docs/frontends/FTP-and-SFTP.txt .

daira commented

2010-07-18 00:35:42 +00:00

Out of time for 1.7.1.

daira modified the milestone from 1.7.1 to 1.8β

2010-07-18 00:35:42 +00:00

daira commented

2010-09-11 01:05:36 +00:00

Out of time for 1.8.

This ticket does not affect SFTP; that was fixed for mutable files in 1.7.0.

Out of time for 1.8. This ticket does not affect SFTP; that was fixed for mutable files in 1.7.0.

daira modified the milestone from 1.8β to 1.9.0

2010-09-11 01:05:36 +00:00

daira commented

2010-09-11 01:22:08 +00:00

Note that in changeset:e05c6c2c7d25db66, source:docs/frontends/FTP-and-SFTP.txt was changed to say:

Mutable files are not supported by the FTP frontend (ticket #680). Currently, a directory containing mutable files cannot even be listed over FTP.

Note that in changeset:e05c6c2c7d25db66, source:docs/frontends/FTP-and-SFTP.txt was changed to say: > Mutable files are not supported by the FTP frontend (ticket #680). Currently, a directory containing mutable files cannot even be listed over FTP.

zooko commented

2011-04-24 02:50:46 +00:00

So I believe the next step is to update [NEWS]source:NEWS and [FTP-and-SFTP.rst]source:trunk/docs/frontends/FTP-and-SFTP.rst#known-issues.

Also we should commit it to trunk early in the v1.9.0 process and ask people to try it out with their favorite FTP client.

Well if I understand comment:14 correctly, this ticket merely needs NEWS and docs and can be merged for v1.9.0. If I understand correctly, this match makes it so that mutable files get a 0 size instead of a `'?'` size. A 0 size makes certain FTP clients (I don't know which) fail to download the mutable file. A `'?'` size makes certain other ones (I don't know which) hang when listing the directory containing the file. I'm not 100% sure that the latter kind hang when listing the directory containing the file, or hang when trying to download the file. So I believe the next step is to update [NEWS]source:NEWS and [FTP-and-SFTP.rst]source:trunk/docs/frontends/FTP-and-SFTP.rst#known-issues. Also we should commit it to trunk early in the v1.9.0 process and ask people to try it out with their favorite FTP client.

daira commented

2011-04-24 18:56:29 +00:00

Replying to zooko:

So I believe the next step is to update [NEWS]source:NEWS and [FTP-and-SFTP.rst]source:trunk/docs/frontends/FTP-and-SFTP.rst#known-issues.

+1. I'll do that.

Replying to [zooko](/tahoe-lafs/trac/issues/680#issuecomment-371468): > So I believe the next step is to update [NEWS]source:NEWS and [FTP-and-SFTP.rst]source:trunk/docs/frontends/FTP-and-SFTP.rst#known-issues. +1. I'll do that.

warner was unassigned by daira

2011-04-24 18:56:29 +00:00

daira self-assigned this 2011-04-24 18:56:29 +00:00

daira commented

2011-07-16 21:08:47 +00:00

Because we'll probably have a fairly long test cycle for 1.9, it seems like a good time to apply the fix to give the size as zero, and do interoperability testing with that.

I'll rebase the patch for trunk if needed.

Because we'll probably have a fairly long test cycle for 1.9, it seems like a good time to apply the fix to give the size as zero, and do interoperability testing with that. I'll rebase the patch for trunk if needed.

daira commented

2011-08-14 00:58:47 +00:00

This missed the 1.9 deadline.

daira modified the milestone from 1.9.0 to 1.10.0

2011-08-14 00:58:47 +00:00

daira commented

2012-03-31 01:33:15 +00:00

(https://tahoe-lafs.org/trac/tahoe-lafs/attachment/ticket/1688/fix_ftpd_size_type.dpatch) has been committed (in changeset:7f6ee7e9180377bc) and makes a change equivalent to attachment:ftp.patch.

This doesn't necessarily make mutable files work with FTP. We should test interoperability with several FTP clients.

(https://tahoe-lafs.org/trac/tahoe-lafs/attachment/ticket/1688/fix_ftpd_size_type.dpatch) has been committed (in changeset:7f6ee7e9180377bc) and makes a change equivalent to [attachment:ftp.patch](/tahoe-lafs/trac/attachments/000078ac-fab1-9abf-c186-ead80dbc1936). This doesn't necessarily make mutable files work with FTP. We should test interoperability with several FTP clients.

daira modified the milestone from 1.10.0 to undecided

2012-03-31 01:33:15 +00:00

daira commented

2012-03-31 01:48:17 +00:00

Oh, the reason it fails is that ReadFile in source:src/allmydata/frontends/ftpd.py calls the read method of a filenode, which only exists for immutable filenodes. (It also exists on MutableFileVersion, but not MutableFileNode.)

Oh, the reason it fails is that `ReadFile` in source:src/allmydata/frontends/ftpd.py calls the `read` method of a filenode, which only exists for immutable filenodes. (It also exists on `MutableFileVersion`, but not `MutableFileNode`.)

daira commented

2012-03-31 01:54:50 +00:00

Replying to davidsarah:

Oh, the reason it fails is that ReadFile in source:src/allmydata/frontends/ftpd.py calls the read method of a filenode, which only exists for immutable filenodes.

To fix that, do something like:

    def send(self, consumer):
        d = self.node.get_best_readable_version()
        d.addCallback(lambda v: v.read(consumer))
        return d # when consumed

in ReadFile. But we'd also need tests for FTP (#512).

Replying to [davidsarah](/tahoe-lafs/trac/issues/680#issuecomment-371477): > Oh, the reason it fails is that `ReadFile` in source:src/allmydata/frontends/ftpd.py calls the `read` method of a filenode, which only exists for immutable filenodes. To fix that, do something like: ``` def send(self, consumer): d = self.node.get_best_readable_version() d.addCallback(lambda v: v.read(consumer)) return d # when consumed ``` in `ReadFile`. But we'd also need tests for FTP (#512).

daira modified the milestone from undecided to eventually

2012-03-31 01:54:50 +00:00

daira commented

2012-03-31 02:01:14 +00:00

I think we ended up being deeply confused on this ticket by warner's speculation in comment:371450 :-)

I think we ended up being deeply confused on this ticket by warner's speculation in [comment:371450](/tahoe-lafs/trac/issues/680#issuecomment-371450) :-)

daira commented

2012-03-31 02:13:20 +00:00

In changeset:dde822e26d694ae6:

FTP-and-SFTP.rst: directories containing mutable files should now be listable via FTP. refs #680

In changeset:dde822e26d694ae6: ``` FTP-and-SFTP.rst: directories containing mutable files should now be listable via FTP. refs #680 ```

daira commented

2012-06-18 00:26:52 +00:00

In changeset:f86a411928367998:

docs/frontends/FTP-and-SFTP.rst: listing of directories containing mutable files (but not reading/writing mutable files) is fixed for FTP. Also remove the orphaned trac link to #1688. refs #680

In changeset:f86a411928367998: ``` docs/frontends/FTP-and-SFTP.rst: listing of directories containing mutable files (but not reading/writing mutable files) is fixed for FTP. Also remove the orphaned trac link to #1688. refs #680 ```

david-sarah <david-sarah@jacaranda.org> commented

2012-06-18 00:40:28 +00:00

Author

Owner

In changeset:f86a411928367998:

docs/frontends/FTP-and-SFTP.rst: listing of directories containing mutable files (but not reading/writing mutable files) is fixed for FTP. Also remove the orphaned trac link to #1688. refs #680

daira commented

2012-06-18 02:06:21 +00:00

In changeset:5519/1.9.2:

docs/frontends/FTP-and-SFTP.rst: listing of directories containing mutable files (but not reading/writing mutable files) is fixed for FTP. Also remove the orphaned trac link to #1688. refs #680

In changeset:5519/1.9.2: ``` docs/frontends/FTP-and-SFTP.rst: listing of directories containing mutable files (but not reading/writing mutable files) is fixed for FTP. Also remove the orphaned trac link to #1688. refs #680 ```

daira commented

2012-07-13 01:19:03 +00:00

In changeset:5869/cloud-backend:

docs/frontends/FTP-and-SFTP.rst: listing of directories containing mutable files (but not reading/writing mutable files) is fixed for FTP. Also remove the orphaned trac link to #1688. refs #680

In changeset:5869/cloud-backend: ``` docs/frontends/FTP-and-SFTP.rst: listing of directories containing mutable files (but not reading/writing mutable files) is fixed for FTP. Also remove the orphaned trac link to #1688. refs #680 ```

daira removed their assignment 2014-03-15 08:26:09 +00:00

daira self-assigned this 2014-03-15 08:26:09 +00:00

daira modified the milestone from eventually to 1.12.0

2014-04-15 01:05:26 +00:00

warner added

c/code-frontend-ftp-sftp

and removed

c/code-frontend

labels 2014-12-02 19:40:59 +00:00

warner commented

2016-03-22 05:02:25 +00:00

Milestone renamed

warner modified the milestone from 1.12.0 to 1.13.0

2016-03-22 05:02:25 +00:00

warner commented

2016-06-28 18:17:14 +00:00

renaming milestone

warner modified the milestone from 1.13.0 to 1.14.0

2016-06-28 18:17:14 +00:00

exarkun commented

2020-06-30 14:45:13 +00:00

Moving open issues out of closed milestones.

exarkun modified the milestone from 1.14.0 to 1.15.0

2020-06-30 14:45:13 +00:00

exarkun commented

2021-01-15 20:25:29 +00:00

The FTP frontend has been removed and according to https://tahoe-lafs.org/trac/tahoe-lafs/timeline?from=2010-09-11T01%3A05%3A36Z&precision=second this does not affect SFTP.

The FTP frontend has been removed and according to <https://tahoe-lafs.org/trac/tahoe-lafs/timeline?from=2010-09-11T01%3A05%3A36Z&precision=second> this does not affect SFTP.

exarkun added the

r/wontfix

label 2021-01-15 20:25:29 +00:00

exarkun closed this issue

2021-01-15 20:25:29 +00:00