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.

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

Request init hook edge cases tests #583

Merged
merged 5 commits into from Sep 20, 2019
Merged

Request init hook edge cases tests #583

merged 5 commits into from Sep 20, 2019

Conversation

SammyK
Copy link
Contributor

@SammyK SammyK commented Sep 19, 2019 •
edited

Description

  • This PR pulls Ensure the request init hook is bound by the open_basedir INI directive #579 into master.
  • It also adds more tests to the request init hook and specifically tests that the request init hook is properly sandboxed such that errors raised and exceptions thrown in the hook do not crash the app.
  • It also includes a tweak to dd-doctor.php to better accommodate environments jailed with open_basedir and more debug info.
  • It also includes a small fix for a compiler warning that was making the master build fail.

Readiness checklist

  • (only for Members) Changelog has been added to the appropriate release draft. Create one if necessary.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the appropriate release draft. For community contributors the reviewer is in charge of this task.

@SammyK
Ensure the request init hook is bound by the open_basedir INI directive
e7c8193 
Also sandbox the request init hook so that errors and exceptions are ignored
@SammyK SammyK added dev/testing c-extension Apply this label to issues and prs related to the C-extension labels Sep 19, 2019
@SammyK SammyK force-pushed the sammyk/build/request-init-hook branch from 70d420a to 4b8e5bf Compare September 19, 2019 21:44
@SammyK SammyK added this to the 0.31.0 milestone Sep 19, 2019
@SammyK SammyK marked this pull request as ready for review September 19, 2019 21:51
morrisonlevi
morrisonlevi approved these changes Sep 20, 2019
View reviewed changes
Copy link
Collaborator

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for noting why we can't call zend_clear_exception in PHP 5.

@SammyK SammyK merged commit 69467c1 into master Sep 20, 2019
@SammyK SammyK deleted the sammyk/build/request-init-hook branch September 20, 2019 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morrisonlevi morrisonlevi morrisonlevi approved these changes

Assignees
No one assigned
Labels
c-extension Apply this label to issues and prs related to the C-extension dev/testing
Projects
None yet
Milestone
0.31.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@SammyK @morrisonlevi