Giter VIP home page Giter VIP logo

Comments (17)

fclairamb avatar fclairamb commented on June 27, 2024

Thank you for submitting this.

The server is definitely not behaving correctly. It shouldn't start a data connection if it knows it's unable to write the file.

I will fix it

from ftpserverlib.

fclairamb avatar fclairamb commented on June 27, 2024

This first fix seems to do the trick. I'll update it with some tests:

ftp> put ftpserver /tmp/test1.txt
local: ftpserver remote: /tmp/test1.txt
DBUG[09-22|01:16:41] FTP RECV                                 action=ftp.cmd_recv id=1 line="EPSV\r\n"
DBUG[09-22|01:16:41] FTP SEND                                 action=ftp.cmd_send id=1 line="229 Entering Extended Passive Mode (|||50348|)"
229 Entering Extended Passive Mode (|||50348|)
DBUG[09-22|01:16:41] FTP RECV                                 action=ftp.cmd_recv id=1 line="STOR /tmp/test1.txt\r\n"
DBUG[09-22|01:16:41] FTP SEND                                 action=ftp.cmd_send id=1 line="550 Could not open file: open /var/folders/vk/vgsfkf9975xfrc4_fk102g200000gn/T/ftpserver446543218/tmp/test1.txt: no such file or directory"
550 Could not open file: open /var/folders/vk/vgsfkf9975xfrc4_fk102g200000gn/T/ftpserver446543218/tmp/test1.txt: no such file or directory

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

Thanks for the super quick response!
Not sure I explained fully what was happening:

Referring to the test below:
In my OpenFile function I explicitly throw an error for the first case (relative root outside my base dir).

The second and third calls fail.
If I remove the first call, the second and third scenarios work..

Here's my test

func TestCreateFile(t *testing.T){

	srv := runServer(t)

	con := connect(t)

       // should and does throw an error
       if err := con.Stor("../create1.txt", strings.NewReader("test1")); err == nil {
		t.Fatal("Should not be able to make file outside of root")
	}

        // shouldn't throw but does
	if err := con.Stor("./create1.txt", strings.NewReader("test1")); err != nil {
		t.Fatal(errors.Wrap(err, "Failed to save simple file"))
	}

	if err := con.Stor("./create2.txt", strings.NewReader("test2")); err != nil {
		t.Fatal(errors.Wrap(err, "Failed to save simple file"))
	}

	srv.Stop()
}

My dirver OpenFile func:

// OpenFile opens a file in 3 possible modes: read, write, appending write (use appropriate flags)
func (driver *MainDriver) OpenFile(cc server.ClientContext, path string, flag int) (server.FileStream, error) {

	path = driver.baseDir + path
	if err := driver.validatePath(path); err != nil {
		return nil, err
	}

	// If we are writing and we are not in append mode, we should remove the file
	if (flag & os.O_WRONLY) != 0 {
		flag |= os.O_CREATE
		if (flag & os.O_APPEND) == 0 {
			os.Remove(path)
		}
	}

	return os.OpenFile(path, flag, 0666)
}

from ftpserverlib.

fclairamb avatar fclairamb commented on June 27, 2024

Yes I think it was clear enough,

basically what happened is that for some crazy reason we were:

  • Accepting the STOR command with a transfer connection (150)
  • Failing to open the file
  • Replying with an error message (550) - This is a second reply, which should never happen. Your client has no way to deal with that.
  • Closing the transfer connection (226)

When what we should have done is:

  • Failing to open the file
  • Replying an error message (550)

In other words, the fix simply tries to open the file before we start the transfer connection:
https://github.com/fclairamb/ftpserver/pull/32/files#diff-0c77d2426ef48618dcfde3946f0cd69eR22

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

Thanks for the quick work!

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

@fclairamb Any idea when you will merge and tag this?

from ftpserverlib.

fclairamb avatar fclairamb commented on June 27, 2024

I think tonight, when I'm home.

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

from ftpserverlib.

fclairamb avatar fclairamb commented on June 27, 2024

Came home a bit later than expected. I added a unit test, @asv and @mgenov had already approved it so I merged it.

All good for you ? If so, I think I'll make a tag v0.3 as it's a freaking big issue.

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

Looks good! Thanks for the quick turn-around on this.

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

So I tried out the new fix but I notice that I still can't make subsequent store requests.

I still get 229 Entering Extended Passive Mode.

I have a flow as follows:
Connect (Passive)
Login
Retrieve one file - > this works
Do some work
Store several files -> get 229 back as error
Quit

I checked and I'm on master's latest commit.

The redacted logs:

document_1            | t=2017-09-27T13:31:27+0000 lvl=info msg="FTP Client connected" action=ftp.connected id=3 src=192.168.96.27:42072 total=1
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="220 Welcome, you're on dir /data/cdn"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="FEAT\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="211- These are my features"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line=" UTF8"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line=" SIZE"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line=" MDTM"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line=" REST STREAM"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line=" MLSD"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="211 end"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="USER foobar\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="331 OK"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="PASS xxxxx\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="230 Password ok, continue"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="TYPE I\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="200 Type set to binary"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="EPSV\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="229 Entering Extended Passive Mode (|||38357|)"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="RETR labels/2017-09-27_13:31:27_734921092_sheet.pdf\r\n"
pdf_1                 | INFO: 2017/09/27 13:31:27 cutter.go:164: getting contents from /tmp/2017-09-27_13:31:27_734921092_sheet.pdf
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="150 Using transfer connection"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP Transfer connection opened" action=ftp.transfer_open id=3 remoteAddr=192.168.96.27:51170 localAddr=192.168.96.26:38357
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="226 Closing transfer connection"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP Transfer connection closed" action=ftp.transfer_close id=3
pdf_1                 | ERROR: 2017/09/27 13:31:27 cropworker.go:202: failed to send file to cdn 229 Entering Extended Passive Mode (|||42857|)
pdf_1                 | ERROR: 2017/09/27 13:31:27 cropworker.go:202: failed to send file to cdn 229 Entering Extended Passive Mode (|||41977|)
pdf_1                 | INFO: 2017/09/27 13:31:27 cropworker.go:96: worker: successfully processed job
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="EPSV\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="229 Entering Extended Passive Mode (|||42857|)"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="PASV\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="227 Entering Passive Mode (192,168,96,26,141,209)"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="EPSV\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="229 Entering Extended Passive Mode (|||41977|)"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="PASV\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="227 Entering Passive Mode (192,168,96,26,168,249)"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP RECV" action=ftp.cmd_recv id=3 line="QUIT\r\n"
document_1            | t=2017-09-27T13:31:27+0000 lvl=dbug msg="FTP SEND" action=ftp.cmd_send id=3 line="221 Goodbye"

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

Might have jumped the gun. This could be due to not closing my ftp client's response Reader before doing the Stor commands.

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

I think it was that, false alarm.

from ftpserverlib.

fclairamb avatar fclairamb commented on June 27, 2024

Interesting logs :) What is your "cropworker" doing ?

I'm quite interested by how people use this library. For me it is to offer a FTP access to google drive, which I offer with https://zoro.io (but the work isn't really finished around that one).

from ftpserverlib.

byrnedo avatar byrnedo commented on June 27, 2024

I had an internal service for fronting a cdn "bucket" which was using nats (https://nats.io/) for sending files to it. I changed it to ftp since sending files on the queue was a bad pattern. Instead of throwing a fully baked ftp on top I thought something like ftpserver was a better way since I potentially want more dynamic features in the future that we can integrate with our auth.

Cropworker is just cropping sections out of pdfs :) The files I'm sending back are those cropped images.

Nice idea with Zoro!

from ftpserverlib.

fclairamb avatar fclairamb commented on June 27, 2024

I created release v0.3 which contains a fix for that

from ftpserverlib.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.