proposal: io: add WriteAll function

[quasilyte]

Proposal Details

The docs of the io.Writer guarantee that a proper implementation should return a non-nil error in case if it was a short writer (n < len(data)):

Write must return a non-nil error if it returns n < len(p). Write must not modify the slice data, even temporarily.

It's cool, but we don't trust this contract even in the stdlib. There are several examples to it, most types would check whether n is in valid range, etc.

The possibility of a weird implementation returning an invalid combination of n and err encourages complicated solutions like:

return io.Copy(w, bytes.NewBuffer(data))

Instead of just:

return w.Write(data)

There are multiple patterns like it, but overall people may resort to stuff that may even give more guarantees. The copy solution above relies on the implementation details; it never claims to be more reliable if there is a bad io.WriterTo implementation or whatever.

Since we already have io.ReadAll, perhaps we can add a io.WriteAll to have something that guarantees the behavior we want to enforce on our io.Writers.

An example implementation could look like this (just to illustrate the idea):

func WriteAll(w Writer, p []byte) (n int64, err error) {
  n, err = w.Write(p)
  if n < len(p) && err == nil {
    // This is the case we want to be guaranteed:
    // if it wasn't a full write, an error should never be nil;
    // a check like n != len(p) could capture even stranger cases where n>len(p),
    // this would mean that the ErrShortWrite is not applicable here (maybe we should use some other error instead?)
    return n, ErrShortWrite
  }
  return n, err
}

It doesn't guarantee the full write (because it could be complicated to achieve), but it does guarantee that no matter which implementation we're dealing with, the invariant described in io.Writer interface is intact: err is never nil if the write is "short".

Some things to consider:

• Maybe WriteFull is a better name
• Perhaps returning n int64 is redundant in this case, but it can still be useful to handle the error if it happens

