internal/impl: check for size cache overflow
The size cache is an int32. Store a -1 in it if the message size
overflows, and fall back to recomputing the size if the value is
negative. This means lamentable O(N^2) costs in marshaling,
but that's better than silently producing invalid output.
Also considered: Return an error. Avoids O(N^2) behavior, but gives the
user no good choices if they don't care the output being slow. Encoding
costs of messages this large are likely to be dominated by copying the
bytes rather than the size operation anyway, so slow-but-correct seems
like the most generally useful option.
We could store valid values for the range (0x7fffffff,0xfffffffe)
reserving only 0xffffffff as the overflow sentinel, but optimizing this
case seems less important than the code being obviously correct.
Fixes golang/protobuf#970.
Change-Id: I44f59ff81fdfbc8672dd5aec959d5153a081aab9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220593
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/internal/impl/encode.go b/internal/impl/encode.go
index ec08ed6..8c8a794 100644
--- a/internal/impl/encode.go
+++ b/internal/impl/encode.go
@@ -5,6 +5,7 @@
package impl
import (
+ "math"
"sort"
"sync/atomic"
@@ -48,7 +49,9 @@
return 0
}
if opts.UseCachedSize() && mi.sizecacheOffset.IsValid() {
- return int(atomic.LoadInt32(p.Apply(mi.sizecacheOffset).Int32()))
+ if size := atomic.LoadInt32(p.Apply(mi.sizecacheOffset).Int32()); size >= 0 {
+ return int(size)
+ }
}
return mi.sizePointerSlow(p, opts)
}
@@ -80,7 +83,14 @@
size += len(u)
}
if mi.sizecacheOffset.IsValid() {
- atomic.StoreInt32(p.Apply(mi.sizecacheOffset).Int32(), int32(size))
+ if size > math.MaxInt32 {
+ // The size is too large for the int32 sizecache field.
+ // We will need to recompute the size when encoding;
+ // unfortunately expensive, but better than invalid output.
+ atomic.StoreInt32(p.Apply(mi.sizecacheOffset).Int32(), -1)
+ } else {
+ atomic.StoreInt32(p.Apply(mi.sizecacheOffset).Int32(), int32(size))
+ }
}
return size
}