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

Fix typo in LDAP stub #14313

Merged
merged 1 commit into from
May 25, 2024
Merged

Fix typo in LDAP stub #14313

merged 1 commit into from
May 25, 2024

Conversation

haszi
Copy link
Contributor

@haszi haszi commented May 23, 2024

I think using ORALDAP instead of HAVE_ORALDAP which is used everywhere else was a typo. If it wasn't, feel free to close this PR.

@haszi haszi requested a review from kocsismate as a code owner May 23, 2024 20:37
@haszi haszi closed this May 23, 2024
@haszi haszi deleted the Fix-typo-in-ldap-stub branch May 23, 2024 20:48
@petk
Copy link
Member

petk commented May 23, 2024

Yup, I think so too...

@haszi haszi restored the Fix-typo-in-ldap-stub branch May 24, 2024 14:37
@haszi haszi reopened this May 24, 2024
@haszi
Copy link
Contributor Author

haszi commented May 24, 2024

I probably should have opened an issue first but re-opening this to see whether this was indeed a typo or not.

@petk
Copy link
Member

petk commented May 24, 2024

Thanks for reporting this and opening a PR. This probably could even go into PHP-8.2 branch but it would introduce 3 new PHP constants in case of Oracle client's ldap implementation (this is different than the OpenLDAP):

  • GSLC_SSL_NO_AUTH
  • GSLC_SSL_ONEWAY_AUTH
  • GSLC_SSL_TWOWAY_AUTH

Because this ORALDAP symbol has been there since the beginning of the implementation in php-src. Therefore the master branch is fine to fix this and have these new symbols there.

Interesting that no one has noticed this, I guess these constants were never used.

@petk petk merged commit 9864d91 into php:master May 25, 2024
15 of 17 checks passed
@haszi haszi deleted the Fix-typo-in-ldap-stub branch May 25, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants