#680 closed defect (wontfix)

Fix for mutable files with FTP

Reported by: frozenfire Owned by: daira
Priority: major Milestone: 1.15.0
Component: code-frontend-ftp-sftp Version: 1.3.0
Keywords: ftpd mutable reliability integrity test known-issue Cc:
Launchpad Bug:

Description (last modified by daira)

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:25), mutable files cannot then be downloaded successfully over FTP.

Attachments (1)

ftp.patch (503 bytes) - added by frozenfire at 2009-04-10T22:34:34Z.
The patch, in diff/patch format

Download all attachments as: .zip

Change History (40)

Changed at 2009-04-10T22:34:34Z by frozenfire

The patch, in diff/patch format

comment:1 Changed at 2009-04-11T14:00:00Z by zooko

  • Summary changed from Fix for immutable files with FTP to Fix for mutable files with FTP

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.

comment:2 Changed at 2009-04-12T00:23:27Z by warner

  • Status changed from new to assigned

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.

comment:3 Changed at 2009-04-12T10:05:49Z by frozenfire

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.

comment:4 Changed at 2009-04-12T10:16:28Z by frozenfire

Unusable in normal FTP client too D=

comment:5 Changed at 2009-04-12T20:27:35Z by warner

  • Milestone changed from undecided to 1.4.1

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.

comment:6 follow-up: Changed at 2009-04-12T22:58:12Z by zooko

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

comment:7 Changed at 2009-11-01T06:08:46Z by davidsarah

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.

comment:8 Changed at 2010-01-26T15:39:23Z by zooko

  • Milestone changed from 1.6.0 to eventually

comment:9 Changed at 2010-02-07T18:21:56Z by davidsarah

  • Keywords ftpd sftp mutable reliability test added
  • Milestone changed from eventually to 1.7.0

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

comment:10 in reply to: ↑ 6 Changed at 2010-03-17T00:54:12Z by davidsarah

  • Keywords known-issue added

Replying to zooko:

Should we add it to 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.

comment:11 Changed at 2010-03-17T00:55:36Z by davidsarah

  • Keywords integrity added

comment:12 Changed at 2010-05-16T23:41:50Z by zooko

I guess this doesn't rise to the level of a Known Issue to be added to 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.

comment:13 follow-up: Changed at 2010-06-16T19:04:10Z by zooko

  • Keywords docs added

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 docs/known_issues.txt, but maybe in 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 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?

comment:14 in reply to: ↑ 13 Changed at 2010-06-16T20:30:09Z by davidsarah

  • Description modified (diff)
  • Milestone changed from 1.7.0 to 1.7.1

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 docs/known_issues.txt, but maybe in 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 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:3. 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.

comment:15 Changed at 2010-07-17T21:54:00Z by zooko

  • Keywords docs-needed added

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 .

comment:16 Changed at 2010-07-18T00:35:42Z by davidsarah

  • Milestone changed from 1.7.1 to 1.8β

Out of time for 1.7.1.

comment:17 Changed at 2010-09-11T01:05:36Z by davidsarah

  • Keywords sftp removed
  • Milestone changed from 1.8β to 1.9.0

Out of time for 1.8.

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

comment:18 Changed at 2010-09-11T01:22:08Z by davidsarah

Note that in e05c6c2c7d25db66, 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.

comment:19 follow-up: Changed at 2011-04-24T02:50:46Z by zooko

  • Keywords news-needed added

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 and FTP-and-SFTP.rst.

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.

comment:20 in reply to: ↑ 19 Changed at 2011-04-24T18:56:29Z by davidsarah

  • Owner changed from warner to davidsarah
  • Status changed from assigned to new

Replying to zooko:

So I believe the next step is to update NEWS and FTP-and-SFTP.rst.

+1. I'll do that.

comment:21 Changed at 2011-04-24T18:56:56Z by davidsarah

  • Status changed from new to assigned

comment:22 Changed at 2011-07-16T21:08:47Z by davidsarah

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.

comment:23 Changed at 2011-08-14T00:58:47Z by davidsarah

  • Milestone changed from 1.9.0 to 1.10.0

This missed the 1.9 deadline.

comment:24 Changed at 2012-03-31T01:33:15Z by davidsarah

  • Milestone changed from 1.10.0 to undecided

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

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

Last edited at 2012-03-31T02:19:21Z by davidsarah (previous) (diff)

comment:25 follow-up: Changed at 2012-03-31T01:48:17Z by davidsarah

Oh, the reason it fails is that ReadFile in 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.)

comment:26 in reply to: ↑ 25 Changed at 2012-03-31T01:54:50Z by davidsarah

  • Milestone changed from undecided to eventually

Replying to davidsarah:

Oh, the reason it fails is that ReadFile in 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).

comment:27 Changed at 2012-03-31T02:01:14Z by davidsarah

  • Description modified (diff)

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

comment:28 Changed at 2012-03-31T02:13:20Z by david-sarah@…

In dde822e26d694ae6:

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

comment:29 Changed at 2012-06-18T00:26:52Z by david-sarah@…

In 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

comment:30 Changed at 2012-06-18T00:40:28Z by david-sarah <david-sarah@…>

In 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

comment:31 Changed at 2012-06-18T02:06:21Z by david-sarah@…

In 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

comment:32 Changed at 2012-07-13T01:19:03Z by david-sarah@…

In 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

comment:33 Changed at 2014-03-15T08:26:09Z by daira

  • Description modified (diff)
  • Keywords docs docs-needed news-needed removed
  • Owner changed from davidsarah to daira
  • Status changed from assigned to new

comment:34 Changed at 2014-04-15T01:05:26Z by daira

  • Milestone changed from eventually to 1.12.0
  • Status changed from new to assigned

comment:35 Changed at 2014-12-02T19:40:59Z by warner

  • Component changed from code-frontend to code-frontend-ftp-sftp

comment:36 Changed at 2016-03-22T05:02:25Z by warner

  • Milestone changed from 1.12.0 to 1.13.0

Milestone renamed

comment:37 Changed at 2016-06-28T18:17:14Z by warner

  • Milestone changed from 1.13.0 to 1.14.0

renaming milestone

comment:38 Changed at 2020-06-30T14:45:13Z by exarkun

  • Milestone changed from 1.14.0 to 1.15.0

Moving open issues out of closed milestones.

comment:39 Changed at 2021-01-15T20:25:29Z by exarkun

  • Resolution set to wontfix
  • Status changed from assigned to closed

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.

Note: See TracTickets for help on using tickets.