[ 
https://issues.apache.org/jira/browse/THRIFT-4650?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Craig W updated THRIFT-4650:
----------------------------
    Description: 
The "Read" function that gets generated incorrectly marks required fields as 
being set when the type of the field does not match what's expected.

{{For example, the following IDL:}}
 {{struct Foo {}}
 {{  1: required string id}}
 {{}}}

{{The generated Read function has the following:}}

{{switch fieldId {}}
 {{  case 1:}}
 {{    if fieldTypeId == thrift.STRING {}}
 {{      if err := p.ReadField1(iprot); err != nil {}}
 {{        return err}}

             }
 {{    } else {}}
 {{      if err := iprot.Skip(fieldTypeId); err != nil {}}
 {{        return err}}

            }

        }
 {{    issetID = true}}
 {{}}}

{{I ran into a case where I attempted to deserialize another message type into 
type Foo, and I expected it to return an error, but it did not.}}

{{f : = Foo}}
 {{err := thrift.NewTDeserializer().Read(&f, bytes)}}

{{I think [this 
code|https://github.com/apache/thrift/blob/master/compiler/cpp/src/thrift/generate/t_go_generator.cc#L1600]
 in the generator needs to be moved inside the first "if" block, so that its 
only set to true if it actually reads the data of the correct type.}}
 {{ }}

  was:
The "Read" function that gets generated incorrectly marks required fields as 
being set when the type of the field does not match what's expected.

{{For example, the following IDL:}}
{{struct Foo {}}
{{  1: required string id}}
{{}}}

{{The generated Read function has the following:}}

{{switch fieldId {}}
{{  case 1:}}
{{    if fieldTypeId == thrift.STRING {}}
{{      if err := p.ReadField1(iprot); err != nil {}}
{{        return err}}
{{      }}}
{{    } else {}}
{{      if err := iprot.Skip(fieldTypeId); err != nil {}}
{{        return err}}
{{      }}}
{{    }}}
{{    issetID = true}}
{{}}}

{{I ran into a case where I attempted to deserialize another message type into 
type Foo, and I expected it to return an error, but it did not.}}

{{f : = Foo{}}}
{{err := thrift.NewTDeserializer().Read(&f, bytes)}}

{{I think [this 
code|https://github.com/apache/thrift/blob/master/compiler/cpp/src/thrift/generate/t_go_generator.cc#L1600]
 in the generator needs to be moved inside the first "if" block, so that its 
only set to true if it actually reads the data of the correct type.}}
{{ }}


> Required field incorrectly marked as set when fieldType does not match
> ----------------------------------------------------------------------
>
>                 Key: THRIFT-4650
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4650
>             Project: Thrift
>          Issue Type: Bug
>          Components: Go - Compiler
>    Affects Versions: 0.11.0
>            Reporter: Craig W
>            Priority: Major
>
> The "Read" function that gets generated incorrectly marks required fields as 
> being set when the type of the field does not match what's expected.
> {{For example, the following IDL:}}
>  {{struct Foo {}}
>  {{  1: required string id}}
>  {{}}}
> {{The generated Read function has the following:}}
> {{switch fieldId {}}
>  {{  case 1:}}
>  {{    if fieldTypeId == thrift.STRING {}}
>  {{      if err := p.ReadField1(iprot); err != nil {}}
>  {{        return err}}
>              }
>  {{    } else {}}
>  {{      if err := iprot.Skip(fieldTypeId); err != nil {}}
>  {{        return err}}
>             }
>         }
>  {{    issetID = true}}
>  {{}}}
> {{I ran into a case where I attempted to deserialize another message type 
> into type Foo, and I expected it to return an error, but it did not.}}
> {{f : = Foo}}
>  {{err := thrift.NewTDeserializer().Read(&f, bytes)}}
> {{I think [this 
> code|https://github.com/apache/thrift/blob/master/compiler/cpp/src/thrift/generate/t_go_generator.cc#L1600]
>  in the generator needs to be moved inside the first "if" block, so that its 
> only set to true if it actually reads the data of the correct type.}}
>  {{ }}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to