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

Ruby structs are not recursively validated

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 0.9.2
    • Fix Version/s: None
    • Component/s: Ruby - Compiler
    • Labels:
      None
    • Environment:

      Ruby client & Ruby Server

      Description

      I've come across something that I had fixed in a third party library, but after working with generated & ruby thrift library code I decided to file an issue here. The root cause is that Thrift::Struct & Thrift::Struct_Union sub-classes do not call validate on struct members of the same type. This is problematic because the library does correctly call validate to prevent the server or client from sending mal-formated message. Unfortunately this only works for simple protocols that don't use structs. This creates a bad user experience. Take this example protocol.

      struct EchoMessage {
        1: required string message
      }
      
      struct NestedEchoMessage {
        1: required EchoMessage echo
      }
      
      service EchoService {
        NestedEcho echo(1: string message)
      }
      

      And a simple & incorrect implementation in ruby:

      class Handler
        def echo(message)
           # note the nested Echo struct is missing
           NestedEcho.new
        end
      end
      

      This code has a few problems, but the key issue is that thrift library will attempt to send this message over the write & the client incorrectly receive a Thrift::Transport exception when it should receive a Thrift::ProtocolException.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              ahawkins Adam Hawkins
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated: