Skip to content

Commit

Permalink
Deprecate Valid() and add Wellformed() to replace it
Browse files Browse the repository at this point in the history 
Valid() is deprecated and only calls WellFormed(), so it will
behave the same way between current v2.4 and upcoming v2.5 release.

Projects should begin replacing calls to Valid() with calls to WellFormed().

Although the old docs for Valid() correctly said it checks for well-formedness,
this refactor was done because Valid() is not an appropriate function name.

RFC 8949 distinctly defines what is "Valid" and what is "Well-formed".

Wellformed() checks whether data is a well-formed encoded CBOR data item and
that it complies with additional restrictions such as MaxNestedLevels,
MaxArrayElements, MaxMapPairs, etc.  If there are any remaining bytes
after the encoded CBOR data item, then ExtraneousDataError is returned.
  • Loading branch information
@fxamacker
fxamacker committed May 1, 2023
1 parent 6dfffb1 commit c9e9b26
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 48 deletions.
82 changes: 73 additions & 9 deletions decode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,32 @@ func Unmarshal(data []byte, v interface{}) error {
return defaultDecMode.Unmarshal(data, v)
}

// Valid checks whether the CBOR data is complete and well-formed.
// Valid checks whether data is a well-formed encoded CBOR data item and
// that it complies with default restrictions such as MaxNestedLevels,
// MaxArrayElements, MaxMapPairs, etc.
//
// If there are any remaining bytes after the CBOR data item,
// an ExtraneousDataError is returned.
//
// WARNING: Valid doesn't check if encoded CBOR data item is valid (i.e. validity)
// and RFC 8949 distinctly defines what is "Valid" and what is "Well-formed".
//
// Deprecated: Valid is kept for compatibility and should not be used.
// Use Wellformed instead because it has a more appropriate name.
func Valid(data []byte) error {
return defaultDecMode.Valid(data)
}

// Wellformed checks whether data is a well-formed encoded CBOR data item and
// that it complies with default restrictions such as MaxNestedLevels,
// MaxArrayElements, MaxMapPairs, etc.
//
// If there are any remaining bytes after the CBOR data item,
// an ExtraneousDataError is returned.
func Wellformed(data []byte) error {
return defaultDecMode.Wellformed(data)
}

