From b201f0f879694877bbcfb91a8d195326bedec5fb Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Sun, 13 Sep 2015 14:59:54 +0200 Subject: [PATCH 1/8] Remove xdr_test package to ease forking. The usage of absolute import paths in testsuite is not compatible with allowing a package to easily fork. One has to replace all global paths in test files. I tried to use relative import paths such as "../xdr2" but there appears to be a Go 1.5 bug for which the internal_test.go functions TstEncode/TstDecode are not exposed to tests. --- xdr2/bench_test.go | 8 +++----- xdr2/decode_test.go | 4 +--- xdr2/encode_test.go | 4 +--- xdr2/error_test.go | 4 +--- xdr2/example_test.go | 12 +++++------- xdr2/fixedIO_test.go | 2 +- 6 files changed, 12 insertions(+), 22 deletions(-) diff --git a/xdr2/bench_test.go b/xdr2/bench_test.go index 0fea4db..531bb1a 100644 --- a/xdr2/bench_test.go +++ b/xdr2/bench_test.go @@ -14,14 +14,12 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -package xdr_test +package xdr import ( "bytes" "testing" "unsafe" - - "github.com/davecgh/go-xdr/xdr2" ) // BenchmarkUnmarshal benchmarks the Unmarshal function by using a dummy @@ -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))) } @@ -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)) } diff --git a/xdr2/decode_test.go b/xdr2/decode_test.go index 310629d..1c3dfde 100644 --- a/xdr2/decode_test.go +++ b/xdr2/decode_test.go @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -package xdr_test +package xdr import ( "bytes" @@ -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 diff --git a/xdr2/encode_test.go b/xdr2/encode_test.go index 1b04cfa..630ce89 100644 --- a/xdr2/encode_test.go +++ b/xdr2/encode_test.go @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -package xdr_test +package xdr import ( "fmt" @@ -22,8 +22,6 @@ import ( "reflect" "testing" "time" - - . "github.com/davecgh/go-xdr/xdr2" ) // testExpectedMRet is a convenience method to test an expected number of bytes diff --git a/xdr2/error_test.go b/xdr2/error_test.go index ac9c156..1a2c010 100644 --- a/xdr2/error_test.go +++ b/xdr2/error_test.go @@ -14,13 +14,11 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -package xdr_test +package xdr import ( "errors" "testing" - - . "github.com/davecgh/go-xdr/xdr2" ) // TestErrorCodeStringer tests the stringized output for the ErrorCode type. diff --git a/xdr2/example_test.go b/xdr2/example_test.go index 1272862..2e71251 100644 --- a/xdr2/example_test.go +++ b/xdr2/example_test.go @@ -14,13 +14,11 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -package xdr_test +package xdr import ( "bytes" "fmt" - - "github.com/davecgh/go-xdr/xdr2" ) // This example demonstrates how to use Marshal to automatically XDR encode @@ -40,7 +38,7 @@ func ExampleMarshal() { // Use marshal to automatically determine the appropriate underlying XDR // types and encode. var w bytes.Buffer - bytesWritten, err := xdr.Marshal(&w, &h) + bytesWritten, err := Marshal(&w, &h) if err != nil { fmt.Println(err) return @@ -78,7 +76,7 @@ func ExampleUnmarshal() { // Declare a variable to provide Unmarshal with a concrete type and // instance to decode into. var h ImageHeader - bytesRead, err := xdr.Unmarshal(bytes.NewReader(encodedData), &h) + bytesRead, err := Unmarshal(bytes.NewReader(encodedData), &h) if err != nil { fmt.Println(err) return @@ -111,7 +109,7 @@ func ExampleNewDecoder() { } // Get a new decoder for manual decoding. - dec := xdr.NewDecoder(bytes.NewReader(encodedData)) + dec := NewDecoder(bytes.NewReader(encodedData)) signature, _, err := dec.DecodeFixedOpaque(3) if err != nil { @@ -167,7 +165,7 @@ func ExampleNewEncoder() { // Get a new encoder for manual encoding. var w bytes.Buffer - enc := xdr.NewEncoder(&w) + enc := NewEncoder(&w) _, err := enc.EncodeFixedOpaque(signature) if err != nil { diff --git a/xdr2/fixedIO_test.go b/xdr2/fixedIO_test.go index d329a9a..baac54a 100644 --- a/xdr2/fixedIO_test.go +++ b/xdr2/fixedIO_test.go @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -package xdr_test +package xdr import ( "io" From 8564dcc061136c8674f010897d1e7de5e8162863 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Sun, 13 Sep 2015 16:24:22 +0200 Subject: [PATCH 2/8] Add support for discriminated unions. Unions are represented by normal Go structs, using different struct tags to pinpoint the union discriminant, and to select the various cases. Since more struct tags were required, I introduced a new general tag called "xdr" with key/value options, and thus changed the previous "xdropaque" struct tag into the new canonical name form `xdr:"opaque=false"`, while retaining the previous form for backward compatibility. --- xdr2/decode.go | 27 +++++++++++++--- xdr2/decode_test.go | 29 +++++++++++++++-- xdr2/doc.go | 38 ++++++++++++++++++++-- xdr2/encode.go | 32 +++++++++++++++---- xdr2/encode_test.go | 12 +++++++ xdr2/error.go | 5 +++ xdr2/tag.go | 78 +++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 206 insertions(+), 15 deletions(-) create mode 100644 xdr2/tag.go diff --git a/xdr2/decode.go b/xdr2/decode.go index 494dae6..0cf7ccf 100644 --- a/xdr2/decode.go +++ b/xdr2/decode.go @@ -21,6 +21,7 @@ import ( "io" "math" "reflect" + "strconv" "time" ) @@ -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. @@ -530,10 +532,11 @@ func (d *Decoder) decodeStruct(v reflect.Value) (int, error) { return n, err } + tag := parseTag(vtf.Tag) + // 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) @@ -553,25 +556,39 @@ 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)) { + msg := fmt.Sprintf("type '%s' is not valid", v.Kind().String()) + return n, unmarshalError("decodeStruct", ErrBadDiscriminant, msg, nil, nil) + } + union = strconv.Itoa(int(vf.Int())) + } + } 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 diff --git a/xdr2/decode_test.go b/xdr2/decode_test.go index 1c3dfde..51cdcd0 100644 --- a/xdr2/decode_test.go +++ b/xdr2/decode_test.go @@ -58,8 +58,19 @@ 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 `xdr:"opaque=false"` +} + +type unionStruct struct { + UV int `xdr:"union"` + V0 byte `xdr:"unioncase=0"` + V1 byte `xdr:"unioncase=1"` + VA byte +} + +type invalidUnionStruct struct { + UV string `xdr:"union"` } // testExpectedURet is a convenience method to test an expected number of bytes @@ -348,6 +359,20 @@ 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, 0x02, 0x00, 0x00, 0x00, 0x01}, + invalidUnionStruct{}, + 8, &UnmarshalError{ErrorCode: ErrBadDiscriminant}}, + // Expected errors {nil, nilInterface, 0, &UnmarshalError{ErrorCode: ErrNilInterface}}, {nil, &nilInterface, 0, &UnmarshalError{ErrorCode: ErrNilInterface}}, diff --git a/xdr2/doc.go b/xdr2/doc.go index 8823d62..7cae07f 100644 --- a/xdr2/doc.go +++ b/xdr2/doc.go @@ -66,14 +66,14 @@ and Unmarshal functions has specific details of how the mapping proceeds. [#]byte <-> XDR Fixed-Length Opaque Data [] <-> XDR Variable-Length Array [#] <-> XDR Fixed-Length Array - struct <-> XDR Structure + 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 @@ -159,6 +159,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 diff --git a/xdr2/encode.go b/xdr2/encode.go index 7bac268..8d716cf 100644 --- a/xdr2/encode.go +++ b/xdr2/encode.go @@ -21,6 +21,7 @@ import ( "io" "math" "reflect" + "strconv" "time" ) @@ -440,6 +441,7 @@ func (enc *Encoder) encodeArray(v reflect.Value, ignoreOpaque bool) (int, error) // XDR encoded elements in the order of their declaration in the struct func (enc *Encoder) encodeStruct(v reflect.Value) (int, error) { var n int + var union string vt := v.Type() for i := 0; i < v.NumField(); i++ { // Skip unexported fields and indirect through pointers. @@ -450,9 +452,10 @@ func (enc *Encoder) encodeStruct(v reflect.Value) (int, error) { vf := v.Field(i) vf = enc.indirect(vf) + tag := parseTag(vtf.Tag) + // 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 := enc.encodeArray(vf, true) @@ -472,6 +475,25 @@ func (enc *Encoder) encodeStruct(v reflect.Value) (int, error) { } } + // RFC Section 4.15 - Discriminated Union + // The tag option "union" marks the discriminant in the struct; the tag + // option "unioncase=N" marks a struct field that is only serialized + // when the discriminant has the specified value. + if tag.Get("union") == "true" { + if !vf.Type().ConvertibleTo(reflect.TypeOf(0)) { + msg := fmt.Sprintf("type '%s' is not valid", v.Kind().String()) + return n, marshalError("encodeStruct", ErrBadDiscriminant, msg, nil, nil) + } + union = strconv.Itoa(int(vf.Int())) + } + + if union != "" { + ucase := tag.Get("unioncase") + if ucase != "" && ucase != union { + continue + } + } + // Encode each struct field. n2, err := enc.encode(vf) n += n2 @@ -483,14 +505,12 @@ func (enc *Encoder) encodeStruct(v reflect.Value) (int, error) { 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. +// RFC Sections 4.16 though 4.19 only apply to the data specification language +// which is not implemented by this package. // encodeMap treats the map represented by the passed reflection value as a // variable-length array of 2-element structures whose fields are of the same diff --git a/xdr2/encode_test.go b/xdr2/encode_test.go index 630ce89..fbf8dcd 100644 --- a/xdr2/encode_test.go +++ b/xdr2/encode_test.go @@ -305,6 +305,18 @@ func TestMarshal(t *testing.T) { []byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01}, 8, &MarshalError{ErrorCode: ErrIO}}, + // Discriminated unions + {unionStruct{0, 0, 1, 2}, + []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}, + 12, nil}, + {unionStruct{1, 0, 1, 2}, + []byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02}, + 12, nil}, + {unionStruct{2, 0, 1, 2}, + []byte{0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x02}, + 8, nil}, + {invalidUnionStruct{"err"}, []byte{}, 0, &MarshalError{ErrorCode: ErrBadDiscriminant}}, + // Expected errors {nilInterface, []byte{}, 0, &MarshalError{ErrorCode: ErrNilInterface}}, {&nilInterface, []byte{}, 0, &MarshalError{ErrorCode: ErrNilInterface}}, diff --git a/xdr2/error.go b/xdr2/error.go index 42079ad..496a517 100644 --- a/xdr2/error.go +++ b/xdr2/error.go @@ -61,6 +61,10 @@ const ( // RFC3339 formatted time value. The actual underlying error will be // available via the Err field of the UnmarshalError struct. ErrParseTime + + // ErrBadDiscriminant indicates that a non-integer field of a struct + // was marked as a union discriminant through a struct tag. + ErrBadDiscriminant ) // Map of ErrorCode values back to their constant names for pretty printing. @@ -73,6 +77,7 @@ var errorCodeStrings = map[ErrorCode]string{ ErrNilInterface: "ErrNilInterface", ErrIO: "ErrIO", ErrParseTime: "ErrParseTime", + ErrBadDiscriminant: "ErrBadDiscriminant", } // String returns the ErrorCode as a human-readable name. diff --git a/xdr2/tag.go b/xdr2/tag.go new file mode 100644 index 0000000..170e776 --- /dev/null +++ b/xdr2/tag.go @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2012-2014 Dave Collins + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +package xdr + +import ( + "reflect" + "strings" +) + +// xdrtag represents a XDR struct tag, identified by the name "xdr:". +// The value of the tag is a string that is parsed as a comma-separated +// list of =-separated key-value options. If an option has no value, +// "true" is assumed to be the default value. +// +// For instance: +// +// `xdr:"foo,bar=2,baz=false" +// +// After parsing this tag, Get("foo") will return "true", Get("bar") +// will return "2", and Get("baz") will return "false". +type xdrtag string + +// parseTag extracts a xdrtag from the original reflect.StructTag as found in +// in the struct field. If the tag was not specified, an empty strtag is +// returned. +func parseTag(tag reflect.StructTag) xdrtag { + t := tag.Get("xdr") + // Handle backward compatibility with the previous "xdropaque" + // tag which is now deprecated. + if tag.Get("xdropaque") == "false" { + if t == "" { + t = "," + } + t += ",opaque=false" + } + return xdrtag(t) +} + +// Get returns the value for the specified option. If the option is not +// present in the tag, an empty string is returned. If the option is +// present but has no value, the string "true" is returned as default value. +func (t xdrtag) Get(opt string) string { + tag := string(t) + for tag != "" { + var next string + i := strings.Index(tag, ",") + if i >= 0 { + tag, next = tag[:i], tag[i+1:] + } + if tag == opt { + return "true" + } + if len(tag) > len(opt) && tag[:len(opt)] == opt && tag[len(opt)] == '=' { + val := tag[len(opt)+1:] + i = strings.Index(val, ",") + if i >= 0 { + val = val[i:] + } + return val + } + tag = next + } + return "" +} From 19be307230d5041d0202d2421f3aa97afd435148 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Sun, 13 Sep 2015 16:24:32 +0200 Subject: [PATCH 3/8] Add support for bool type in discriminant --- xdr2/decode.go | 13 ++++++++++--- xdr2/decode_test.go | 13 +++++++++++++ xdr2/encode.go | 13 ++++++++++--- xdr2/encode_test.go | 6 ++++++ 4 files changed, 39 insertions(+), 6 deletions(-) diff --git a/xdr2/decode.go b/xdr2/decode.go index 0cf7ccf..bc7ddeb 100644 --- a/xdr2/decode.go +++ b/xdr2/decode.go @@ -571,11 +571,18 @@ func (d *Decoder) decodeStruct(v reflect.Value) (int, error) { } if tag.Get("union") == "true" { - if !vf.Type().ConvertibleTo(reflect.TypeOf(0)) { - msg := fmt.Sprintf("type '%s' is not valid", v.Kind().String()) + 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) } - union = strconv.Itoa(int(vf.Int())) } } diff --git a/xdr2/decode_test.go b/xdr2/decode_test.go index 51cdcd0..d1eae47 100644 --- a/xdr2/decode_test.go +++ b/xdr2/decode_test.go @@ -69,6 +69,13 @@ type unionStruct struct { 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"` } @@ -369,6 +376,12 @@ func TestUnmarshal(t *testing.T) { {[]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}}, diff --git a/xdr2/encode.go b/xdr2/encode.go index 8d716cf..c5ff6a5 100644 --- a/xdr2/encode.go +++ b/xdr2/encode.go @@ -480,11 +480,18 @@ func (enc *Encoder) encodeStruct(v reflect.Value) (int, error) { // option "unioncase=N" marks a struct field that is only serialized // when the discriminant has the specified value. if tag.Get("union") == "true" { - if !vf.Type().ConvertibleTo(reflect.TypeOf(0)) { - msg := fmt.Sprintf("type '%s' is not valid", v.Kind().String()) + 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, marshalError("encodeStruct", ErrBadDiscriminant, msg, nil, nil) } - union = strconv.Itoa(int(vf.Int())) } if union != "" { diff --git a/xdr2/encode_test.go b/xdr2/encode_test.go index fbf8dcd..5f8aff7 100644 --- a/xdr2/encode_test.go +++ b/xdr2/encode_test.go @@ -315,6 +315,12 @@ func TestMarshal(t *testing.T) { {unionStruct{2, 0, 1, 2}, []byte{0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x02}, 8, nil}, + {unionBoolStruct{false, 0, 1, 2}, + []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}, + 12, nil}, + {unionBoolStruct{true, 0, 1, 2}, + []byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02}, + 12, nil}, {invalidUnionStruct{"err"}, []byte{}, 0, &MarshalError{ErrorCode: ErrBadDiscriminant}}, // Expected errors From ca920fe7ad80694a0928389baf3b206b6e9bba2c Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Sun, 13 Sep 2015 20:49:23 +0200 Subject: [PATCH 4/8] Add support for optional-data. In Go, they are expressed through indirection: a pointer to the actualy data, that can be nil to express that the field is missing. The field must be marked with the new tag option "optional", because otherwise it is automatically indirected as per default behaviour of marshallers. --- xdr2/decode.go | 27 ++++++++++++++++++++++++--- xdr2/decode_test.go | 17 +++++++++++++++++ xdr2/encode.go | 26 ++++++++++++++++++++++++-- xdr2/encode_test.go | 7 +++++++ xdr2/error.go | 5 +++++ 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/xdr2/decode.go b/xdr2/decode.go index bc7ddeb..364326b 100644 --- a/xdr2/decode.go +++ b/xdr2/decode.go @@ -517,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 n2, 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()) @@ -532,8 +555,6 @@ func (d *Decoder) decodeStruct(v reflect.Value) (int, error) { return n, err } - tag := parseTag(vtf.Tag) - // Handle non-opaque data to []uint8 and [#]uint8 based on // struct tag. if tag.Get("opaque") == "false" { diff --git a/xdr2/decode_test.go b/xdr2/decode_test.go index d1eae47..935830e 100644 --- a/xdr2/decode_test.go +++ b/xdr2/decode_test.go @@ -80,6 +80,15 @@ 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 // read and error for an unmarshal. func testExpectedURet(t *testing.T, name string, n, wantN int, err, wantErr error) bool { @@ -386,6 +395,14 @@ func TestUnmarshal(t *testing.T) { 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, 0x00}, + invalidOptionalDataStruct{}, + 0, &UnmarshalError{ErrorCode: ErrBadOptional}}, + // Expected errors {nil, nilInterface, 0, &UnmarshalError{ErrorCode: ErrNilInterface}}, {nil, &nilInterface, 0, &UnmarshalError{ErrorCode: ErrNilInterface}}, diff --git a/xdr2/encode.go b/xdr2/encode.go index c5ff6a5..6813036 100644 --- a/xdr2/encode.go +++ b/xdr2/encode.go @@ -449,11 +449,33 @@ func (enc *Encoder) encodeStruct(v reflect.Value) (int, error) { if vtf.PkgPath != "" { continue } - vf := v.Field(i) - vf = enc.indirect(vf) + 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 := marshalError("encodeStruct", ErrBadOptional, + msg, nil, nil) + return n, err + } + + hasopt := !vf.IsNil() + n2, err := enc.EncodeBool(hasopt) + n += n2 + if err != nil { + return n2, err + } + if !hasopt { + continue + } + } + + vf = enc.indirect(vf) + // Handle non-opaque data to []uint8 and [#]uint8 based on struct tag. if tag.Get("opaque") == "false" { switch vf.Kind() { diff --git a/xdr2/encode_test.go b/xdr2/encode_test.go index 5f8aff7..25e6096 100644 --- a/xdr2/encode_test.go +++ b/xdr2/encode_test.go @@ -323,6 +323,13 @@ func TestMarshal(t *testing.T) { 12, nil}, {invalidUnionStruct{"err"}, []byte{}, 0, &MarshalError{ErrorCode: ErrBadDiscriminant}}, + // Optional data + {optionalDataStruct{1, &optionalDataStruct{2, nil}}, + []byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00}, + 16, nil}, + {invalidOptionalDataStruct{1}, + []byte{}, 0, &MarshalError{ErrorCode: ErrBadOptional}}, + // Expected errors {nilInterface, []byte{}, 0, &MarshalError{ErrorCode: ErrNilInterface}}, {&nilInterface, []byte{}, 0, &MarshalError{ErrorCode: ErrNilInterface}}, diff --git a/xdr2/error.go b/xdr2/error.go index 496a517..8c1f3ae 100644 --- a/xdr2/error.go +++ b/xdr2/error.go @@ -65,6 +65,10 @@ const ( // ErrBadDiscriminant indicates that a non-integer field of a struct // was marked as a union discriminant through a struct tag. ErrBadDiscriminant + + // ErrBadOptional indicates that a non-pointer field of a struct + // was marked as an optional-data. + ErrBadOptional ) // Map of ErrorCode values back to their constant names for pretty printing. @@ -78,6 +82,7 @@ var errorCodeStrings = map[ErrorCode]string{ ErrIO: "ErrIO", ErrParseTime: "ErrParseTime", ErrBadDiscriminant: "ErrBadDiscriminant", + ErrBadOptional: "ErrBadOptional", } // String returns the ErrorCode as a human-readable name. From 382fb42feef197b0f7d1e77cab66daf862749040 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Sun, 13 Sep 2015 22:42:45 +0200 Subject: [PATCH 5/8] Document optional-data --- xdr2/doc.go | 1 + 1 file changed, 1 insertion(+) diff --git a/xdr2/doc.go b/xdr2/doc.go index 7cae07f..1904d2b 100644 --- a/xdr2/doc.go +++ b/xdr2/doc.go @@ -66,6 +66,7 @@ and Unmarshal functions has specific details of how the mapping proceeds. [#]byte <-> XDR Fixed-Length Opaque Data [] <-> XDR Variable-Length Array [#] <-> XDR Fixed-Length Array + * <-> 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 From 1a41d1a06c93bf8a0e0385be3847a277bb793187 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Mon, 14 Sep 2015 13:51:23 +0200 Subject: [PATCH 6/8] Complete test coverage --- xdr2/decode.go | 2 +- xdr2/decode_test.go | 3 +++ xdr2/encode.go | 2 +- xdr2/encode_test.go | 6 +++++- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/xdr2/decode.go b/xdr2/decode.go index 364326b..6f37f76 100644 --- a/xdr2/decode.go +++ b/xdr2/decode.go @@ -533,7 +533,7 @@ func (d *Decoder) decodeStruct(v reflect.Value) (int, error) { hasopt, n2, err := d.DecodeBool() n += n2 if err != nil { - return n2, err + return n, err } if !hasopt { continue diff --git a/xdr2/decode_test.go b/xdr2/decode_test.go index 935830e..6c3e768 100644 --- a/xdr2/decode_test.go +++ b/xdr2/decode_test.go @@ -399,6 +399,9 @@ func TestUnmarshal(t *testing.T) { {[]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}}, diff --git a/xdr2/encode.go b/xdr2/encode.go index 6813036..73a2ff0 100644 --- a/xdr2/encode.go +++ b/xdr2/encode.go @@ -467,7 +467,7 @@ func (enc *Encoder) encodeStruct(v reflect.Value) (int, error) { n2, err := enc.EncodeBool(hasopt) n += n2 if err != nil { - return n2, err + return n, err } if !hasopt { continue diff --git a/xdr2/encode_test.go b/xdr2/encode_test.go index 25e6096..7e59f30 100644 --- a/xdr2/encode_test.go +++ b/xdr2/encode_test.go @@ -327,8 +327,12 @@ func TestMarshal(t *testing.T) { {optionalDataStruct{1, &optionalDataStruct{2, nil}}, []byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00}, 16, nil}, + {optionalDataStruct{1, nil}, + []uint8{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00}, + 7, &MarshalError{ErrorCode: ErrIO}}, {invalidOptionalDataStruct{1}, - []byte{}, 0, &MarshalError{ErrorCode: ErrBadOptional}}, + []byte{}, + 0, &MarshalError{ErrorCode: ErrBadOptional}}, // Expected errors {nilInterface, []byte{}, 0, &MarshalError{ErrorCode: ErrNilInterface}}, From d3abeacb8247b8cc4fa8764b17ce556aa4a47dfe Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Tue, 24 Jan 2017 17:45:45 +0100 Subject: [PATCH 7/8] Add test for tag (and remove spurious code) --- xdr2/decode_test.go | 2 +- xdr2/tag.go | 7 +------ xdr2/tag_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 xdr2/tag_test.go diff --git a/xdr2/decode_test.go b/xdr2/decode_test.go index 6c3e768..fef1e7d 100644 --- a/xdr2/decode_test.go +++ b/xdr2/decode_test.go @@ -59,7 +59,7 @@ type allTypesTest struct { // opaqueStruct is used to test handling of uint8 slices and arrays. type opaqueStruct struct { Slice []uint8 `xdr:"opaque=false"` - Array [1]uint8 `xdr:"opaque=false"` + Array [1]uint8 `xdropaque:"false"` // old syntax, backward compatibility } type unionStruct struct { diff --git a/xdr2/tag.go b/xdr2/tag.go index 170e776..03a4d4e 100644 --- a/xdr2/tag.go +++ b/xdr2/tag.go @@ -65,12 +65,7 @@ func (t xdrtag) Get(opt string) string { return "true" } if len(tag) > len(opt) && tag[:len(opt)] == opt && tag[len(opt)] == '=' { - val := tag[len(opt)+1:] - i = strings.Index(val, ",") - if i >= 0 { - val = val[i:] - } - return val + return tag[len(opt)+1:] } tag = next } diff --git a/xdr2/tag_test.go b/xdr2/tag_test.go new file mode 100644 index 0000000..6ae013f --- /dev/null +++ b/xdr2/tag_test.go @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2012-2014 Dave Collins + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +package xdr + +import ( + "reflect" + "testing" +) + +type structWithTag struct { + A int `xdr:"a=123,b,c=true,d=false"` +} + +func TestTag(t *testing.T) { + s := structWithTag{} + rt := reflect.TypeOf(s).Field(0) + tag := parseTag(rt.Tag) + + if tag.Get("a") != "123" { + t.Errorf("wrong value for a: %v", tag.Get("a")) + } + if tag.Get("b") != "true" { + t.Errorf("wrong value for b: %v", tag.Get("b")) + } + if tag.Get("c") != "true" { + t.Errorf("wrong value for b: %v", tag.Get("c")) + } + if tag.Get("d") != "false" { + t.Errorf("wrong value for b: %v", tag.Get("d")) + } +} From 4930550ba2e22f87187498acfd78348b15f4e7a8 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Fri, 17 Feb 2017 18:21:19 +0100 Subject: [PATCH 8/8] Updated copyright --- xdr2/tag.go | 2 +- xdr2/tag_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xdr2/tag.go b/xdr2/tag.go index 03a4d4e..7b5fe24 100644 --- a/xdr2/tag.go +++ b/xdr2/tag.go @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2014 Dave Collins + * Copyright (c) 2015-2017 Giovanni Bajo * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above diff --git a/xdr2/tag_test.go b/xdr2/tag_test.go index 6ae013f..26a438b 100644 --- a/xdr2/tag_test.go +++ b/xdr2/tag_test.go @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2014 Dave Collins + * Copyright (c) 2015-2017 Giovanni Bajo * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above