[jira] Updated: (THRIFT-840) Perl protocol handler could be more robust against unrecognised types

["Conrad Hughes (JIRA)" <ji...@apache.org>]

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

Conrad Hughes updated THRIFT-840:
---------------------------------

    Attachment: perl_unrecognised_types.patch

Throw exception instead of silent return on unrecognised type.

> Perl protocol handler could be more robust against unrecognised types
> ---------------------------------------------------------------------
>
>                 Key: THRIFT-840
>                 URL: https://issues.apache.org/jira/browse/THRIFT-840
>             Project: Thrift
>          Issue Type: Bug
>          Components: Library (Perl)
>    Affects Versions: 0.3
>         Environment: Debian Linux 5.0 (stable), Perl 5.10.0.
>            Reporter: Conrad Hughes
>            Priority: Minor
>             Fix For: 0.3
>
>         Attachments: perl_unrecognised_types.patch
>
>
> The Perl protocol implementation can loop arbitrarily on corrupt data because Protocol::skip() skips nothing if it doesn't recognise a type.  It might be nice if it threw an exception instead.  For example, the loop to skip a map (lib/perl/lib/Thrift/Protocol.pm:374) will keep looking at the same invalid bytes for every iteration if it hits an unrecognised type.  If corrupt data has resulted in a bogus huge map size, then the loop goes on for a long while, rather than dying quickly after running out of available data.
> I recognise that input validation probably isn't a priority for Thrift (usually communications are well-formed!), but wonder if you'd consider the attached patch which dies if skip or skipBinary encounter types that they don't know what to do with.  It probably needs more thought than I've given it, as I'm not sure what the correct behaviour should be for valid types not handled by the existing code: in the patch I'm throwing an exception for those saying that they cannot be skipped.  An additional TType constant of MAX_TYPE might be helpful in writing a better solution.
> I know it's a bit weird to be suggesting this; I'm using Thrift for serialisation among other things, and with an occasionally-imperfect data store.  This "infinite" loop was the only instance in which your existing error handling didn't adequately flag up bad data.  Clearly I should also be doing checksums on the data beforehand, but I just thought I'd suggest this to you.
> Thanks,
> Conrad

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.