// Unmarshaler is the interface implemented by types that wish to unmarshal
// CBOR data themselves. The input is a valid CBOR value. UnmarshalCBOR
// must copy the CBOR data if it needs to use it after returning.
Expand Down Expand Up @@ -499,10 +520,32 @@ type DecMode interface {
//
// See the documentation for Unmarshal for details.
Unmarshal(data []byte, v interface{}) error
// Valid checks whether the CBOR data is complete and well-formed.

// Valid checks whether data is a well-formed encoded CBOR data item and
// that it complies with configurable restrictions such as MaxNestedLevels,
// MaxArrayElements, MaxMapPairs, etc.
//
// If there are any remaining bytes after the CBOR data item,
// an ExtraneousDataError is returned.
//
// WARNING: Valid doesn't check if encoded CBOR data item is valid (i.e. validity)
// and RFC 8949 distinctly defines what is "Valid" and what is "Well-formed".
//
// Deprecated: Valid is kept for compatibility and should not be used.
// Use Wellformed instead because it has a more appropriate name.
Valid(data []byte) error

// Wellformed checks whether data is a well-formed encoded CBOR data item and
// that it complies with configurable restrictions such as MaxNestedLevels,
// MaxArrayElements, MaxMapPairs, etc.
//
// If there are any remaining bytes after the CBOR data item,
// an ExtraneousDataError is returned.
Wellformed(data []byte) error

// NewDecoder returns a new decoder that reads from r using dm DecMode.
NewDecoder(r io.Reader) *Decoder

// DecOptions returns user specified options used to create this DecMode.
DecOptions() DecOptions
}
Expand Down Expand Up @@ -550,21 +593,42 @@ func (dm *decMode) DecOptions() DecOptions {
func (dm *decMode) Unmarshal(data []byte, v interface{}) error {
d := decoder{data: data, dm: dm}

// check valid
off := d.off // Save offset before data validation
err := d.valid(false) // don't allow any extra data after valid data item.
d.off = off // Restore offset
// Check well-formedness.
off := d.off // Save offset before data validation
err := d.wellformed(false) // don't allow any extra data after valid data item.
d.off = off // Restore offset
if err != nil {
return err
}

return d.value(v)
}

// Valid checks whether the CBOR data is complete and well-formed.
// Valid checks whether data is a well-formed encoded CBOR data item and
// that it complies with configurable restrictions such as MaxNestedLevels,
// MaxArrayElements, MaxMapPairs, etc.
//
// If there are any remaining bytes after the CBOR data item,
// an ExtraneousDataError is returned.
//
// WARNING: Valid doesn't check if encoded CBOR data item is valid (i.e. validity)
// and RFC 8949 distinctly defines what is "Valid" and what is "Well-formed".
//
// Deprecated: Valid is kept for compatibility and should not be used.
// Use Wellformed instead because it has a more appropriate name.
func (dm *decMode) Valid(data []byte) error {
return dm.Wellformed(data)
}

// Wellformed checks whether data is a well-formed encoded CBOR data item and
// that it complies with configurable restrictions such as MaxNestedLevels,
// MaxArrayElements, MaxMapPairs, etc.
//
// If there are any remaining bytes after the CBOR data item,
// an ExtraneousDataError is returned.
func (dm *decMode) Wellformed(data []byte) error {
d := decoder{data: data, dm: dm}
return d.valid(false)
return d.wellformed(false)
}

// NewDecoder returns a new decoder that reads from r using dm DecMode.
Expand All @@ -581,7 +645,7 @@ type decoder struct {
// value decodes CBOR data item into the value pointed to by v.
// If CBOR data item fails to be decoded into v,
// error is returned and offset is moved to the next CBOR data item.
// Precondition: d.data contains at least one valid CBOR data item.
// Precondition: d.data contains at least one well-formed CBOR data item.
func (d *decoder) value(v interface{}) error {
// v can't be nil, non-pointer, or nil pointer value.
if v == nil {
Expand Down
2 changes: 1 addition & 1 deletion stream.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (dec *Decoder) readNext() (int, error) {
if dec.off < len(dec.buf) {
dec.d.reset(dec.buf[dec.off:])
off := dec.off // Save offset before data validation
validErr = dec.d.valid(true)
validErr = dec.d.wellformed(true)
dec.off = off // Restore offset

if validErr == nil {
Expand Down
40 changes: 20 additions & 20 deletions valid.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (e *TagsMdError) Error() string {
return "cbor: CBOR tag isn't allowed"
}

// ExtraneousDataError indicates found extraneous data following valid CBOR message.
// ExtraneousDataError indicates found extraneous data following well-formed CBOR data item.
type ExtraneousDataError struct {
numOfBytes int // number of bytes of extraneous data
index int // location of extraneous data
Expand All @@ -78,15 +78,15 @@ func (e *ExtraneousDataError) Error() string {
return "cbor: " + strconv.Itoa(e.numOfBytes) + " bytes of extraneous data starting at index " + strconv.Itoa(e.index)
}

// valid checks whether the CBOR data is complete and well-formed.
// wellformed checks whether the CBOR data item is well-formed.
// allowExtraData indicates if extraneous data is allowed after the CBOR data item.
// - use allowExtraData = true when using Decoder.Decode()
// - use allowExtraData = false when using Unmarshal()
func (d *decoder) valid(allowExtraData bool) error {
func (d *decoder) wellformed(allowExtraData bool) error {
if len(d.data) == d.off {
return io.EOF
}
_, err := d.validInternal(0)
_, err := d.wellformedInternal(0)
if err == nil {
if !allowExtraData && d.off != len(d.data) {
err = &ExtraneousDataError{len(d.data) - d.off, d.off}
Expand All @@ -95,9 +95,9 @@ func (d *decoder) valid(allowExtraData bool) error {
return err
}

// validInternal checks data's well-formedness and returns max depth and error.
func (d *decoder) validInternal(depth int) (int, error) {
t, ai, val, err := d.validHead()
// wellformedInternal checks data's well-formedness and returns max depth and error.
func (d *decoder) wellformedInternal(depth int) (int, error) {
t, ai, val, err := d.wellformedHead()
if err != nil {
return 0, err
}
Expand All @@ -108,7 +108,7 @@ func (d *decoder) validInternal(depth int) (int, error) {
if d.dm.indefLength == IndefLengthForbidden {
return 0, &IndefiniteLengthError{t}
}
return d.validIndefiniteString(t, depth)
return d.wellformedIndefiniteString(t, depth)
}
valInt := int(val)
if valInt < 0 {
Expand All @@ -129,7 +129,7 @@ func (d *decoder) validInternal(depth int) (int, error) {
if d.dm.indefLength == IndefLengthForbidden {
return 0, &IndefiniteLengthError{t}
}
return d.validIndefiniteArrayOrMap(t, depth)
return d.wellformedIndefiniteArrayOrMap(t, depth)
}

valInt := int(val)
Expand All @@ -156,7 +156,7 @@ func (d *decoder) validInternal(depth int) (int, error) {
for j := 0; j < count; j++ {
for i := 0; i < valInt; i++ {
var dpt int
if dpt, err = d.validInternal(depth); err != nil {
if dpt, err = d.wellformedInternal(depth); err != nil {
return 0, err
}
if dpt > maxDepth {
Expand All @@ -178,7 +178,7 @@ func (d *decoder) validInternal(depth int) (int, error) {
if cborType(d.data[d.off]&0xe0) != cborTypeTag {
break
}
if _, _, _, err = d.validHead(); err != nil {
if _, _, _, err = d.wellformedHead(); err != nil {
return 0, err
}
depth++
Expand All @@ -187,13 +187,13 @@ func (d *decoder) validInternal(depth int) (int, error) {
}
}
// Check tag content.
return d.validInternal(depth)
return d.wellformedInternal(depth)
}
return depth, nil
}

// validIndefiniteString checks indefinite length byte/text string's well-formedness and returns max depth and error.
func (d *decoder) validIndefiniteString(t cborType, depth int) (int, error) {
// wellformedIndefiniteString checks indefinite length byte/text string's well-formedness and returns max depth and error.
func (d *decoder) wellformedIndefiniteString(t cborType, depth int) (int, error) {
var err error
for {
if len(d.data) == d.off {
Expand All @@ -211,15 +211,15 @@ func (d *decoder) validIndefiniteString(t cborType, depth int) (int, error) {
if (d.data[d.off] & 0x1f) == 31 {
return 0, &SyntaxError{"cbor: indefinite-length " + t.String() + " chunk is not definite-length"}
}
if depth, err = d.validInternal(depth); err != nil {
if depth, err = d.wellformedInternal(depth); err != nil {
return 0, err
}
}
return depth, nil
}

// validIndefiniteArrayOrMap checks indefinite length array/map's well-formedness and returns max depth and error.
func (d *decoder) validIndefiniteArrayOrMap(t cborType, depth int) (int, error) {
// wellformedIndefiniteArrayOrMap checks indefinite length array/map's well-formedness and returns max depth and error.
func (d *decoder) wellformedIndefiniteArrayOrMap(t cborType, depth int) (int, error) {
var err error
maxDepth := depth
i := 0
Expand All @@ -232,7 +232,7 @@ func (d *decoder) validIndefiniteArrayOrMap(t cborType, depth int) (int, error)
break
}
var dpt int
if dpt, err = d.validInternal(depth); err != nil {
if dpt, err = d.wellformedInternal(depth); err != nil {
return 0, err
}
if dpt > maxDepth {
Expand All @@ -255,7 +255,7 @@ func (d *decoder) validIndefiniteArrayOrMap(t cborType, depth int) (int, error)
return maxDepth, nil
}

func (d *decoder) validHead() (t cborType, ai byte, val uint64, err error) {
func (d *decoder) wellformedHead() (t cborType, ai byte, val uint64, err error) {
dataLen := len(d.data) - d.off
if dataLen == 0 {
return 0, 0, 0, io.ErrUnexpectedEOF
Expand Down Expand Up @@ -308,7 +308,7 @@ func (d *decoder) validHead() (t cborType, ai byte, val uint64, err error) {
switch t {
case cborTypePositiveInt, cborTypeNegativeInt, cborTypeTag:
return 0, 0, 0, &SyntaxError{"cbor: invalid additional information " + strconv.Itoa(int(ai)) + " for type " + t.String()}
case cborTypePrimitives: // 0xff (break code) should not be outside validIndefinite().
case cborTypePrimitives: // 0xff (break code) should not be outside wellformedIndefinite().
return 0, 0, 0, &SyntaxError{"cbor: unexpected \"break\" code"}
}
return t, ai, val, nil
Expand Down
36 changes: 18 additions & 18 deletions valid_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ import (

func TestValid1(t *testing.T) {
for _, mt := range marshalTests {
if err := Valid(mt.cborData); err != nil {
t.Errorf("Valid() returned error %v", err)
if err := Wellformed(mt.cborData); err != nil {
t.Errorf("Wellformed() returned error %v", err)
}
}
}

func TestValid2(t *testing.T) {
for _, mt := range marshalTests {
dm, _ := DecOptions{DupMapKey: DupMapKeyEnforcedAPF}.DecMode()
if err := dm.Valid(mt.cborData); err != nil {
t.Errorf("Valid() returned error %v", err)
if err := dm.Wellformed(mt.cborData); err != nil {
t.Errorf("Wellformed() returned error %v", err)
}
}
}
Expand All @@ -39,17 +39,17 @@ func TestValidExtraneousData(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := Valid(tc.cborData)
err := Wellformed(tc.cborData)
if err == nil {
t.Errorf("Valid(0x%x) didn't return an error", tc.cborData)
t.Errorf("Wellformed(0x%x) didn't return an error", tc.cborData)
} else {
ederr, ok := err.(*ExtraneousDataError)
if !ok {
t.Errorf("Valid(0x%x) error type %T, want *ExtraneousDataError", tc.cborData, err)
t.Errorf("Wellformed(0x%x) error type %T, want *ExtraneousDataError", tc.cborData, err)
} else if ederr.numOfBytes != tc.extraneousDataNumOfBytes {
t.Errorf("Valid(0x%x) returned %d bytes of extraneous data, want %d", tc.cborData, ederr.numOfBytes, tc.extraneousDataNumOfBytes)
t.Errorf("Wellformed(0x%x) returned %d bytes of extraneous data, want %d", tc.cborData, ederr.numOfBytes, tc.extraneousDataNumOfBytes)
} else if ederr.index != tc.extraneousDataIndex {
t.Errorf("Valid(0x%x) returned extraneous data index %d, want %d", tc.cborData, ederr.index, tc.extraneousDataIndex)
t.Errorf("Wellformed(0x%x) returned extraneous data index %d, want %d", tc.cborData, ederr.index, tc.extraneousDataIndex)
}
}
})
Expand All @@ -63,8 +63,8 @@ func TestValidOnStreamingData(t *testing.T) {
}
d := decoder{data: buf.Bytes(), dm: defaultDecMode}
for i := 0; i < len(marshalTests); i++ {
if err := d.valid(true); err != nil {
t.Errorf("valid() returned error %v", err)
if err := d.wellformed(true); err != nil {
t.Errorf("wellformed() returned error %v", err)
}
}
}
Expand Down Expand Up @@ -111,12 +111,12 @@ func TestDepth(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
d := decoder{data: tc.cborData, dm: defaultDecMode}
depth, err := d.validInternal(0)
depth, err := d.wellformedInternal(0)
if err != nil {
t.Errorf("valid(0x%x) returned error %v", tc.cborData, err)
t.Errorf("wellformed(0x%x) returned error %v", tc.cborData, err)
}
if depth != tc.wantDepth {
t.Errorf("valid(0x%x) returned depth %d, want %d", tc.cborData, depth, tc.wantDepth)
t.Errorf("wellformed(0x%x) returned depth %d, want %d", tc.cborData, depth, tc.wantDepth)
}
})
}
Expand Down Expand Up @@ -176,12 +176,12 @@ func TestDepthError(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
dm, _ := tc.opts.decMode()
d := decoder{data: tc.cborData, dm: dm}
if _, err := d.validInternal(0); err == nil {
t.Errorf("valid(0x%x) didn't return an error", tc.cborData)
if _, err := d.wellformedInternal(0); err == nil {
t.Errorf("wellformed(0x%x) didn't return an error", tc.cborData)
} else if _, ok := err.(*MaxNestedLevelError); !ok {
t.Errorf("valid(0x%x) returned wrong error type %T, want (*MaxNestedLevelError)", tc.cborData, err)
t.Errorf("wellformed(0x%x) returned wrong error type %T, want (*MaxNestedLevelError)", tc.cborData, err)
} else if err.Error() != tc.wantErrorMsg {
t.Errorf("valid(0x%x) returned error %q, want error %q", tc.cborData, err.Error(), tc.wantErrorMsg)
t.Errorf("wellformed(0x%x) returned error %q, want error %q", tc.cborData, err.Error(), tc.wantErrorMsg)
}
})
}
Expand Down

0 comments on commit c9e9b26

Please sign in to comment.