Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #42 SoapClient->_stream_context is private in php 8 #46

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

fougere2713
Copy link

@fougere2713 fougere2713 commented Dec 9, 2022

SoapClient->_stream_context is private in php8.
To work around that a stream context is systematically created during init and provided to SoapClient.

Another aproach I tried before was to get access to SoapClient->_stream_context through reflection or a closure.
That trick does not work for php internal classes, and felt a bit hacky.

I had to remove AbstractSoapClientBase->setSoapClient, since we lose access to the stream context if SoapClient is created externally.
That's a potential breaking change.

@fougere2713 fougere2713 changed the title Fixes #42. SoapClient->_stream_context is private in php 8 Fixes #42 SoapClient->_stream_context is private in php 8 Dec 9, 2022
@mikaelcom
Copy link
Member

Hi, thanks for the PR.
As the composer.json currently stipulates it must work from PHP>=7.4, even if the tests passes, I must test this enhancement within a real generated package and within the PackageGenerator project. Did you have the chance to do so already?

@fougere2713
Copy link
Author

Yes, that change is live on a project running php 7.4 with a generated client and custom http headers.
No problems so far.

@mikaelcom
Copy link
Member

As you mentionned SoapClient->_stream_context is private in php8., wouldn't it more appropriate to create a new major version requiring PHP >= 8 embedding this enhancement?

@fougere2713
Copy link
Author

The removal of AbstractSoapClientBase->setSoapClient may break stuff downstream, so a new major version makes sense.

Copy link

@devloic devloic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worked for me on both php7.4 and 8.2.4, macos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants