Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for discriminated unions and optional data #6

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions xdr2/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/

package xdr_test
package xdr
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sure @davecgh has a reason to split test from package. Seems like a gratuitous change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is that this style of testing makes forking impossible, because of the absolute import path used to import the xdr2 package. And unfortunately, you can't use relative imports in this context.

So for instance, the CI would be unable to test this pull request because it would use the upstream version of the xdr2 package, rather than the version in this branch.


import (
"bytes"
"testing"
"unsafe"

"github.com/davecgh/go-xdr/xdr2"
)

// BenchmarkUnmarshal benchmarks the Unmarshal function by using a dummy
Expand All @@ -47,7 +45,7 @@ func BenchmarkUnmarshal(b *testing.B) {

for i := 0; i < b.N; i++ {
r := bytes.NewReader(encodedData)
_, _ = xdr.Unmarshal(r, &h)
_, _ = Unmarshal(r, &h)
}
b.SetBytes(int64(len(encodedData)))
}
Expand All @@ -70,7 +68,7 @@ func BenchmarkMarshal(b *testing.B) {

for i := 0; i < b.N; i++ {
w.Reset()
_, _ = xdr.Marshal(w, &h)
_, _ = Marshal(w, &h)
}
b.SetBytes(int64(size))
}
57 changes: 51 additions & 6 deletions xdr2/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"io"
"math"
"reflect"
"strconv"
"time"
)

Expand Down Expand Up @@ -507,6 +508,7 @@ func (d *Decoder) decodeArray(v reflect.Value, ignoreOpaque bool) (int, error) {
// XDR encoded elements in the order of their declaration in the struct
func (d *Decoder) decodeStruct(v reflect.Value) (int, error) {
var n int
var union string
vt := v.Type()
for i := 0; i < v.NumField(); i++ {
// Skip unexported fields.
Expand All @@ -515,13 +517,36 @@ func (d *Decoder) decodeStruct(v reflect.Value) (int, error) {
continue
}

vf := v.Field(i)
tag := parseTag(vtf.Tag)

// RFC Section 4.19 - Optional data
if tag.Get("optional") == "true" {
if vf.Type().Kind() != reflect.Ptr {
msg := fmt.Sprintf("optional must be a pointer, not '%v'",
vf.Type().String())
err := unmarshalError("decodeStruct", ErrBadOptional,
msg, nil, nil)
return n, err
}

hasopt, n2, err := d.DecodeBool()
n += n2
if err != nil {
return n, err
}
if !hasopt {
continue
}
}

// Indirect through pointers allocating them as needed and
// ensure the field is settable.
vf := v.Field(i)
vf, err := d.indirect(vf)
if err != nil {
return n, err
}

if !vf.CanSet() {
msg := fmt.Sprintf("can't decode to unsettable '%v'",
vf.Type().String())
Expand All @@ -532,8 +557,7 @@ func (d *Decoder) decodeStruct(v reflect.Value) (int, error) {

// Handle non-opaque data to []uint8 and [#]uint8 based on
// struct tag.
tag := vtf.Tag.Get("xdropaque")
if tag == "false" {
if tag.Get("opaque") == "false" {
switch vf.Kind() {
case reflect.Slice:
n2, err := d.decodeArray(vf, true)
Expand All @@ -553,25 +577,46 @@ func (d *Decoder) decodeStruct(v reflect.Value) (int, error) {
}
}

if union != "" {
ucase := tag.Get("unioncase")
if ucase != "" && ucase != union {
continue
}
}

// Decode each struct field.
n2, err := d.decode(vf)
n += n2
if err != nil {
return n, err
}

if tag.Get("union") == "true" {
if vf.Type().ConvertibleTo(reflect.TypeOf(0)) {
union = strconv.Itoa(int(vf.Convert(reflect.TypeOf(0)).Int()))
} else if vf.Kind() == reflect.Bool {
if vf.Bool() {
union = "1"
} else {
union = "0"
}
} else {
msg := fmt.Sprintf("type '%s' is not valid", vf.Kind().String())
return n, unmarshalError("decodeStruct", ErrBadDiscriminant, msg, nil, nil)
}
}

}

return n, nil
}

// RFC Section 4.15 - Discriminated Union
// RFC Section 4.16 - Void
// RFC Section 4.17 - Constant
// RFC Section 4.18 - Typedef
// RFC Section 4.19 - Optional data
// RFC Sections 4.15 though 4.19 only apply to the data specification language
// which is not implemented by this package. In the case of discriminated
// unions, struct tags are used to perform a similar function.
// which is not implemented by this package.

// decodeMap treats the next bytes as an XDR encoded variable array of 2-element
// structures whose fields are of the same type as the map keys and elements
Expand Down
66 changes: 61 additions & 5 deletions xdr2/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/

package xdr_test
package xdr

import (
"bytes"
Expand All @@ -23,8 +23,6 @@ import (
"reflect"
"testing"
"time"

. "github.com/davecgh/go-xdr/xdr2"
)

// subTest is used to allow testing of the Unmarshal function into struct fields
Expand Down Expand Up @@ -60,8 +58,35 @@ type allTypesTest struct {

// opaqueStruct is used to test handling of uint8 slices and arrays.
type opaqueStruct struct {
Slice []uint8 `xdropaque:"false"`
Array [1]uint8 `xdropaque:"false"`
Slice []uint8 `xdr:"opaque=false"`
Array [1]uint8 `xdropaque:"false"` // old syntax, backward compatibility
}

type unionStruct struct {
UV int `xdr:"union"`
V0 byte `xdr:"unioncase=0"`
V1 byte `xdr:"unioncase=1"`
VA byte
}

type unionBoolStruct struct {
UV bool `xdr:"union"`
V0 byte `xdr:"unioncase=0"`
V1 byte `xdr:"unioncase=1"`
VA byte
}

type invalidUnionStruct struct {
UV string `xdr:"union"`
}

type optionalDataStruct struct {
Data int
Next *optionalDataStruct `xdr:"optional"`
}

type invalidOptionalDataStruct struct {
Data int `xdr:"optional"`
}

// testExpectedURet is a convenience method to test an expected number of bytes
Expand Down Expand Up @@ -350,6 +375,37 @@ func TestUnmarshal(t *testing.T) {
{[]byte{0x00, 0x00, 0x00}, opaqueStruct{}, 3, &UnmarshalError{ErrorCode: ErrIO}},
{[]byte{0x00, 0x00, 0x00, 0x00, 0x00}, opaqueStruct{}, 5, &UnmarshalError{ErrorCode: ErrIO}},

// Discriminated unions
{[]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01},
unionStruct{0, 1, 0, 1},
12, nil},
{[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01},
unionStruct{1, 0, 1, 1},
12, nil},
{[]byte{0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01},
unionStruct{2, 0, 0, 1},
8, nil},
{[]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01},
unionBoolStruct{false, 1, 0, 1},
12, nil},
{[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01},
unionBoolStruct{true, 0, 1, 1},
12, nil},
{[]byte{0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01},
invalidUnionStruct{},
8, &UnmarshalError{ErrorCode: ErrBadDiscriminant}},

// Optional data
{[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00},
optionalDataStruct{1, &optionalDataStruct{2, nil}},
16, nil},
{[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00},
optionalDataStruct{1, nil},
7, &UnmarshalError{ErrorCode: ErrIO}},
{[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00},
invalidOptionalDataStruct{},
0, &UnmarshalError{ErrorCode: ErrBadOptional}},

// Expected errors
{nil, nilInterface, 0, &UnmarshalError{ErrorCode: ErrNilInterface}},
{nil, &nilInterface, 0, &UnmarshalError{ErrorCode: ErrNilInterface}},
Expand Down
39 changes: 37 additions & 2 deletions xdr2/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,15 @@ and Unmarshal functions has specific details of how the mapping proceeds.
[#]byte <-> XDR Fixed-Length Opaque Data
[]<type> <-> XDR Variable-Length Array
[#]<type> <-> XDR Fixed-Length Array
struct <-> XDR Structure
*<type> <-> XDR Optional data (when marked with struct tag `xdr:"optional"`)
struct <-> XDR Structure or Discriminated Unions
map <-> XDR Variable-Length Array of two-element XDR Structures
time.Time <-> XDR String encoded with RFC3339 nanosecond precision

Notes and Limitations:

* Automatic marshalling and unmarshalling of variable and fixed-length
arrays of uint8s require a special struct tag `xdropaque:"false"`
arrays of uint8s require a special struct tag `xdr:"opaque=false"`
since byte slices and byte arrays are assumed to be opaque data and
byte is a Go alias for uint8 thus indistinguishable under reflection
* Channel, complex, and function types cannot be encoded
Expand Down Expand Up @@ -159,6 +160,40 @@ manually decode XDR primitives for complex scenarios where automatic
reflection-based decoding won't work. The included examples provide a sample of
manual usage via a Decoder.


Discriminated Unions

Discriminated unions are marshalled via Go structs, using special struct tags
to mark the discriminant and the different cases. For instance:

type ReturnValue struct {
Status int `xdr:"union"`
StatusOk struct {
Width int
Height int
} `xdr:"unioncase=0"`
StatusError struct {
ErrMsg string
} `xdr:"unioncase=-1"`
}

The Status field is the discriminant of the union, and is always serialized;
if its value is 0, the StatusOK struct is serialized while the StatusErr struct
is ignored; if its value is -1, the opposite happens. If the value is different
from both 0 and -1, only the Status field is serialized. Any additional field
not marked with unioncase is always serialized as normal.

You are not forced to use sub-structures; for instance, the following is also
valid:

type ReturnValue struct {
Status int `xdr:"union"`
Width int `xdr:"unioncase=0"`
Height int `xdr:"unioncase=0"`
ErrMsg string `xdr:"unioncase=-1"`
}


Errors

All errors are either of type UnmarshalError or MarshalError. Both provide
Expand Down
Loading