understanding bytes.Buffer.WriteTo panic

[Justin Israel]

Since the Go1 release, I have made some unicode updates to go-
socket.io to keep it current. As a result, a new panic has appeared in
every transport beside websocket connections. I was wondering if
someone could help me understand the circumstance behind the panic so
I can try to make some head way fixing this...

As a result of the goroutine loop that is flushing the outgoing buffer
to the underlying socket connection:

c.mutex.Lock()
_, err = buf.WriteTo(c.socket)
c.mutex.Unlock()

...it's panicing with: panic: bytes.Buffer.WriteTo: invalid Write
count

In looking at the source of Buffer.WriteTo, I am trying to understand:

182 func (b *Buffer) WriteTo(w io.Writer) (n int64, err error) {
183 b.lastRead = opInvalid
184 if b.off < len(b.buf) {
185 nBytes := b.Len()
186 m, e := w.Write(b.buf[b.off:])
187 if m > nBytes {
188 panic("bytes.Buffer.WriteTo: invalid Write count")
189 }

... what is the circumstance where more bytes can be written to the
socket than are available in the length of the source buffer?
Could this be related to my previous unicode updates and how its
comparing lengths? Or is this really some issue where its writing
unexpected amounts?

Appreciate any help that gets me pointed the right way.

[Kyle Lemons]

If you're doing any transformations (especially ones that might increase the number of bytes) on the output before you write it, you could cause more bytes to be written.  So, in the code provided, it depends on what Write() does.  In my transforming writers, usually I return the length of the input or zero instead of the number returned by the underlying write.  Probably could confuse a writer that's trying to be smart, so maybe not the best solution, but it works :).  I ran up against this first when I confused fmt.* by doing my own tab replacement on the back-end.

[Justin Israel]

Kyle, you are a genius. Thank you!
I took your advise and just returned the length of the input slice (if
not error) and it worked!
The strange thing is how this all just started happening with the
conforming to Go1. The Write methods haven't changed in this library.
There are about 5 different "transports" in this lib with Write
methods, so they all do different ops, but for instance the XHR-
POLLING transport Write looks like this:

func (s *xhrPollingSocket) Write(p []byte) (int, error) {
if !s.connected {
return 0, ErrNotConnected
}
defer s.Close()

buf := new(bytes.Buffer)

buf.WriteString("HTTP/1.0 200 OK\r\n")
buf.WriteString("Content-Type: text/plain; charset=UTF-8\r\n")
fmt.Fprintf(buf, "Content-Length: %d\r\n", len(p))

if origin := s.req.Header.Get("Origin"); origin != "" {
fmt.Fprintf(buf, "Access-Control-Allow-Origin: %s\r\n", origin)
buf.WriteString("Access-Control-Allow-Credentials: true\r\n")
}

buf.WriteString("\r\n")
buf.Write(p)

// nr, err := buf.WriteTo(s.rwc)
_, err := buf.WriteTo(s.rwc)

// return int(nr), err
return len(p), err
}

Those comments are where I applied the suggested fix that now works.
It does indeed modify the buffer before its written, causing the
length to mismatch for the Buffer.WriteTo
But strangely this never crashed it in the past.

On Apr 29, 3:36 pm, Kyle Lemons <kev...@google.com> wrote:
> If you're doing any transformations (especially ones that might increase
> the number of bytes) on the output before you write it, you could cause
> more bytes to be written.  So, in the code provided, it depends on what
> Write() does.  In my transforming writers, usually I return the length of
> the input or zero instead of the number returned by the underlying write.
>  Probably could confuse a writer that's trying to be smart, so maybe not
> the best solution, but it works :).  I ran up against this first when I
> confused fmt.* by doing my own tab replacement on the back-end.
>

[Rémy Oudompheng]

2012/4/30 Justin Israel <justin...@gmail.com>:

> Kyle, you are a genius. Thank you!
> I took your advise and just returned the length of the input slice (if
> not error) and it worked!
> The strange thing is how this all just started happening with the
> conforming to Go1. The Write methods haven't changed in this library.

> [...]

> Those comments are where I applied the suggested fix that now works.
> It does indeed modify the buffer before its written, causing the
> length to mismatch for the Buffer.WriteTo
> But strangely this never crashed it in the past.

Suppose the buffer wants to send 1000 bytes, and due to a timeout,
only 500 bytes could be processed, and due to transformations you
return 600. How does the buffer know it has to advance by 500 bytes?

Rémy.

[Kyle Lemons]

I personally return zero bytes written on error in general.  I haven't seen any write wrapper code in the stdlib that tries to be smart about ErrShortWrite or ErrTimeout, so I'm not too concerned, but in general it's a pain to try to figure out exactly the correlation between how many bytes were written and the number of input bytes to which that corresponds in a short write or error condition unless you want to clutter the code (in a way which is in my opinion) unnecessarily.

[Justin Israel]

So if I understand the logic of Buffer.WriteTo in a nutshell, its a panic to prevent an unchecked transformation in a Write method?

To me, it makes perfect sense what Kyle is suggesting unless you want to try and get super smart about the numbers. In my specific XHR-POLLING example in the previous mail, its prepending the various headers before the source buffer data and writing that. If it fails, I'm fine with considering it a total failure and returning 0. 

[Kyle Lemons]

Nah, it's a sanity check.  Usually there's a bug somewhere if writes are writing more bytes than they're supposed to.  If you're just writing a header, write that and then return the count of the original write.  If you're adding or removing bytes it's harder, because you're technically supposed to tell the caller how many of THEIR bytes you wrote, which is dependent upon (in a difficult-to-calculate way) how many of the transformed bytes you wrote.