Skip to content

Commit b078efb

Browse files
committed
cryptobyte: add (*Builder).Unwrite and (*Builder).SetError
Unwrite allows programs to rollback builders more reliably and efficiently than by copying a Builder (which might waste an allocation and depends on internal behavior). This is useful for example to remove a length-prefixed field if it ends up being empty. SetError allows simple Builder extensions to set errors without making MarshalingValue wrappers. Based on the experience of CL 144115. Change-Id: I9a785b81b51b15af49418b5bdb71c4ef222ccc46 Reviewed-on: https://go-review.googlesource.com/c/145317 Reviewed-by: Adam Langley <agl@golang.org>
1 parent 7e6ffbd commit b078efb

File tree

2 files changed

+93
-6
lines changed

2 files changed

+93
-6
lines changed

cryptobyte/builder.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ func NewFixedBuilder(buffer []byte) *Builder {
5050
}
5151
}
5252

53+
// SetError sets the value to be returned as the error from Bytes. Writes
54+
// performed after calling SetError are ignored.
55+
func (b *Builder) SetError(err error) {
56+
b.err = err
57+
}
58+
5359
// Bytes returns the bytes written by the builder or an error if one has
5460
// occurred during building.
5561
func (b *Builder) Bytes() ([]byte, error) {
@@ -94,7 +100,7 @@ func (b *Builder) AddBytes(v []byte) {
94100
b.add(v...)
95101
}
96102

97-
// BuilderContinuation is continuation-passing interface for building
103+
// BuilderContinuation is a continuation-passing interface for building
98104
// length-prefixed byte sequences. Builder methods for length-prefixed
99105
// sequences (AddUint8LengthPrefixed etc) will invoke the BuilderContinuation
100106
// supplied to them. The child builder passed to the continuation can be used
@@ -290,6 +296,26 @@ func (b *Builder) add(bytes ...byte) {
290296
b.result = append(b.result, bytes...)
291297
}
292298

299+
// Unwrite rolls back n bytes written directly to the Builder. An attempt by a
300+
// child builder passed to a continuation to unwrite bytes from its parent will
301+
// panic.
302+
func (b *Builder) Unwrite(n int) {
303+
if b.err != nil {
304+
return
305+
}
306+
if b.child != nil {
307+
panic("attempted unwrite while child is pending")
308+
}
309+
length := len(b.result) - b.pendingLenLen - b.offset
310+
if length < 0 {
311+
panic("cryptobyte: internal error")
312+
}
313+
if n > length {
314+
panic("cryptobyte: attempted to unwrite more than was written")
315+
}
316+
b.result = b.result[:len(b.result)-n]
317+
}
318+
293319
// A MarshalingValue marshals itself into a Builder.
294320
type MarshalingValue interface {
295321
// Marshal is called by Builder.AddValue. It receives a pointer to a builder

cryptobyte/cryptobyte_test.go

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,14 @@ func TestWriteWithPendingChild(t *testing.T) {
327327
var b Builder
328328
b.AddUint8LengthPrefixed(func(c *Builder) {
329329
c.AddUint8LengthPrefixed(func(d *Builder) {
330-
defer func() {
331-
if recover() == nil {
332-
t.Errorf("recover() = nil, want error; c.AddUint8() did not panic")
333-
}
330+
func() {
331+
defer func() {
332+
if recover() == nil {
333+
t.Errorf("recover() = nil, want error; c.AddUint8() did not panic")
334+
}
335+
}()
336+
c.AddUint8(2) // panics
334337
}()
335-
c.AddUint8(2) // panics
336338

337339
defer func() {
338340
if recover() == nil {
@@ -351,6 +353,65 @@ func TestWriteWithPendingChild(t *testing.T) {
351353
})
352354
}
353355

356+
func TestSetError(t *testing.T) {
357+
const errorStr = "TestSetError"
358+
var b Builder
359+
b.SetError(errors.New(errorStr))
360+
361+
ret, err := b.Bytes()
362+
if ret != nil {
363+
t.Error("expected nil result")
364+
}
365+
if err == nil {
366+
t.Fatal("unexpected nil error")
367+
}
368+
if s := err.Error(); s != errorStr {
369+
t.Errorf("expected error %q, got %v", errorStr, s)
370+
}
371+
}
372+
373+
func TestUnwrite(t *testing.T) {
374+
var b Builder
375+
b.AddBytes([]byte{1, 2, 3, 4, 5})
376+
b.Unwrite(2)
377+
if err := builderBytesEq(&b, 1, 2, 3); err != nil {
378+
t.Error(err)
379+
}
380+
381+
func() {
382+
defer func() {
383+
if recover() == nil {
384+
t.Errorf("recover() = nil, want error; b.Unwrite() did not panic")
385+
}
386+
}()
387+
b.Unwrite(4) // panics
388+
}()
389+
390+
b = Builder{}
391+
b.AddBytes([]byte{1, 2, 3, 4, 5})
392+
b.AddUint8LengthPrefixed(func(b *Builder) {
393+
b.AddBytes([]byte{1, 2, 3, 4, 5})
394+
395+
defer func() {
396+
if recover() == nil {
397+
t.Errorf("recover() = nil, want error; b.Unwrite() did not panic")
398+
}
399+
}()
400+
b.Unwrite(6) // panics
401+
})
402+
403+
b = Builder{}
404+
b.AddBytes([]byte{1, 2, 3, 4, 5})
405+
b.AddUint8LengthPrefixed(func(c *Builder) {
406+
defer func() {
407+
if recover() == nil {
408+
t.Errorf("recover() = nil, want error; b.Unwrite() did not panic")
409+
}
410+
}()
411+
b.Unwrite(2) // panics (attempted unwrite while child is pending)
412+
})
413+
}
414+
354415
// ASN.1
355416

356417
func TestASN1Int64(t *testing.T) {

0 commit comments

Comments
 (0)