search

fennb / phirehose

PHP interface to Twitter Streaming API
706 stars 189 forks source link

Fix interaction with the Sockets API under PHP 8.x #126

Closed spideyfusion closed 3 years ago

spideyfusion commented 3 years ago

This fix addresses the following breaking change introduced in PHP 8.0:

socket_create(), socket_create_listen(), socket_accept(), socket_import_stream(), socket_addrinfo_connect(), socket_addrinfo_bind(), and socket_wsaprotocol_info_import() will now return a Socket object rather than a resource. socket_addrinfo_lookup() will now return an array of AddressInfo objects rather than resources.

I originally thought about using the socket_import_stream function to convert the stream resource into a socket resource, but unfortunately, the function doesn't work in combination with SSL:

PHP Warning: socket_import_stream(): Cannot represent a stream of type tcp_socket/ssl as a Socket Descriptor

With this simple change, the last socket-related error that occurred will be retrieved from PHP's own global internal store, as seen here. This function behavior dates all the way back to PHP 5.x, so the change is pretty safe to apply unconditionally.

The rest of the codebase works fine under PHP 8.0.

fennb commented 3 years ago

I originally thought about using the socket_import_stream function to convert the stream resource into a socket resource, but unfortunately, the function doesn't work in combination with SSL:

PHP Warning: socket_import_stream(): Cannot represent a stream of type tcp_socket/ssl as a Socket Descriptor

Huh, that's annoying that socket_import_stream() won't handle SSL. Merging this as the best/easy fix we have. Thanks for the work/submission.

spideyfusion commented 3 years ago

@fennb Thanks a lot for a prompt response. Could you please create a new tagged release that contains this fix?

fennb commented 3 years ago

@spideyfusion done!

As a favour, I don't supposed you could test that this PR is safe/works? If so, I will merge it in too: https://github.com/fennb/phirehose/pull/125

spideyfusion commented 3 years ago

@fennb I tested the changes and left a review on the PR.