Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-2420

Go argument parser for methods without arguments does not skip fields

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.2
    • Fix Version/s: 0.9.2
    • Component/s: Go - Compiler
    • Labels:
      None
    • Environment:

      All

    • Patch Info:
      Patch Available

      Description

      The Read() method of a struct of a thrift function which does not have arguments (e.g. foo()) calls iprot.ReadFieldBegin() to read the field type but does not consume/skip a field which it was not expecting.

      Read() methods for argument parsers for methods which have arguments have the correct code in the default section of the switch statement.

      This is the current code (I've added where the iprot.Skip() call is missing):

      func (p *GetVersionDataArgs) Read(iprot thrift.TProtocol) error {
      	if _, err := iprot.ReadStructBegin(); err != nil {
      		return fmt.Errorf("%T read error: %s", p, err)
      	}
      	for {
      		_, fieldTypeId, fieldId, err := iprot.ReadFieldBegin()
      		if err != nil {
      			return fmt.Errorf("%T field %d read error: %s", p, fieldId, err)
      		}
      		if fieldTypeId == thrift.STOP {
      			break
      		}
      		// should call iprot.Skip(fieldTypeId) if not STOP
      		if err := iprot.ReadFieldEnd(); err != nil {
      			return err
      		}
      	}
      	if err := iprot.ReadStructEnd(); err != nil {
      		return fmt.Errorf("%T read struct end error: %s", p, err)
      	}
      	return nil
      }
      

      After the check for thrift.STOP there needs to be this code

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

      We've found this because on the Java side we dynamically inject fields into the struct on call which are ignored on the Go side as long as the function has arguments. When we make a call to the no-argument function some bytes are not consumed and make the thrift code go out of sync.

      I will attach a patch which fixes this.

        Attachments

        1. THRIFT-2420-with-indentation.patch
          4 kB
          Frank Schroeder
        2. THRIFT-2420_skip_field_missing-v2.patch
          3 kB
          Jens Geyer

          Activity

            People

            • Assignee:
              jensg Jens Geyer
              Reporter:
              magiconair Frank Schroeder
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified