[
https://issues.apache.org/jira/browse/THRIFT-796?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13018436#comment-13018436
]
Nicholas Telford commented on THRIFT-796:
-----------------------------------------
Looking at the extension source, this issue still seems to exist in trunk.
>From what I can tell, the offending code appears to be a pile of
convert_to_*() calls in binary_serialize(). The same calls elsewhere won't
affect input data so they're safe. I don't have the necessary knowledge of the
PHP extension API to fix this myself, just confirming it still exists in trunk.
> TBinaryProtocolAccelerated changes passed argument types
> --------------------------------------------------------
>
> Key: THRIFT-796
> URL: https://issues.apache.org/jira/browse/THRIFT-796
> Project: Thrift
> Issue Type: Bug
> Components: PHP - Library
> Affects Versions: 0.2
> Environment: Redhat on x86_64 Intel Linux
> Reporter: Juho Mäkinen
> Original Estimate: 48h
> Remaining Estimate: 48h
>
> Thrift PHP TBinaryProtocolAccelerated has a bug which changes passed argument
> types outside of the function, polluting variables in other places of the
> program which uses thrift.
> Consider we have a Thrift service with function specified as:
> void foo(1:string s);
> and we use it in the following way:
> <?php
> $str = 100;
> var_dump($str);
> $client->foo($str);
> var_dump($str);
> ?>
> results
> int(100)
> string(3) "100"
> So thrift_protocol_write_binary takes $str as an argument and internally
> converts the passed argument to the type specified in thrift interface
> (string in this case). This results that $str type is casted from int to
> string. It's a big problem because TBinaryProtocol doesn't do this, but
> changing it to use TBinaryProtocolAccelerated instead breaks working programs
> (it took me a day to figure that thrift was causing a very weird bug in our
> program).
> My teammate digged into the thrift php extension (check his email:
> http://mail-archives.apache.org/mod_mbox/incubator-thrift-user/201006.mbox/browser)
> and said this:
> """I looked through the extension code and there's convert_to_* calls on the
> input
> variables, which causes this suprising behaviour. I also tried to replace
> convert_to_* calls in binary_serialize with _ex counterparts, but something
> went completely wrong."""
> I believe that correct way would be that the extension would check if a type
> conversion needs to be done and makes a copy of the zval before conversion.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
