File.WriteTo silently drops data on short reads when the server's max packet size is smaller than the client's #658

Version: v1.13.10 (also affects earlier versions with the concurrent WriteTo implementation)

Description

When a client is configured with MaxPacketUnchecked(N) for N larger than the server's maxTxPacket (e.g. a pkg/sftp server at its 32 KiB default), the concurrent path of File.WriteTo — which is what io.Copy uses — silently skips data instead of detecting the short reads:

• The dispatcher issues SSH_FXP_READ requests for chunkSize (= client maxPacket) bytes at offsets spaced chunkSize apart, and advances the offset unconditionally (client.go, the "Slice" goroutine in WriteTo).
• The server clamps each response to its maxTxPacket, a legal short read.
• The worker accepts whatever length arrives without comparing it to the requested length (client.go, case sshFxpData: in the WriteTo map phase), and the reduce phase writes it and moves on.

With a 128 KiB client packet size against a 32 KiB server, every chunk delivers only its first 32 KiB and the remaining 96 KiB is skipped. The copy returns exactly 1/4 of the file with a nil error — truncated and corrupted output with no indication of failure. (The sequential paths are not affected: Read/writeToSequential treat a short read as EOF via readChunkAt, and ReadFrom sizes writes from the data it has.)

Found in the wild: Incus clients use MaxPacketUnchecked(128*1024) against pkg/sftp-based servers with the default maxTxPacket; incus file pull of a ~3 GiB file "completed" with ~768 MiB of corrupt output.

Reproduction (self-contained, v1.13.10):

package main

import (
	"crypto/rand"
	"fmt"
	"io"
	"net"
	"os"

	"github.com/pkg/sftp"
)

func main() {
	data := make([]byte, 4*1024*1024)
	rand.Read(data)
	tmp, _ := os.CreateTemp("", "sftp-repro-")
	defer os.Remove(tmp.Name())
	tmp.Write(data)
	tmp.Close()

	c1, c2 := net.Pipe()
	go func() {
		server, _ := sftp.NewServer(c2) // default maxTxPacket = 32768
		server.Serve()
	}()

	client, _ := sftp.NewClientPipe(c1, c1, sftp.MaxPacketUnchecked(128*1024))
	f, _ := client.Open(tmp.Name())

	n, err := io.Copy(io.Discard, f) // delegates to File.WriteTo
	fmt.Printf("source size: %d\ncopied:      %d\nerror:       %v\n", len(data), n, err)
}

Output:

source size: 4194304
copied:      1048576
error:       <nil>

Expected behavior

A short read mid-file should never lose data silently. WriteTo should either re-request the remainder of the chunk, or return an error. Returning fewer bytes than the file contains with err == nil is the worst possible outcome for a copy primitive.

I understand MaxPacketUnchecked is documented as "use at your own risk", but the failure mode here is silent corruption rather than an error, and it only manifests on the concurrent code path — the sequential paths handle the same server behavior safely.