mickael-kerjean / filestash

🦄 A file manager / web client for SFTP, S3, FTP, WebDAV, Git, Minio, LDAP, CalDAV, CardDAV, Mysql, Backblaze, ...
https://www.filestash.app/
GNU Affero General Public License v3.0
10.36k stars 767 forks source link

[bug] Can't open or download files in subfolder using FTP #636

Open velicanu opened 1 year ago

velicanu commented 1 year ago

Description of the bug

I can only open and download files from the home directory of an ftp folder.

Step by step instructions to reproduce the bug

Use the following docker-compose file, connect to the ftp through filestash, try to open a file in subfolder. Example in the gif below.

version: '2'
services:
  app:
    container_name: filestash
    image: machines/filestash
    restart: always
    # environment:
    # - APPLICATION_URL=
    # - GDRIVE_CLIENT_ID=<gdrive_client>
    # - GDRIVE_CLIENT_SECRET=<gdrive_secret>
    # - DROPBOX_CLIENT_ID=<dropbox_key>
    # - ONLYOFFICE_URL=http://onlyoffice
    ports:
    - "8334:8334"
    volumes:
    - ./state/:/app/data/state/

  onlyoffice:
    container_name: filestash_oods
    image: onlyoffice/documentserver
    restart: always
    security_opt:
      - seccomp:unconfined

  demo:
    image: delfer/alpine-ftp-server
    ports:
      - "2113:21"
    environment:
      - USERS=demo|demodemo
    container_name: demo
    restart: always
    volumes:
    - /home/ubuntu/demo/:/ftp/demo

Can you replicate that error from the demo?

Cannot replicate this in the demo.

Observed behavior

filestash

Expected behavior

Subfolders to work.

mickael-kerjean commented 11 months ago

That's interesting, what server are you using for this? Can you send me some test credentials I could use to replicate what you're getting on your side? This is a case of a FTP server I've never seen yet, it seems that somehow you're FTP server always say everything is going well but when queries it says there's nothing in this folder.

velicanu commented 11 months ago

I'm using a docker based ftp server, delfer/alpine-ftp-server . I don't have a URL for you to test this on, but if you do docker-compose up with the following docker-compose.yaml file you should be able to reproduce this locally:

version: "2"
services:
  app:
    container_name: filestash
    image: machines/filestash
    restart: always
    ports:
      - "8334:8334"
    volumes:
      - ./state/:/app/data/state/

  demo:
    image: delfer/alpine-ftp-server
    environment:
      - USERS=demo|foobar
    container_name: demo
    restart: always
    volumes:
      - ./demo:/ftp/demo

Login credentials:

Screenshot 2023-11-14 at 2 42 58 PM
vbochetRTE commented 10 months ago

I noticed the same strange behaviour. In fact, it is even stranger than what @velicanu describes in the original post: as stated above, you can't read/download a file located in a subfolder on your FTP server except if there is a file with the same name located in the current directory of your FTP user (which in the case of filestash looks to be the home directory). There is no need that the file is the same, just it has to have the same name.

After many hours of investigations, it seems that the problem comes from FTP's LIST command itself, as I also replicated the strange behaviour only using FTP command lines.

Maybe the solution would be to change the way you check the existence of the file at the beginning of the Cat method: perform a ReadDir in the parent folder and check if it contains a file with corresponding filename, instead of using Stat method.

I tried this on my side and it seems to work. I add in my message below the code I used to test my solution, but it may not be well written according to Go standards, as I never coded in Go before.

@mickael-kerjean Please let me know if you think my solution is good and if you want me to open a PR or if you implement the solution yourself. :slightly_smiling_face:

func (f Ftp) Cat(path string) (reader io.ReadCloser, err error) {
    fileExists := false
    f.Execute(func(client *goftp.Client) error {
        slashLastIndex := strings.LastIndex(path, "/") // split the path to get the parent path on one side and the filename on the other side
        parentPath := path[0:slashLastIndex]
        filename := path[slashLastIndex+1:]
        var fileInfos []os.FileInfo
        fileInfos, err = client.ReadDir(parentPath) // get the content of the parent folder
        if err != nil {
            return err
        } else {
            for _, fileInfo := range fileInfos { // check if a file in the parent folder matches the name of the file we want to read
                fileExists = fileExists || fileInfo.Name() == filename
            }
        }

        if fileExists { // if the file exists, read it
            pr, pw := io.Pipe()
            go func() {
                err = client.Retrieve(path, pw)
                if err != nil {
                    pr.CloseWithError(NewError("Problem", 409))
                }
                pw.Close()
            }()
            reader = pr
        }

        return nil
    })

    if reader == nil { // if we couldn't find the file, return an error
        err = NewError(fmt.Sprintf("No file matches path %s", path), 404)
    }

    return reader, err
}