The best part is that instead of trusting either io.Copy implementation details (does it call io.WriterTo?, does it somehow more reliable that a plain Write?) or io.Writer` implementation, we can call a function that adds a contract enforcement layer without involving any measurable overhead.

---

Since it's very likely to be rejected, I will not do an exhaustive research. But this is what I have found today:

• https://cs.opensource.google/go/go/+/refs/tags/go1.22.4:src/bytes/buffer.go;l=272
• https://cs.opensource.google/go/go/+/refs/tags/go1.22.4:src/io/io.go;l=432

I can invest more time into it if necessary.

[gabyhelp]

Similar Issues

• https://github.com/golang/go/issues/9096
• https://github.com/golang/go/issues/49474
• https://github.com/golang/go/issues/54548
• https://github.com/golang/go/issues/54111
• https://github.com/golang/go/issues/21904

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[quasilyte]

Ah, so the proper name and issue would probably would be a WriteFull. But since the old issue is closed, maybe it's time to consider the possibility of adding it again.

Or, if we go with "but Writer docs clearly state that it's not allowed", then why not remove all the invariant checks from the standard library?

[ianlancetaylor]

This sounds like a checker that the Write method of io.Writer is implemented correctly. I suggest we instead provide a function, perhaps in the testing/iotest package, that tests whether Write behaves properly.

[quasilyte]

I believe that any means like testing/linting will decrease the risk (if there is one), but they will not make people remove the checks nonetheless:

1. I doubt these checks would go away from types like bytes.Buffer or other code that does it.
2. I doubt it will make people in various project debate whether they need to check against it or not; and whether io.Copy and stuff do handle it for them magically (it does, but in a somewhat indirect way).

[DmitriyMV]

I'm not convinced since ReadFull and ReadAll both retry to fill the passed slice, until they encounter an error. From WriteAll I would expect the same, but the underlying writer may be sensitive to data being written in two separate calls.

[quasilyte]

The difference here is outlined in the #9096:

ReadFull exists because Read is allowed to return less than was asked without an error. Write is not. I don't want to encourage people to think that buggy Writers are okay by introducing a buggy Writer fixer.

A partial write should be an error.

The reason I would propose a function like WriteAll/Full (the naming is just an example) is to avoid the situation where n != len(data) and err is nil. That's about it.

This means that checking for an error would be enough to see whether any actions are required. If err is nil, then everything is fine.

[DmitriyMV]

On further thinking, I think this function will not change how people check the resulting n. They will still do it regardless.

So, from my perspective, there are two choices:

1. Change the signature to func WriteAll(w Writer, p []byte) error and return special wrapped error type on short writes (even those where the actual error occurred in the underlying writer) containing both the underlying error and the number of bytes written. When io.Writer was designed we didn't have errors.Is and errors.As but we do now.
2. Do something like func MustWrite(w Writer, p []byte) error similarly to what we do in MustCompile or MustParse in other packages. The difference here will be the fact that if n < len(p) it will panic (with unrecoverable error?) that will discourage people form writing bad implementations of io.Writer. Or even instrument Go compiler to add check after the completion of Write call to check and possible panic on err == nil && n < len(p).

[quasilyte]

I had this single-error return value in the description. I think it's not the time to finalize on the exact API since the idea behind the function (enforcing the Writer invariant) is not accepted yet.

[ruyi789]

If you are sceptical about Writer, then panic, no need to return, just give feedback to the developer

func WriteTryPanic(w io.Writer, p []byte) (n int, err error) {
	n, err = w.Write(p)
	if err == nil && n != len(p) {
		panic(io.ErrShortWrite.Error())
	}
	return n, err
}

[cristaloleg]

There are 10+ examples in stdlib only where we don't trust writer (grepping for io.ErrShortWrite) :

11 results - 11 files

src/bufio/bufio.go:
  640  	if n < b.n && err == nil {
  641: 		err = io.ErrShortWrite
  642  	}

src/bytes/buffer.go:
  272  		if m != nBytes {
  273: 			return n, io.ErrShortWrite
  274  		}

src/bytes/reader.go:
  149  	if m != len(b) && err == nil {
  150: 		err = io.ErrShortWrite
  151  	}

src/crypto/cipher/io.go:
  40  	if n != len(src) && err == nil { // should never happen
  41: 		err = io.ErrShortWrite
  42  	}

src/net/http/h2_bundle.go:
  1777  	if err == nil && n != len(f.wbuf) {
  1778: 		err = io.ErrShortWrite
  1779  	}

src/net/http/httputil/reverseproxy.go:
  660  			if nr != nw {
  661: 				return written, io.ErrShortWrite
  662  			}

src/net/http/internal/chunked.go:
  238  	if n != len(data) {
  239: 		err = io.ErrShortWrite
  240  		return

src/os/file.go:
  199  	if n != len(b) {
  200: 		err = io.ErrShortWrite
  201  	}

src/strings/reader.go:
  149  	if m != len(s) && err == nil {
  150: 		err = io.ErrShortWrite
  151  	}

src/text/tabwriter/tabwriter.go:
  253  	if n != len(buf) && err == nil {
  254: 		err = io.ErrShortWrite
  255  	}

src/vendor/golang.org/x/net/http2/hpack/encode.go:
  78  	if err == nil && n != len(e.buf) {
  79: 		err = io.ErrShortWrite
  80  	}

[dsnet]

The logical extension of this would be that nearly every callsite like:

n, err := w.Write(b)

would end up being replaced by:

n, err := io.WriteAll(w, b)

without clear guidance on when you should do the latter over the former.

The whole purpose of this helper is because incorrect io.Writer implementations exist. Perhaps, we should invest in static or dynamic analysis tooling to detect buggy implementations?

You could imagine a build mode similar to -race that augments the compilation of every Write method with this check and loudly prints to stderr if it were ever violated.

[ruyi789]

If you do encounter a defective Writer, is it completely safe by checking ErrShortWrite? No. The return values are all accurate, and the defect is actually still there, which is why the data validation exists.

[quasilyte]

@dsnet I would do that, yes; replacing most write calls just to keep the debates about whether checking n is necessary or not away.

without clear guidance on when you should do the latter over the former.

I would say: always if you're thinking about checking for n (see examples above). If you call it like that:

_, err := w.Write(data)

...then you don't need it. It's a good replacement for the cases where someone may want to check for it, but they don't know whether the case with n != len(data) && err == nil is possible. There are also cases outlined above that do pretty much the same thing, but a little bit differently.

If you do encounter a defective Writer, is it completely safe by checking ErrShortWrite?

You check for err != nil and then handle the error normally, like you would. The only benefit is the guarantee that if n != len(data) the err is never nil, this way, this pattern is enough to detect an error:

n, err := write(...)
if err != nil {
}

Instead of:

n, err := write(...)
if err != nil {
}
if n != len(data) {
  // can still get here if err is nil
  // although it's very unlikely, but there are examples above
  // from the real code that do guard against it (see cristaloleg message above)
}

Whether n should be returned or not is an open question.

The naming could also be adjusted, maybe it's io.Write, because the io.Writer (in theory) should also be a "write all" thing. What this function does is something that the interface can not: it enforces the implementation contract that was documented.

[ruyi789]

Return n is some Write after the need to try again Write, if you only need to ensure that n is complete, there is no need to judge again err. io.WriteAll/Full will rewrite Write err logic undesirable

[rsc]

I agree with ten-years-ago me from #9096:

I would really rather not. ReadFull exists because Read is allowed to return less than was asked without an error. Write is not. I don't want to encourage people to think that buggy Writers are okay by introducing a buggy Writer fixer.

Let's not do this.

[rsc]

This proposal has been declined as infeasible. — rsc for the proposal review group