request-server doesn't handle uploads of empty files

[an2deg]

Hi!

I figured out that request-server has a problem with uploading empty files. Uploading of an empty file finishes without errors but no file has created.
It happens because the file supposed to be created after the first SSH2_FXP_WRITE packet but SFTP client just sends SSH2_FXP_OPEN and than SSH2_FXP_CLOSE.
Possible solution:

1. Extend isOpener interface by methods isOpenedForWrite() bool and isOpenedForRead() bool.

2. In packetWorker() in case of isOpener packet if the file is opening for write, create empty sshFxpWritePacket structure and process it. Dirty and hacky example:

    case isOpener:
    	handle := rs.nextRequest(requestFromPacket(pkt))
    	if pkt.isOpenedForWrite() {
    		request, _ := rs.getRequest(handle)
    		wpkt := &sshFxpWritePacket{}
    		uerr := request.updateMethod(wpkt)
    		if uerr != nil {
    			rpkt = statusFromError(pkt, syscall.EBADF)
    		} else {
    			rpkt = request.call(rs.Handlers, wpkt)
    		}
    	}
    	rpkt = sshFxpHandlePacket{pkt.id(), handle}
   

In this case will be invoked a write of empty buffer and the file will be created. In case of already existent file the file content will no be overwritten because we use empty byte buffer.

It will also cover scenario when some SFTP client creates a file using SSH2_FXP_OPEN and then sends immediately SSH2_FXP_STAT packet just to check file existents.

P.S. Adding methods isOpenedForWrite() and isOpenedForRead() to the isOpener interface doesn't cover situation when SFTP client opens a file with O_TRUNC flag.

[puellanivis]

I don’t think injecting a sshFxpWritePacket is the right thing to do… by specification, if an SSH_FXP_OPEN request has the O_CREAT flag set, it is the server’s responsibility to create that file if it does not already exist. So, I am unsure why one would want to inject a write like that…

[an2deg]

Then I think we need special handler for SSH_FXP_OPEN packet.

[eikenb]

First let me say Thanks @an2deg for the report. It never crossed my mind to upload a empty file, but it makes perfect sense to want/need to do it. Having people to point out this kind of personal blindness is one of my favorite things about free software projects.

I'd really rather not add another Handler. I'm playing with either adding an "Open" request method [1] too the FileCmd handler or do something a bit like @an2deg first suggested but instead of creating an empty write packet, adding the necessary support for the open packet in the request handling code in fileput(), packetData(), etc. Looking through the code I'm leaning toward the latter solution as I think it makes more sense and doesn't require a new request method.

I'll create a new branch for this work and post it here so you can follow along if you'd like.

[1] a 'request method' here means the plain text http-like methods attached to the request objects. Things like "Get", "Put", "Rename", "Remove", etc. See request.call() for all of them in a switch:

sftp/request.go

Line 139 in 9e9438a

func (r *Request) call(handlers Handlers, pkt requestPacket) responsePacket {

[eikenb]

@an2deg @puellanivis, I just pushed a branch with a fix for this and opened a PR with the branch. If you have any thoughts, please comment here or on the PR, whichever is most convenient.

[an2deg]

@eikenb sorry for a late reply. MR looks good.
Thank you!