[
https://issues.apache.org/jira/browse/THRIFT-840?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12898024#action_12898024
]
Conrad Hughes commented on THRIFT-840:
--------------------------------------
No, you're exactly right. I separated them out because I thought it
might be informative in case it turns out that all those valid types for
which there's no skip code should just return 0. I don't know what
should be done for them really, but looking at the Java skip code they
aren't handled there either. It appears to me that you can only reach
the end of that method in error, so any exception will do as far as I'm
concerned.
Conrad
--
The University of Edinburgh is a charitable body, registered in
Scotland, with registration number SC005336.
> 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.5
>
> 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.
