Fixed bugs in varint encoding / decoding

Changed enum value encoding to unsigned varint

Author: bminer

encoding-format.md

@@ -36,7 +36,7 @@ The following table describes how schemer encodes different values. Nullable val
 | Variable-size Integer    | Each byte's most significant bit indicates more bytes follow. Lower 7 bits are concatenated (in big-endian order) to form [ZigZag-encoded integer](https://developers.google.com/protocol-buffers/docs/encoding?csw=1#types). |
 | Floating-point Number    | Exactly `4 << n` bytes corresponding to the IEEE 754 binary representation of the floating-point number |
 | Complex Number           | Exactly `4 << n` bytes for each floating-point number `a` and `b` where the complex number is `a + bi`. |
-| Enum                     | Stored as an unsigned fixed-size integer (see above), where `n` is determined by the number of enumerated values |
+| Enum                     | Stored as an unsigned variable-size integer (see above) |
 | Boolean                  | A single boolean value is encoded as 1 byte. 0 indicates `false` and any other value indicates `true`. |
 | Fixed-Length String      | UTF-8 encoding of the string, padded with spaces to fit within allotted space. |
 | Variable-Length String   | Length of the string is encoded as an unsigned variable-size integer followed by UTF-8 encoding of the string |
@@ -56,4 +56,4 @@ The following optimizations modify the above rules for encoding values:
 * For objects with a fixed number of nullable or boolean fields, a single bit map is placed a the start of the object
 * For arrays of a nullable type, each group of 8 elements is preceded by the corresponding null bit map
 * Arrays of type boolean are encoded as bit maps. The final byte's least significant bits are padded with zeros.
-* Variable-length arrays and objects w/variable fields may be stored in blocks, where each block indicates the number of elements or key-value pairs. A block size of 0 indicates the end of the array or object. A negative block size indicates that the block size is followed by the number of bytes in the block.
+* Variable-length arrays and objects w/variable fields may be stored in blocks, where each block indicates the number of elements or key-value pairs. A block size of 0 indicates the end of the array or object. A negative block size indicates that the block size is followed by the number of bytes in the block.

enum.go

