[jira] [Commented] (THRIFT-796) TBinaryProtocolAccelerated changes passed argument types

["Nicholas Telford (JIRA)" <ji...@apache.org>]

    [ 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