samuel / go-thrift

A native Thrift package for Go
BSD 3-Clause "New" or "Revised" License
385 stars 110 forks source link

Missing required field even though it exists #93

Open ghost opened 8 years ago

ghost commented 8 years ago

Hi, tried to use this library (after some problems with Apache's one), and ran into a problem with binary codec. I was looking at the source code and this place looks suspicious to me (sorry, I don't know the binary format spec, so it may be completely off the mark):

if req != 0 {
    for i := 0; req != 0; i, req = i+1, req>>1 {
        if req&1 != 0 {
            d.error(&MissingRequiredField{
                StructName: v.Type().Name(),
                FieldName:  meta.fields[i].name,
            })
        }
    }
}

Notice how i is never modified here. You also don't seem to need the outer if, besides, req = i+1 is the same as req = 1, but since you always discard the last bit, this operation has no effect on the following req & 1 != 0 test. I think there must be some kind of a typo here, but I cannot tell where exactly.

struct ResponseSysInfo {
  10: Response  response,
  20: CpuStats  cpu,
  30: i64       total_mem,
  40: i64       available_mem,
  50: i64       uptime,
  60: string    kernel,
}

This is Thrift description of the message I was trying to parse when I encountered this error: reading body thrift: missing required field: ResponseSysInfo.Cpu.