You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2020/05/29 05:21:35 UTC
[GitHub] [thrift] sveneld opened a new pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
sveneld opened a new pull request #2160:
URL: https://github.com/apache/thrift/pull/2160
Warning in TSocket when using ssl connection
When you using ssl host in
new TSocket('ssl://test.com')
you will get a warning
PHP Warning: socket_import_stream(): cannot represent a stream of type tcp_socket/ssl as a Socket Descriptor in /usr/share/php/Thrift/Transport/TSocket.php on line 254
due to the bug https://bugs.php.net/bug.php?id=70939
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669290824
Would love to but PHP is not my area of expertise. Could you ask on the dev mailing list instead?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-679264346
@Jens-G i need to make something else in this pull request?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ulidtko commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
ulidtko commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669785048
@sveneld can you please specify exact steps to reproduce? I don't get any warning even on [several builds of] 7.3.14.
```
$ cd thrift_clone_dir/lib/php
$
$ cat > test.php <<EOF
<?php
require "Thrift/Type/TType.php";
require "Thrift/Transport/TTransport.php";
require "Thrift/Transport/TSocket.php";
require "Thrift/Exception/TException.php";
$buzz = new Thrift\Transport\TSocket('ssl://ifconfig.me', 443);
$buzz->open();
var_dump($buzz);
?>
EOF
$
$ docker run --rm -it -v $PWD/test.php:/test.php -v $PWD/lib:/usr/local/lib/php/Thrift:ro -w $PWD php:7.3.14 php -f /test.php
object(Thrift\Transport\TSocket)#1 (10) {
["handle_":protected]=>
resource(8) of type (stream)
["host_":protected]=>
string(17) "ssl://ifconfig.me"
["port_":protected]=>
int(443)
["sendTimeoutSec_":protected]=>
int(0)
["sendTimeoutUsec_":protected]=>
int(100000)
["recvTimeoutSec_":protected]=>
int(0)
["recvTimeoutUsec_":protected]=>
int(750000)
["persist_":protected]=>
bool(false)
["debug_":protected]=>
bool(false)
["debugHandler_":protected]=>
string(9) "error_log"
}
```
See, no warnings.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ulidtko commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
ulidtko commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669892204
> Thanks. What else is need for merging this pull request?
@Jens-G action.
Ideally, you need to get the CI to become green, too. Unfortunately Thrift's test suites aren't as stable, so you get unrelated test failures on Travis like this:
```
===============================================================================
*** Following 6 failures were unexpected ***:
If it is introduced by you, please fix it before submitting the code.
===============================================================================
server-client: protocol: transport: result:
java-erl multi-binary fastframed-framed-ip-ssl failure(1)
java-erl multi-binary buffered-ip-ssl failure(1)
java-erl multi-binary framed-ip-ssl failure(1)
java-erl binary fastframed-framed-ip-ssl failure(1)
java-erl binary buffered-ip-ssl failure(1)
java-erl binary framed-ip-ssl failure(1)
===============================================================================
```
I don't think this PR should do anything to address that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on a change in pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on a change in pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#discussion_r466148589
##########
File path: lib/php/lib/Transport/TSocket.php
##########
@@ -251,8 +251,8 @@ public function open()
}
if (function_exists('socket_import_stream') && function_exists('socket_set_option')) {
- $socket = socket_import_stream($this->handle_);
- socket_set_option($socket, SOL_TCP, TCP_NODELAY, 1);
+ $socket = @socket_import_stream($this->handle_);
+ @socket_set_option($socket, SOL_TCP, TCP_NODELAY, 1);
}
Review comment:
I understand that @ does not fix trouble. I added a comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669696190
> Hi!
>
> Could you please elaborate your PHP build version? I cannot reproduce the warning on 5.6.33.
PHP 7.3.14
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld edited a comment on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld edited a comment on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669788238
i will try to prepare example.
here it is log what i get on production
`
PHP Warning: socket_import_stream(): cannot represent a stream of type tcp_socket/ssl as a Socket Descriptor in ...Thrift/Transport/TSocket.php on line 254
PHP Warning: socket_set_option() expects parameter 1 to be resource, bool given in ...Thrift/Transport/TSocket.php on line 255
`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] stale[bot] commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-665430359
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] ulidtko commented on a change in pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
ulidtko commented on a change in pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#discussion_r465970045
##########
File path: lib/php/lib/Transport/TSocket.php
##########
@@ -251,8 +251,8 @@ public function open()
}
if (function_exists('socket_import_stream') && function_exists('socket_set_option')) {
- $socket = socket_import_stream($this->handle_);
- socket_set_option($socket, SOL_TCP, TCP_NODELAY, 1);
+ $socket = @socket_import_stream($this->handle_);
+ @socket_set_option($socket, SOL_TCP, TCP_NODELAY, 1);
}
Review comment:
I don't think this patch fixes anything. The [`@` operator in PHP][•], apparently, just silences errors.
However, it remedies the SSL warnings burden for (some of) the consumers of Thrift PHP library.
Please at least add a comment saying ` // warnings silenced due to bug https://bugs.php.net/bug.php?id=70939` @sveneld
[•]: https://stackoverflow.com/q/1032161/531179
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-679273804
No, will be done shortly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669173054
@Jens-G can you pay attention to this pull request?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669788238
i will try to prepare example.
here it is log what i get on production
`
PHP Warning: socket_import_stream(): cannot represent a stream of type tcp_socket/ssl as a Socket Descriptor in ...Thrift/Transport/TSocket.php on line 254
PHP Warning: socket_set_option() expects parameter 1 to be resource, bool given in ...Thrift/Transport/TSocket.php on line 25
`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] Jens-G closed pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
Jens-G closed pull request #2160:
URL: https://github.com/apache/thrift/pull/2160
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on a change in pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on a change in pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#discussion_r466148589
##########
File path: lib/php/lib/Transport/TSocket.php
##########
@@ -251,8 +251,8 @@ public function open()
}
if (function_exists('socket_import_stream') && function_exists('socket_set_option')) {
- $socket = socket_import_stream($this->handle_);
- socket_set_option($socket, SOL_TCP, TCP_NODELAY, 1);
+ $socket = @socket_import_stream($this->handle_);
+ @socket_set_option($socket, SOL_TCP, TCP_NODELAY, 1);
}
Review comment:
I understand that @ it does not fix trouble. I will add a comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-665435589
issue is still actual
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-671806409
@Jens-G it's enough for merging pull request?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] sveneld commented on pull request #2160: THRIFT-5132 Warning in TSocket when using ssl connection
Posted by GitBox <gi...@apache.org>.
sveneld commented on pull request #2160:
URL: https://github.com/apache/thrift/pull/2160#issuecomment-669879023
Thanks. What else is need for merging this pull request?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org