From ebfd80c80ce8c167a307f68f19a3e9e17c4c8fec Mon Sep 17 00:00:00 2001 From: Patricio Whittingslow Date: Sun, 26 Jul 2026 21:29:02 -0300 Subject: [PATCH] ai insists with backoffs --- http/httphi/exchange.go | 156 +++++++++++++++------ http/httphi/exchange_test.go | 243 ++++++++++++--------------------- http/httpraw/multipart.go | 117 ++++++---------- http/httpraw/multipart_test.go | 117 ++++++++++------ 4 files changed, 323 insertions(+), 310 deletions(-) diff --git a/http/httphi/exchange.go b/http/httphi/exchange.go index 938ed78..ba24ac1 100644 --- a/http/httphi/exchange.go +++ b/http/httphi/exchange.go @@ -1,6 +1,7 @@ package httphi import ( + "io" "net" "slices" "strconv" @@ -441,53 +442,11 @@ func (exch *Exchange) RequestParseForm(dst *httpraw.Form, buf []byte, backoff ln return dst.Parse() } -// RequestParseMultipart prepares dst from the boundary parameter of the +// RequestMultipart returns a parser prepared from the boundary parameter of the // request's Content-Type field. It reads no body: multipart parts declare no // length, so the caller drives the loop with a buffer it owns and decides per -// part what to keep and when a part has grown too large. -// -// A part header and the bytes held back by [httpraw.Multipart.NextBody] both ask -// to be completed the same way: compact what is left to the front of the buffer -// and read more in behind it. A buffer that fills without completing either is -// the caller's cue that the part is too large to go on with. -// -// // refill compacts rest to the front of buf and reads more of the body in. -// refill := func(rest []byte) ([]byte, error) { -// n := copy(buf, rest) -// if n == len(buf) { -// return nil, lneto.ErrBufferFull -// } -// nr, err := exch.ReadBody(buf[n:]) -// return buf[:n+nr], err -// } -// -// err := exch.RequestParseMultipart(&mp) -// rest := buf[:0] -// for { -// next, err := mp.NextHeader(&hdr, rest) -// if err == io.EOF { -// break // Closing delimiter, body done. -// } else if err == httpraw.ErrNeedMoreData { -// rest, err = refill(rest) -// // ...handle err, then: -// continue -// } else if err != nil { -// return err -// } -// rest = next -// for { -// body, next, done := mp.NextBody(rest) -// // Consume body for hdr.Name, hdr.Filename. -// rest = next -// if done { -// break -// } -// rest, err = refill(rest) -// if err != nil { -// return err -// } -// } -// } +// part what to keep and when a part has grown too large. See +// [Exchange.ReadMultiparts] for that loop already written. func (exch *Exchange) RequestMultipart() (mp httpraw.Multipart, err error) { contentType := exch.RequestContentType() if !httpraw.MediaTypeIs(contentType, "multipart/form-data") { @@ -496,6 +455,113 @@ func (exch *Exchange) RequestMultipart() (mp httpraw.Multipart, err error) { return mp, mp.SetContentType(contentType) } +// MultipartSink is a part of a multipart body together with the writer its +// content was streamed to, as appended by [Exchange.ReadMultiparts]. +type MultipartSink struct { + // Header identifies the part. Name and Filename are copies, so they + // outlive the read buffer; PartView does not, see [httpraw.MultipartHeader]. + Header httpraw.MultipartHeader + // Sink received the part's content and was closed when the part ended, + // nil for a part newSink chose to discard. + Sink io.WriteCloser +} + +// ReadMultiparts streams the request's "multipart/form-data" body, writing each +// part to a sink newSink returns for it and appending the pair to dst. buf is the +// only storage used and content is never held whole, so a part of any length +// streams through a buffer the caller sized. dst is appended to and returned, so +// a handler may hand back the slice of a previous request to reuse its parts. +// +// newSink is called once per part, before any of its content is read, and picks +// what to do with it from hdr.Name and hdr.Filename: return a writer to keep the +// part, or nil to discard its content and keep only the header. Each sink is +// closed as soon as its part ends, so Close reports the part arrived whole; on +// error the sink of the part being read is left open for the caller to deal with. +// +// A part header that does not fit buf is refused with [lneto.ErrShortBuffer], +// since reading more can never complete it, leaving the caller free to answer +// 413. backoff paces reads that return no data, as in [Handle]. The body is +// consumed, so call this before [Exchange.ReadBody]. +func (exch *Exchange) ReadMultiparts(dst []MultipartSink, buf []byte, newSink func(hdr *httpraw.MultipartHeader) io.WriteCloser, backoff lneto.BackoffStrategy) (_ []MultipartSink, _ error) { + mp, err := exch.RequestMultipart() + if err != nil { + return dst, err + } else if newSink == nil || len(buf) <= len("\r\n--")+len(mp.Boundary) { + // A buffer that cannot outgrow a delimiter never makes progress. + return dst, lneto.ErrInvalidConfig + } + buflen := 0 + for { + // Slot for the next part, given back when the body turns out to be + // over, so its Name and Filename buffers stay available for reuse. + part := internal.SliceReclaim(&dst) + var parsed int + for { + parsed, err = mp.NextHeader(&part.Header, buf[:buflen]) + if err != nil { + dst = dst[:len(dst)-1] + if err == io.EOF { + err = nil // Closing delimiter, body done. + } + return dst, err + } else if parsed > 0 { + break // Delimiter and header block complete. + } else if buflen == len(buf) { + dst = dst[:len(dst)-1] + return dst, lneto.ErrShortBuffer // Header longer than buf. + } + buflen, err = exch.readBodyMore(buf, buflen, backoff) + if err != nil { + dst = dst[:len(dst)-1] + return dst, err + } + } + part.Sink = newSink(&part.Header) + buflen = copy(buf, buf[parsed:buflen]) + for { + bodyLen, restOff, done := mp.NextBody(buf[:buflen]) + if bodyLen > 0 && part.Sink != nil { + _, err = part.Sink.Write(buf[:bodyLen]) + if err != nil { + return dst, err + } + } + buflen = copy(buf, buf[restOff:buflen]) + if done { + break // Buffer now starts at the next part's delimiter. + } + buflen, err = exch.readBodyMore(buf, buflen, backoff) + if err != nil { + return dst, err + } + } + if part.Sink != nil { + if err = part.Sink.Close(); err != nil { + return dst, err + } + } + } +} + +// readBodyMore reads more of the body in behind the buflen bytes already in buf, +// returning the new length once at least one byte arrived. A parser that stalled +// for want of data always makes progress or gets an error, never spins. +func (exch *Exchange) readBodyMore(buf []byte, buflen int, backoff lneto.BackoffStrategy) (int, error) { + for backoffs := uint(0); ; backoffs++ { + n, err := exch.ReadBody(buf[buflen:]) + buflen += n + if n > 0 { + // Data first: a read that both delivered and failed, as the last + // of the body followed by a hangup does, may still hold the + // closing delimiter. The error surfaces on the next read. + return buflen, nil + } else if err != nil { + return buflen, err + } + backoff.Do(backoffs) // Nothing pending on the conn yet. + } +} + // RequestHeader returns the value of the first request header field matching // key, or nil if absent. Key matching is case sensitive. func (exch *Exchange) RequestHeader(key string) []byte { diff --git a/http/httphi/exchange_test.go b/http/httphi/exchange_test.go index e9c3db3..d5e679d 100644 --- a/http/httphi/exchange_test.go +++ b/http/httphi/exchange_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/soypat/lneto/http/httpraw" - "github.com/soypat/lneto/internal" "github.com/soypat/lneto" ) @@ -752,151 +751,46 @@ func TestExchangeRequestParseFormDecode(t *testing.T) { } } -// refill compacts the bytes the multipart parser held back to the front of buf -// and reads more of the body in behind them. It returns once at least one byte -// arrived, so a caller that got [httpraw.ErrNeedMoreData] always makes progress -// or gets an error, never spins. -func refill(exch *Exchange, buf, rest []byte, backoff lneto.BackoffStrategy) ([]byte, error) { - n := copy(buf, rest) - if n == len(buf) { - return nil, lneto.ErrBufferFull // A delimiter or part header longer than buf. - } - for backoffs := uint(0); ; backoffs++ { - nr, err := exch.ReadBody(buf[n:]) - if nr > 0 || err != nil { - return buf[:n+nr], err - } - backoff.Do(backoffs) // Nothing pending on the conn yet. - } +// partBuffer is a sink that keeps a part's content in memory and records that +// [Exchange.ReadMultiparts] closed it. +type partBuffer struct { + content []byte + closed bool } -// multiPart is one part of a multipart body, copied out of the read buffer so it -// outlives the reads that produced it. -type multiPart struct { - name string - filename string - content []byte +func (p *partBuffer) Write(b []byte) (int, error) { + if p.closed { + return 0, errors.New("write to closed part sink") + } + p.content = append(p.content, b...) + return len(b), nil } -// readMultiPart drains the request's multipart body into parts, using buf as its -// only scratch space. It is the shape of the loop a handler writes: NextHeader -// and NextBody both ask to be completed the same way, by compacting what is left -// to the front of buf and reading more in behind it, so parts of any length fit -// a buffer the caller sized. A part header that does not fit is reported as -// [lneto.ErrBufferFull], since reading more can never complete it. -func readMultiPart(exch *Exchange, buf []byte, backoff lneto.BackoffStrategy) (parts []multiPart, _ error) { - mp, err := exch.RequestMultipart() - if err != nil { - return nil, err - } - var hdr httpraw.MultipartHeader - rest := buf[:0] - for { - next, err := mp.NextHeader(&hdr, rest) - if err == io.EOF { - return parts, nil // Closing delimiter, body done. - } else if err == httpraw.ErrNeedMoreData { - if rest, err = refill(exch, buf, rest, backoff); err != nil { - return nil, err - } - continue - } else if err != nil { - return nil, err - } - // hdr aliases buf, so copy it out before a refill moves those bytes. - part := internal.SliceReclaim(&parts) - part.name, part.filename = string(hdr.Name), string(hdr.Filename) - part.content = part.content[:0] // Reclaimed from an earlier body. - rest = next - for { - body, next, done := mp.NextBody(rest) - part.content = append(part.content, body...) - rest = next - if done { - break // rest begins the next part's delimiter. - } - if rest, err = refill(exch, buf, rest, backoff); err != nil { - return nil, err - } - } - } -} +func (p *partBuffer) Close() error { p.closed = true; return nil } -type MultipartSink struct { - Header httpraw.MultipartHeader - Sink io.Writer -} - -func (exch *Exchange) ReadMultiparts(dst []MultipartSink, buf []byte, newSink func(hdr *httpraw.MultipartHeader) io.WriteCloser) (_ []MultipartSink, _ error) { - if newSink == nil || len(buf) < 72 { - return dst, lneto.ErrInvalidConfig - } - mp, err := exch.RequestMultipart() - if err != nil { - return dst, err - } - buflen := 0 - for { - n, err := exch.ReadBody(buf[buflen:]) - if err != nil { - return dst, err - } - buflen += n - extendedDst := dst - maybePart := internal.SliceReclaim(&extendedDst) - parsed, err := mp.NextHeaderInt(&maybePart.Header, buf[:buflen]) - if err != nil { - if err == io.EOF { - err = nil // Closing delimiter, body done. - } - return dst, err - } else if parsed == 0 { - // Need more data before being able to parse header. - if buflen == len(buf) { - return dst, lneto.ErrShortBuffer - } - continue - } - dst = extendedDst - maybePart.Sink = newSink(&maybePart.Header) - buflen = copy(buf, buf[parsed:]) - for { - bodylen, done := mp.NextBodyInt(buf[:buflen]) - if bodylen > 0 { - _, err = maybePart.Sink.Write(buf[:bodylen]) - if err != nil { - return dst, err - } - } - if done { - break - } - buflen = copy(buf, buf[bodylen:buflen]) - n, err := exch.ReadBody(buf[buflen:]) - if err != nil { - return dst, err - } - buflen += n - } - } -} - -// serveMultipart serves request to a handler that drains its multipart body -// with readMultiPart over a buffer of bufSize bytes. segments are delivered on -// later reads, so the parser must compact and refill to see them. -func serveMultipart(t *testing.T, request string, bufSize int, segments ...string) ([]multiPart, error) { +// serveMultipart serves request to a handler that streams its multipart body +// with [Exchange.ReadMultiparts] over a buffer of bufSize bytes. segments are +// delivered on later reads, so the parser must compact and read more to see +// them. skip names the parts whose sink is refused, exercising discarding. +func serveMultipart(t *testing.T, request string, bufSize int, skip string, segments ...string) ([]MultipartSink, error) { t.Helper() conn := newConn(request) for _, segment := range segments { conn.AddSegment(segment) } conn.Hangup() - var parts []multiPart + var parts []MultipartSink var gotErr error var sm MuxSlice sm.Reset(1) sm.Handle("/f", func(exch *Exchange) { - parts, gotErr = readMultiPart(exch, make([]byte, bufSize), nopBackoff) + newSink := func(hdr *httpraw.MultipartHeader) io.WriteCloser { + if skip != "" && string(hdr.Name) == skip { + return nil // Discard this part's content. + } + return new(partBuffer) + } + parts, gotErr = exch.ReadMultiparts(parts, make([]byte, bufSize), newSink, nopBackoff) }) exch := newExchange(t, conn, 1024, false) if err := Handle(exch, &sm, nopBackoff); err != nil { @@ -906,70 +800,115 @@ func serveMultipart(t *testing.T, request string, bufSize int, segments ...strin } // partsString renders parts as "name=content" joined by '|', a file part shown -// as "name(filename)=content". -func partsString(parts []multiPart) string { +// as "name(filename)=content" and a discarded one as "name=". Fails the +// test if a sink was left open, which would hide a part that never ended. +func partsString(t *testing.T, parts []MultipartSink) string { + t.Helper() var sb strings.Builder - for i, part := range parts { + for i := range parts { if i > 0 { sb.WriteByte('|') } - sb.WriteString(part.name) - if part.filename != "" { + part := &parts[i] + sb.Write(part.Header.Name) + if len(part.Header.Filename) > 0 { sb.WriteByte('(') - sb.WriteString(part.filename) + sb.Write(part.Header.Filename) sb.WriteByte(')') } sb.WriteByte('=') - sb.Write(part.content) + if part.Sink == nil { + sb.WriteString("") + continue + } + sink := part.Sink.(*partBuffer) + if !sink.closed { + t.Errorf("part %q: sink left open", part.Header.Name) + } + sb.Write(sink.content) } return sb.String() } // Names, filenames and content of every part, over a body split so that a part -// straddles two reads and the caller must compact and refill. -func TestExchangeReadMultipart(t *testing.T) { +// straddles two reads and the parser must compact and read more. +func TestExchangeReadMultiparts(t *testing.T) { const ( head = "POST /f HTTP/1.1\r\nHost: h\r\nContent-Type: multipart/form-data; boundary=--xyz\r\n\r\n" part1 = "----xyz\r\nContent-Disposition: form-data; name=\"caption\"\r\n\r\nhi there\r\n" part2 = "----xyz\r\nContent-Disposition: form-data; name=\"photo\"; filename=\"beach.png\"\r\n\r\n\x89PNG\r\n\x00\r\n" tail = "----xyz--\r\n" ) - parts, err := serveMultipart(t, head+part1+part2[:20], 128, part2[20:]+tail) + parts, err := serveMultipart(t, head+part1+part2[:20], 128, "", part2[20:]+tail) if err != nil { t.Fatal(err) } const want = "caption=hi there|photo(beach.png)=\x89PNG\r\n\x00" - if got := partsString(parts); got != want { + if got := partsString(t, parts); got != want { t.Errorf("want %q, got %q", want, got) } } -// A part longer than the buffer must come out whole: each refill has to keep the -// tail NextBody held back, or content that looks like the start of a delimiter -// is dropped. -func TestExchangeReadMultipartPartLargerThanBuffer(t *testing.T) { +// A part longer than the buffer must come out whole: every compaction has to +// keep the tail NextBody held back, or content that looks like the start of a +// delimiter is dropped. The header's Name must survive those reads too. +func TestExchangeReadMultipartsPartLargerThanBuffer(t *testing.T) { // Content teases the parser with delimiter prefixes that never complete. content := strings.Repeat("\r\n--xy", 16) + strings.Repeat("A", 100) + "\r\n--xyy" body := "--xyz\r\nContent-Disposition: form-data; name=\"blob\"\r\n\r\n" + content + "\r\n--xyz--\r\n" head := "POST /f HTTP/1.1\r\nHost: h\r\nContent-Type: multipart/form-data; boundary=xyz\r\n\r\n" - parts, err := serveMultipart(t, head, 64, body[:30], body[30:]) + parts, err := serveMultipart(t, head, 64, "", body[:30], body[30:]) if err != nil { t.Fatal(err) } want := "blob=" + content - if got := partsString(parts); got != want { + if got := partsString(t, parts); got != want { + t.Errorf("want %q, got %q", want, got) + } +} + +// A nil sink discards a part's content without losing its place in the body: +// the parts around it must still arrive whole. +func TestExchangeReadMultipartsDiscardsPart(t *testing.T) { + const ( + head = "POST /f HTTP/1.1\r\nHost: h\r\nContent-Type: multipart/form-data; boundary=xyz\r\n\r\n" + part1 = "--xyz\r\nContent-Disposition: form-data; name=\"keep\"\r\n\r\nkept\r\n" + part2 = "--xyz\r\nContent-Disposition: form-data; name=\"huge\"; filename=\"big.bin\"\r\n\r\n" + part3 = "--xyz\r\nContent-Disposition: form-data; name=\"also\"\r\n\r\nkept too\r\n" + tail = "--xyz--\r\n" + ) + discarded := strings.Repeat("Z", 200) + "\r\n" + parts, err := serveMultipart(t, head+part1+part2+discarded+part3+tail, 96, "huge") + if err != nil { + t.Fatal(err) + } + const want = "keep=kept|huge(big.bin)=|also=kept too" + if got := partsString(t, parts); got != want { t.Errorf("want %q, got %q", want, got) } } // A part header that does not fit the buffer cannot be completed by reading // more, so the caller is told instead of spinning. -func TestExchangeReadMultipartHeaderLargerThanBuffer(t *testing.T) { +func TestExchangeReadMultipartsHeaderLargerThanBuffer(t *testing.T) { head := "POST /f HTTP/1.1\r\nHost: h\r\nContent-Type: multipart/form-data; boundary=xyz\r\n\r\n" body := "--xyz\r\nContent-Disposition: form-data; name=\"" + strings.Repeat("n", 64) + "\"\r\n\r\nv\r\n--xyz--\r\n" - _, err := serveMultipart(t, head+body, 32) - if err != lneto.ErrBufferFull { - t.Errorf("want %v, got %v", lneto.ErrBufferFull, err) + parts, err := serveMultipart(t, head+body, 32, "") + if err != lneto.ErrShortBuffer { + t.Errorf("want %v, got %v", lneto.ErrShortBuffer, err) + } + if len(parts) != 0 { + t.Errorf("want no parts reported for a header that never parsed, got %d", len(parts)) + } +} + +// A buffer too small to ever outgrow a delimiter is a caller error, refused +// before any of the body is read. +func TestExchangeReadMultipartsBufferUnusable(t *testing.T) { + head := "POST /f HTTP/1.1\r\nHost: h\r\nContent-Type: multipart/form-data; boundary=xyz\r\n\r\n" + body := "--xyz\r\nContent-Disposition: form-data; name=\"a\"\r\n\r\nv\r\n--xyz--\r\n" + if _, err := serveMultipart(t, head+body, len("\r\n--xyz"), ""); err != lneto.ErrInvalidConfig { + t.Errorf("want %v, got %v", lneto.ErrInvalidConfig, err) } } diff --git a/http/httpraw/multipart.go b/http/httpraw/multipart.go index 95d7df4..fd783e8 100644 --- a/http/httpraw/multipart.go +++ b/http/httpraw/multipart.go @@ -10,20 +10,25 @@ import ( // value has no length: it ends where the next delimiter begins. // // Multipart stores none of the body, leaving the caller to decide what to keep, -// what to skip and when a part has grown too large: +// what to skip and when a part has grown too large. Both methods report how much +// of buf they consumed, which the caller compacts away before reading more in: // -// m := httpraw.Multipart{Boundary: httpraw.MultipartBoundary(contentType)} +// var m httpraw.Multipart +// m.SetContentType(contentType) // var hdr httpraw.MultipartHeader // for { -// rest, err := m.NextHeader(&hdr, buf) +// parsed, err := m.NextHeader(&hdr, buf[:buflen]) // if err != nil { -// break // io.EOF, or ErrNeedMoreData: read more into buf and retry. +// break // io.EOF at the closing delimiter, body done. +// } else if parsed == 0 { +// // Header block incomplete: read more into buf[buflen:] and retry. +// continue // } +// buflen = copy(buf, buf[parsed:buflen]) // for { -// body, next, done := m.NextBody(rest) -// // Consume body for hdr.Name, then compact next to the front of buf -// // and read more. -// rest = next +// bodyLen, restOff, done := m.NextBody(buf[:buflen]) +// // Consume buf[:bodyLen] for hdr.Name, then compact and read more. +// buflen = copy(buf, buf[restOff:buflen]) // if done { // break // } @@ -36,19 +41,29 @@ type Multipart struct { } // MultipartHeader is a part's header block and the Content-Disposition -// parameters that identify it. All fields alias the buffer they were parsed -// from and stay valid as long as it does. +// parameters that identify it. type MultipartHeader struct { - // PartView is the part's raw header block, ending in its final CRLF. + // PartView is the part's raw header block, ending in its final CRLF. It + // aliases the buffer it was parsed from, so it is only valid until that + // buffer is compacted or read into again. PartView []byte // Name is the name parameter of a part's Content-Disposition field, // i.e: "photo" for `form-data; name="photo"; filename="beach.png"`. + // Copied out of the buffer, so it outlives it, and reused between parts. Name []byte // Filename is the filename parameter of a part's Content-Disposition - // field, nil when the part is not a file upload. + // field, empty when the part is not a file upload. Copied like Name. Filename []byte } +// Reset clears the header for the next part, keeping the buffers Name and +// Filename were copied into so a reused header stops allocating. +func (hdr *MultipartHeader) Reset() { + hdr.PartView = nil + hdr.Name = hdr.Name[:0] + hdr.Filename = hdr.Filename[:0] +} + // SetContentType sets [Multipart.Boundary] from the boundary parameter of a // Content-Type field value, i.e: "abc123" for // "multipart/form-data; boundary=abc123". The leading "--" the delimiter carries @@ -63,48 +78,13 @@ func (m *Multipart) SetContentType(contentType []byte) error { return nil } -// NextHeader splits the leading part's header block off a multipart body into -// dst, returning the body bytes that follow it. Returns [ErrNeedMoreData] while -// data holds no complete delimiter and header block, and [io.EOF] once the -// closing delimiter is reached. dst is left zeroed on error. -func (m *Multipart) NextHeader(dst *MultipartHeader, data []byte) (rest []byte, err error) { - *dst = MultipartHeader{} - if len(m.Boundary) == 0 { - return nil, errNoBoundary - } - idx := m.indexDelimiter(data) - if idx < 0 { - return nil, ErrNeedMoreData - } - after := idx + len("--") + len(m.Boundary) - if after+2 > len(data) { - return nil, ErrNeedMoreData // Cannot tell a closing delimiter yet. - } else if data[after] == '-' && data[after+1] == '-' { - return nil, io.EOF - } - // Delimiter is followed by CRLF, then the part's header block. - if data[after] == '\r' { - after++ - } - if after >= len(data) { - return nil, ErrNeedMoreData - } else if data[after] != '\n' { - return nil, errBadDelimiter - } - after++ - end := bytes.Index(data[after:], []byte("\r\n\r\n")) - if end < 0 { - return nil, ErrNeedMoreData - } - dst.PartView = data[after : after+end+2] - disposition := partField(dst.PartView) - dst.Name = append(dst.Name[:0], ContentParam(disposition, "name")...) - dst.Filename = append(dst.Filename[:0], ContentParam(disposition, "filename")...) - return data[after+end+4:], nil -} - -func (m *Multipart) NextHeaderInt(dst *MultipartHeader, data []byte) (parsedLen int, err error) { - *dst = MultipartHeader{} +// NextHeader parses the leading part's header block off a multipart body into +// dst, returning how many bytes of data it consumed: the part's content begins +// at data[parsedLen]. A zero parsedLen and no error means data holds no complete +// delimiter and header block yet, so the caller reads more in and retries. +// Returns [io.EOF] once the closing delimiter is reached. dst is reset on error. +func (m *Multipart) NextHeader(dst *MultipartHeader, data []byte) (parsedLen int, err error) { + dst.Reset() if len(m.Boundary) == 0 { return 0, errNoBoundary } @@ -139,35 +119,22 @@ func (m *Multipart) NextHeaderInt(dst *MultipartHeader, data []byte) (parsedLen return after + end + 4, nil } -func (m *Multipart) NextBodyInt(data []byte) (bodyLen int, done bool) { +// NextBody reports how much of data is part content, data[:bodyLen], and where +// what is left begins, data[restOff:], which the caller compacts to the front of +// its buffer before reading more in. done reports the part ended, in which case +// data[restOff:] begins the next part's delimiter; otherwise the bytes past +// bodyLen are a tail held back because it could still turn into a delimiter. +func (m *Multipart) NextBody(data []byte) (bodyLen, restOff int, done bool) { idx := m.indexPartEnd(data) if idx >= 0 { - return idx, true - // return data[:idx], data[idx+len("\r\n"):], true + return idx, idx + len("\r\n"), true } // Longest prefix of "\r\n--"+boundary that could still be completed. hold := len("\r\n--") + len(m.Boundary) - 1 if hold > len(data) { hold = len(data) } - return len(data) - hold, false -} - -// NextBody returns the part bytes available in data, holding back any tail -// that could be the start of a delimiter. done reports the part ended, in which -// case rest begins the next part's delimiter; otherwise rest is the held back -// tail, which the caller compacts before reading more data into the buffer. -func (m *Multipart) NextBody(data []byte) (body, rest []byte, done bool) { - idx := m.indexPartEnd(data) - if idx >= 0 { - return data[:idx], data[idx+len("\r\n"):], true - } - // Longest prefix of "\r\n--"+boundary that could still be completed. - hold := len("\r\n--") + len(m.Boundary) - 1 - if hold > len(data) { - hold = len(data) - } - return data[:len(data)-hold], data[len(data)-hold:], false + return len(data) - hold, len(data) - hold, false } // indexDelimiter returns the offset of the leading "--"+Boundary in data. diff --git a/http/httpraw/multipart_test.go b/http/httpraw/multipart_test.go index 8dcbc33..971e522 100644 --- a/http/httpraw/multipart_test.go +++ b/http/httpraw/multipart_test.go @@ -101,7 +101,7 @@ func TestContentParam(t *testing.T) { func TestNextPartHeader(t *testing.T) { m := Multipart{Boundary: []byte(multiBoundary)} var hdr MultipartHeader - rest, err := m.NextHeader(&hdr, []byte(multiBody)) + parsed, err := m.NextHeader(&hdr, []byte(multiBody)) if err != nil { t.Fatal(err) } @@ -112,11 +112,28 @@ func TestNextPartHeader(t *testing.T) { if string(hdr.Name) != "caption" { t.Errorf("want name %q, got %q", "caption", hdr.Name) } - if hdr.Filename != nil { - t.Errorf("want nil filename for a non file part, got %q", hdr.Filename) + if len(hdr.Filename) != 0 { + t.Errorf("want no filename for a non file part, got %q", hdr.Filename) } - if !strings.HasPrefix(string(rest), "hi there\r\n") { - t.Errorf("want rest at part body, got %q", rest) + if !strings.HasPrefix(multiBody[parsed:], "hi there\r\n") { + t.Errorf("want rest at part body, got %q", multiBody[parsed:]) + } +} + +// Names and filenames must outlive the buffer they were parsed from, so a +// caller may compact it and read more without losing the part it is reading. +func TestNextPartHeaderOutlivesBuffer(t *testing.T) { + m := Multipart{Boundary: []byte(multiBoundary)} + data := []byte(multiBody) + var hdr MultipartHeader + if _, err := m.NextHeader(&hdr, data); err != nil { + t.Fatal(err) + } + for i := range data { + data[i] = 'x' // Buffer reused for the next read. + } + if string(hdr.Name) != "caption" { + t.Errorf("want name %q to survive the buffer, got %q", "caption", hdr.Name) } } @@ -130,8 +147,9 @@ func TestNextPartHeaderNeedMore(t *testing.T) { "------abc123\r\nContent-Disposition: form-", // Header block unterminated. } { var hdr MultipartHeader - if _, err := m.NextHeader(&hdr, []byte(data)); err != ErrNeedMoreData { - t.Errorf("%q: want ErrNeedMoreData, got %v", data, err) + parsed, err := m.NextHeader(&hdr, []byte(data)) + if parsed != 0 || err != nil { + t.Errorf("%q: want (0, nil) asking for more data, got (%d, %v)", data, parsed, err) } } } @@ -158,45 +176,48 @@ func TestNextPartHeaderEnd(t *testing.T) { func TestNextPartBody(t *testing.T) { m := Multipart{Boundary: []byte(multiBoundary)} var hdr MultipartHeader - rest, err := m.NextHeader(&hdr, []byte(multiBody)) + parsed, err := m.NextHeader(&hdr, []byte(multiBody)) if err != nil { t.Fatal(err) } - body, rest, done := m.NextBody(rest) + rest := multiBody[parsed:] + bodyLen, restOff, done := m.NextBody([]byte(rest)) if !done { t.Fatal("want the part to end within the buffer") } - if string(body) != "hi there" { - t.Errorf("want body %q, got %q", "hi there", body) + if rest[:bodyLen] != "hi there" { + t.Errorf("want body %q, got %q", "hi there", rest[:bodyLen]) } - if !strings.HasPrefix(string(rest), "------abc123\r\n") { - t.Errorf("want rest at next delimiter, got %q", rest) + if !strings.HasPrefix(rest[restOff:], "------abc123\r\n") { + t.Errorf("want rest at next delimiter, got %q", rest[restOff:]) } } // A part whose bytes contain CRLFs and boundary-like text must survive intact. func TestNextPartBodyBinary(t *testing.T) { m := Multipart{Boundary: []byte(multiBoundary)} - data := []byte(multiBody) + rest := []byte(multiBody) var hdr MultipartHeader - rest, err := m.NextHeader(&hdr, data) // caption part. + parsed, err := m.NextHeader(&hdr, rest) // caption part. if err != nil { t.Fatal(err) } - _, rest, _ = m.NextBody(rest) - rest, err = m.NextHeader(&hdr, rest) // photo part. + _, restOff, _ := m.NextBody(rest[parsed:]) + rest = rest[parsed+restOff:] + parsed, err = m.NextHeader(&hdr, rest) // photo part. if err != nil { t.Fatal(err) } - body, rest, done := m.NextBody(rest) + rest = rest[parsed:] + bodyLen, restOff, done := m.NextBody(rest) if !done { t.Fatal("want the part to end within the buffer") } const want = "\x89PNG\r\n--not-the-boundary\r\n\x00\xff" - if string(body) != want { - t.Errorf("want body %q, got %q", want, body) + if string(rest[:bodyLen]) != want { + t.Errorf("want body %q, got %q", want, rest[:bodyLen]) } - if _, err = m.NextHeader(&hdr, rest); err != io.EOF { + if _, err = m.NextHeader(&hdr, rest[restOff:]); err != io.EOF { t.Errorf("want io.EOF after last part, got %v", err) } } @@ -208,15 +229,16 @@ func TestNextPartBodySplitDelimiter(t *testing.T) { const part = "hi there" full := part + "\r\n------abc123\r\n" for split := 1; split < len(full); split++ { - body, rest, done := m.NextBody([]byte(full[:split])) + data := full[:split] + bodyLen, restOff, done := m.NextBody([]byte(data)) if done { continue // Whole delimiter already present, nothing to prove. } - if len(body) > len(part) { - t.Fatalf("split %d: emitted %q, past the end of the part", split, body) + if bodyLen > len(part) { + t.Fatalf("split %d: emitted %q, past the end of the part", split, data[:bodyLen]) } - if string(body)+string(rest) != full[:split] { - t.Fatalf("split %d: body+rest %q%q does not reconstruct input", split, body, rest) + if data[:bodyLen]+data[restOff:] != data { + t.Fatalf("split %d: body+rest %q%q does not reconstruct input", split, data[:bodyLen], data[restOff:]) } } } @@ -225,12 +247,13 @@ func TestNextPartBodySplitDelimiter(t *testing.T) { func TestNextHeaderFilePart(t *testing.T) { m := Multipart{Boundary: []byte(multiBoundary)} var hdr MultipartHeader - rest, err := m.NextHeader(&hdr, []byte(multiBody)) // caption part. + parsed, err := m.NextHeader(&hdr, []byte(multiBody)) // caption part. if err != nil { t.Fatal(err) } - _, rest, _ = m.NextBody(rest) - if _, err = m.NextHeader(&hdr, rest); err != nil { // photo part. + rest := []byte(multiBody[parsed:]) + _, restOff, _ := m.NextBody(rest) + if _, err = m.NextHeader(&hdr, rest[restOff:]); err != nil { // photo part. t.Fatal(err) } if got := string(hdr.Name); got != "photo" { @@ -245,7 +268,7 @@ func TestNextHeaderFilePart(t *testing.T) { } // A failed call must not leave the previous part's fields behind. -func TestNextHeaderZeroesOnError(t *testing.T) { +func TestNextHeaderResetsOnError(t *testing.T) { m := Multipart{Boundary: []byte(multiBoundary)} var hdr MultipartHeader if _, err := m.NextHeader(&hdr, []byte(multiBody)); err != nil { @@ -254,31 +277,49 @@ func TestNextHeaderZeroesOnError(t *testing.T) { if _, err := m.NextHeader(&hdr, []byte("------abc123--\r\n")); err != io.EOF { t.Fatalf("want io.EOF, got %v", err) } - if hdr.PartView != nil || hdr.Name != nil || hdr.Filename != nil { - t.Errorf("want zeroed header on error, got %+v", hdr) + if hdr.PartView != nil || len(hdr.Name) != 0 || len(hdr.Filename) != 0 { + t.Errorf("want cleared header on error, got %+v", hdr) } } -// The whole loop, as a caller writes it. +// A header reused across parts must stop allocating once its name and filename +// buffers are big enough. +func TestNextHeaderReuseNoAlloc(t *testing.T) { + m := Multipart{Boundary: []byte(multiBoundary)} + data := []byte(multiBody) + var hdr MultipartHeader + allocs := testing.AllocsPerRun(10, func() { + if _, err := m.NextHeader(&hdr, data); err != nil { + t.Fatal(err) + } + }) + if allocs != 0 { + t.Errorf("want a reused header to allocate 0 times, got %v", allocs) + } +} + +// The whole loop, as a caller writes it over a buffer it compacts. func TestMultipartLoop(t *testing.T) { m := Multipart{Boundary: []byte(multiBoundary)} rest := []byte(multiBody) var got []string var hdr MultipartHeader for { - next, err := m.NextHeader(&hdr, rest) + parsed, err := m.NextHeader(&hdr, rest) if err == io.EOF { break } else if err != nil { t.Fatal(err) + } else if parsed == 0 { + t.Fatal("header must complete within the buffer") } name := string(hdr.Name) total := 0 - rest = next + rest = rest[parsed:] for { - body, next, done := m.NextBody(rest) - total += len(body) - rest = next + bodyLen, restOff, done := m.NextBody(rest) + total += bodyLen + rest = rest[restOff:] if done { break }