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

Use correct namespace for checking if the isDefaultRootsPemSet method exists. #31580

Conversation

frankdejonge
Copy link
Contributor

@frankdejonge frankdejonge commented Nov 8, 2022

The method_exists function requires a fully qualified class name to be sent to check if a method exists. The current class was missing the namespace, which means the function always returns false. In our application this caused the credentials to be loaded many times over, which ate up some CPU. This bug fix ensures that this is only run once per request.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 8, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: frankdejonge / name: Frank de Jonge (306dba2)

@barrycommins
Copy link

barrycommins commented Dec 14, 2022

Hi, is there any possibility of getting this reviewed?

method_exists('ChannelCredentials', 'isDefaultRootsPemSet') always returns false because ChannelCredentials is in the Grpc namespace and not the default namespace.

Thanks!

@frankdejonge
Copy link
Contributor Author

@stanley-cheung sorry for the ping, I'm trying to fix a bug in the client library but I have no clue how to push this forward. Do you have any advice?

@alto-php
Copy link

alto-php commented Mar 4, 2023

Thanks for the fix, any chance you can add a test for it as well?

@frankdejonge
Copy link
Contributor Author

frankdejonge commented Mar 4, 2023 via email

@alto-php alto-php merged commit 7b3977e into grpc:master Mar 7, 2023
1 check failed
@frankdejonge frankdejonge deleted the php/bugfix/correct-namespace-for-method_exists-call branch March 7, 2023 04:45
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Mar 8, 2023
@stanley-cheung stanley-cheung added the release notes: yes Indicates if PR needs to be in release notes label Apr 7, 2023
XuanWang-Amos pushed a commit to XuanWang-Amos/grpc that referenced this pull request May 1, 2023
… exists. (grpc#31580)

The `method_exists` function requires a fully qualified class name to be
sent to check if a method exists. The current class was missing the
namespace, which means the function always returns `false`. In our
application this caused the credentials to be loaded many times over,
which ate up some CPU. This bug fix ensures that this is only run once
per request.
wanlin31 pushed a commit that referenced this pull request May 18, 2023
… exists. (#31580)

The `method_exists` function requires a fully qualified class name to be
sent to check if a method exists. The current class was missing the
namespace, which means the function always returns `false`. In our
application this caused the credentials to be loaded many times over,
which ate up some CPU. This bug fix ensures that this is only run once
per request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported Specifies if the PR has been imported to the internal repository lang/php release notes: yes Indicates if PR needs to be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants