Skip to content

Add LDAP_OPT_X_TLS_PROTOCOL_MAX and LDAP_OPT_X_TLS_PROTOCOL_TLS1_3 #13405

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Add LDAP_OPT_X_TLS_PROTOCOL_MAX and LDAP_OPT_X_TLS_PROTOCOL_TLS1_3 #13405

wants to merge 2 commits into from

Conversation

StephenWall
Copy link
Contributor

@devnexen
Copy link
Member

devnexen commented Feb 15, 2024 •
edited
Loading

nit: you need to use build/gen_stub.php to apply on ext/ldap/ldap.stub.php instead of manually updating ext/ldap/ldap_arginfo.h.

@devnexen
Copy link
Member

devnexen commented Feb 15, 2024 •
edited
Loading

Looking good :) would be great to squash all your commits into one.

Note for next time, it s better to create a new branch from master, but that s fine for now. Thanks !

@StephenWall
Copy link
Contributor Author

StephenWall commented Feb 15, 2024 •
edited
Loading

would be great to squash all your commits into one.

Don't know how to do that without a force push, which github is not allowing. I created and pushed a tls13 branch with the changes squashed, but also don't know how to reset my master to php-src master. :/

@devnexen
Copy link
Member

I see, that is fine I ll manage ;-)

@StephenWall
Copy link
Contributor Author

I'm fine creating a new pull request if you'd like.

@devnexen devnexen closed this in f6016c7 Feb 15, 2024
@devnexen
Copy link
Member

Thanks for your work !

@iluuu1994
Copy link
Member

This broke some tests in nightly, because the constant is not always available. Can you please have a look?
https://github.com/php/php-src/actions/runs/7924648702/job/21636612515

@devnexen
Copy link
Member

I just pushed a fix earlier.

@iluuu1994
Copy link
Member

Oh, great! Thanks!

@jrfnl jrfnl mentioned this pull request Aug 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devnexen devnexen devnexen approved these changes

@kocsismate kocsismate Awaiting requested review from kocsismate kocsismate is a code owner

Assignees
No one assigned
Labels
Extension: ldap
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@StephenWall @devnexen @iluuu1994