[
https://issues.apache.org/jira/browse/THRIFT-840?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12897969#action_12897969
]
Bryan Duxbury commented on THRIFT-840:
--------------------------------------
I'm no Perl guy, but it seems like there's no reason to have two different
kinds of "die" cases. The one with the if seems to be subsumed by the one
without - am I wrong?
> 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.4
>
> 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.
