golang: binary fields in union types lacking omitted from "CountSetFields" method about frugal HOT 9 CLOSED

Note: I believe the workaround of just using strings works here, but obviously isn't preferred.

from frugal.

This diff seems to resolve the issue, but I'm not informed enough on thrift/frugal to say whether it would cause any problems elsewhere...

diff --git a/compiler/generator/golang/generator.go b/compiler/generator/golang/generator.go
index 216dafa..d5b82bd 100644
--- a/compiler/generator/golang/generator.go
+++ b/compiler/generator/golang/generator.go
@@ -2152,10 +2152,7 @@ func (g *Generator) isPointerField(field *parser.Field) bool {
        hasDefault := field.Default != nil
        switch underlyingType.Name {
-       case "binary":
-               // According to thrift, these are always like this, not sure why
-               return false
-       case "bool", "byte", "i8", "i16", "i32", "i64", "double", "string":
+       case "bool", "byte", "i8", "i16", "i32", "i64", "double", "string", "binary":
                // If there's no default, needs to be a pointer to be nillable
                return !hasDefault
        case "list", "set", "map":

from frugal.

@justinlindh-wf Thanks for the detailed issue. We will work on adding the patch.

from frugal.

Also, when @benechols-wf and I were testing this, we found compiled our client to use string fields and had a server using binary fields and we were able to successfully communicate between each other. Thus the Audit tool should be updated as well to allow binary <-> string type changes w/o raising an error.

from frugal.

@natewoods-wf there could be potential for issues there as string is encoded as UTF-8 and binary is an unencoded sequence of bytes.

from frugal.

+1

from frugal.

I also ran into this issue sending a pdf. String type does not work. When trying to open the pdf the conversion to string changes the number of bytes in the pdf. Then when parsing it, the library fails because the PDF streams are the incorrect length.

from frugal.

This appears to be a bug in thrift as well

from frugal.

Addressed in #586.

from frugal.