Ticket #1333 enhancement closed fixed

Opened 8 years ago

Last modified 14 months ago

Survey existing FTP servers and clients to figure out how LIST works

Reported by: exarkun Owned by:
Priority: low Milestone:
Component: ftp Keywords:
Cc: exarkun, spiv, itamarst, jknight, jesstess, adi@… Branch: branches/ftp-list-flags-1333-2
(diff, github, buildbot, log)
Author: spiv, adiroiban Launchpad Bug:

Description


Attachments

1333-1.diff Download (2.0 KB) - added by adiroiban 14 months ago.

Change History

1

  Changed 8 years ago by exarkun

Currently we special case a few options in ftp_LIST.  This sucks and we should
do something better.

2

  Changed 4 years ago by jesstess

  • owner changed from exarkun to jesstess
  • cc jesstess added

3

  Changed 3 years ago by <automation>

  • owner jesstess deleted

4

follow-up: ↓ 6   Changed 22 months ago by adiroiban

  • cc adi@… added

Beside the current code:

        # bug in konqueror
        if path == "-a":
            path = ''
        # bug in gFTP 2.0.15
        if path == "-aL":
            path = ''
        # bug in Nautilus 2.10.0
        if path == "-L":
            path = ''
        # bug in ange-ftp
        if path == "-la":
            path = ''

I have also discovered the following clients:

        # bug in FireFTP
        if path == "-al":
            path = ''

        # bug in BareFTP
        if path == "-La":
            path = ''

Maybe we can use a regex or something simple like:

if path.lower() in ['-a', '-l', '-la', '-al']:
    path = ''

5

  Changed 22 months ago by adiroiban

  • keywords review added

6

in reply to: ↑ 4   Changed 21 months ago by spiv

  • owner set to spiv
  • status changed from new to assigned
  • keywords review removed

Replying to adiroiban:

Maybe we can use a regex or something simple like: {{{ if path.lower() in ['-a', '-l', '-la', '-al']: path = }}}

I like this approach. Obviously it needs tests etc. I'll make a patch, if I don't fall asleep or otherwise get distracted first :)

7

  Changed 21 months ago by spiv

  • branch set to branches/ftp-list-flags-1333
  • branch_author set to spiv

(In [34787]) Branch to ftp-list-flags-1333

8

  Changed 21 months ago by spiv

  • owner spiv deleted
  • keywords review added
  • status changed from assigned to new

9

  Changed 21 months ago by therve

  • owner set to spiv
  • keywords review removed

1. Please name the test test_LISTWithBinLsFlags and add a docstring.

2. This needs a NEWS fragment.

Thanks, merge once those points are fixed!

10

  Changed 21 months ago by therve

#5791 has been closed a duplicate of this bug.

11

  Changed 14 months ago by adiroiban

  • owner spiv deleted
  • keywords review added

I have attached a patch on top of branch 1333

I have renamed the test and add docstring. Also added docstring and renaming for neighbor tests.

It looks like the branch already has a news file.


Please switch to git/github. Working with SVN is so slow, Trac code navigation is slow and uploading patched to tickets is not fun.

Changed 14 months ago by adiroiban

12

  Changed 14 months ago by tomprince

  • branch changed from branches/ftp-list-flags-1333 to branches/ftp-list-flags-1333-2
  • branch_author changed from spiv to spiv, tomprince

(In [37410]) Branching to ftp-list-flags-1333-2.

13

  Changed 14 months ago by tomprince

  • status changed from new to closed
  • resolution set to fixed

(In [37413]) Merge ftp-list-flags-1333-2: Handle bogus options to ftp's LIST command.

Author: spiv, adiroiban Reviewers: therve, tom.prince Fixes: #1333

The twisted.protocols.ftp.FTP server now treats "LIST -La", "LIST -al", and all other combinations of ordering and case of the "-l" and "-a" flags the same: by ignoring them rather than treating them as a pathname.

14

  Changed 14 months ago by tom.prince

  • keywords review removed
  • branch_author changed from spiv, tomprince to spiv, adiroiban

(I accidentally put my review on #5411 by mistake:)

Minor change: Prefer assertIn to assertTrue(... in ...).

I've fixed this up and merged forward.

Will merge once build results are in.

Note: See TracTickets for help on using tickets.