@@ -75,7 +75,10 @@ func (s *EnumSchema) Encode(w io.Writer, i interface{}) error {
 
 // EncodeValue uses the schema to write the encoded value of v to the output streamtream
 func (s *EnumSchema) EncodeValue(w io.Writer, v reflect.Value) error {
-	varIntSchema := VarIntSchema{Signed: true, SchemaOptions: SchemaOptions{nullable: s.Nullable()}}
+	varIntSchema := VarIntSchema{
+		Signed:        false,
+		SchemaOptions: SchemaOptions{nullable: s.Nullable()},
+	}
 	return varIntSchema.EncodeValue(w, v)
 }
 
@@ -92,19 +95,22 @@ func (s *EnumSchema) DecodeValue(r io.Reader, v reflect.Value) error {
 
 	// first we decode the actual encoded binary value
 
-	varIntSchema := VarIntSchema{Signed: true, SchemaOptions: SchemaOptions{nullable: s.Nullable()}}
+	varIntSchema := VarIntSchema{
+		Signed:        false,
+		SchemaOptions: SchemaOptions{nullable: s.Nullable()},
+	}
 
-	var decodedVarInt int64
-	var intPtr *int64 = &decodedVarInt // we pass in a pointer to varIntSchema.Decode so we can potentially
+	var decodedVal uint64
+	var valPtr *uint64 = &decodedVal // we pass in a pointer to varIntSchema.Decode so we can potentially
 	// get back a nil value [in case we are dealing with a nullable type]
 
-	err := varIntSchema.Decode(r, &intPtr)
+	err := varIntSchema.Decode(r, &valPtr)
 	if err != nil {
 		return err
 	}
 
 	// now we check to see if varIntSchema.Decode returned us a nil value
-	if intPtr == nil {
+	if valPtr == nil {
 		if v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface {
 			if v.CanSet() {
 				v.Set(reflect.Zero(v.Type()))
@@ -147,7 +153,7 @@ func (s *EnumSchema) DecodeValue(r io.Reader, v reflect.Value) error {
 
 	// check to see if the decoded value is in our map of enumerated values
 	if s.Values != nil {
-		if _, ok := s.Values[int(decodedVarInt)]; !ok {
+		if _, ok := s.Values[int(decodedVal)]; !ok {
 			// however, maybe it makes sense to allow this scenario
 			// when weak decoding is specified??
 			if !s.WeakDecoding() {
@@ -173,10 +179,11 @@ func (s *EnumSchema) DecodeValue(r io.Reader, v reflect.Value) error {
 	case reflect.Int32:
 		fallthrough
 	case reflect.Int64:
-		if v.OverflowInt(decodedVarInt) {
-			return fmt.Errorf("decoded value %d overflows destination %v", decodedVarInt, k)
+		intVal := int64(decodedVal)
+		if uint64(intVal) != decodedVal || v.OverflowInt(intVal) {
+			return fmt.Errorf("decoded value %d overflows destination %v", decodedVal, k)
 		}
-		v.SetInt(int64(decodedVarInt))
+		v.SetInt(intVal)
 	case reflect.Uint:
 		fallthrough
 	case reflect.Uint8:
@@ -186,31 +193,27 @@ func (s *EnumSchema) DecodeValue(r io.Reader, v reflect.Value) error {
 	case reflect.Uint32:
 		fallthrough
 	case reflect.Uint64:
-		if decodedVarInt < 0 {
-			return fmt.Errorf("decoded value %d incompatible with %v", decodedVarInt, k)
-		}
-		uintVal := uint64(decodedVarInt)
-		if v.OverflowUint(uintVal) {
-			return fmt.Errorf("decoded value %d overflows destination %v", uintVal, k)
+		if v.OverflowUint(decodedVal) {
+			return fmt.Errorf("decoded value %d overflows destination %v", decodedVal, k)
 		}
-		v.SetUint(uintVal)
+		v.SetUint(decodedVal)
 	case reflect.String:
 		if !s.WeakDecoding() {
 			return fmt.Errorf("cannot decode enum to string without weak decoding enabled")
 		}
 
 		// if we have the map, return the string value of the constant
 		if s.Values != nil {
-			if _, ok := s.Values[int(decodedVarInt)]; ok {
-				v.SetString(s.Values[int(decodedVarInt)])
+			if _, ok := s.Values[int(decodedVal)]; ok {
+				v.SetString(s.Values[int(decodedVal)])
 				return nil
 			}
 		}
 
 		// otherwise, just return a string version of the decoded integer value
-		v.SetString(strconv.FormatInt(decodedVarInt, 10))
+		v.SetString(strconv.FormatUint(decodedVal, 10))
 	default:
-		return fmt.Errorf("decoded value %d incompatible with %v", decodedVarInt, k)
+		return fmt.Errorf("decoded value %d incompatible with %v", decodedVal, k)
 	}
 
 	return nil

varint.go

@@ -87,11 +87,19 @@ func (s *VarIntSchema) EncodeValue(w io.Writer, v reflect.Value) error {
 	case reflect.Int64:
 		intVal := v.Int()
 		// Write value
-		uintVal := uint64(intVal) << 1
-		if intVal < 0 {
-			uintVal = ^uintVal
+		if s.Signed {
+			uintVal := uint64(intVal) << 1
+			if intVal < 0 {
+				uintVal = ^uintVal
+			}
+			return WriteUvarint(w, uintVal)
+		} else {
+			if intVal < 0 {
+				return fmt.Errorf("cannot encode negative integer")
+			}
+			uintVal := uint64(intVal)
+			return WriteUvarint(w, uintVal)
 		}
-		return WriteUvarint(w, uintVal)
 	case reflect.Uint:
 		fallthrough
 	case reflect.Uint8:
@@ -102,7 +110,11 @@ func (s *VarIntSchema) EncodeValue(w io.Writer, v reflect.Value) error {
 		fallthrough
 	case reflect.Uint64:
 		uintVal := v.Uint()
-		return WriteUvarint(w, uintVal)
+		if s.Signed {
+			return WriteUvarint(w, uintVal<<1)
+		} else {
+			return WriteUvarint(w, uintVal)
+		}
 	}
 
 	return nil
@@ -239,10 +251,11 @@ func (s *VarIntSchema) DecodeValue(r io.Reader, v reflect.Value) error {
 		case reflect.Int32:
 			fallthrough
 		case reflect.Int64:
-			if v.OverflowUint(uintVal) {
+			intVal := int64(uintVal)
+			if uint64(intVal) != uintVal || v.OverflowInt(intVal) {
 				return fmt.Errorf("decoded value %d overflows destination %v", uintVal, k)
 			}
-			v.SetUint(uintVal)
+			v.SetInt(intVal)
 		case reflect.Uint:
 			fallthrough
 		case reflect.Uint8: