Skip to content

Clear output handler status flags during handler initialization #13087

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

Closed

Conversation

haszi
Copy link
Contributor

@haszi haszi commented Jan 7, 2024

When initializing output handlers, clears user supplied handler status flags (see all flags listed below).

Output buffers accept a bitflag for setting the operations allowed on the buffer (see handler ability flags below). These flags, along with the flags for handler type (see handler types below) and handler status (see handler status flags below) are stored in one bitmask internally. When initializing the buffer, the bits that represent handler type are cleared and set internally (here and here), and the rest of the user supplied bitmask is left unchanged. This has the (IMO unintended) side effect that the handler processing status can be set by the code starting the buffer.

Setting the PHP_OUTPUT_HANDLER_STARTED status flag manually will prevent the passing of the PHP_OUTPUT_HANDLER_START flag to the output handler function which can be used by user defined functions to identify the first time the output buffer calls the handler function. Internally, this flag cannot be set as it will only be set after the handler function first gets called.

Setting the PHP_OUTPUT_HANDLER_DISABLED status flag manually will prevent ob_end_clean, ob_get_clean, ob_end_flush and ob_get_flush from calling the handler function. Internally, this flag will only be set when a handler function fails or when php_output_handler_hook(PHP_OUTPUT_HANDLER_HOOK_DISABLE, NULL) is called.

/* handler types */
#define PHP_OUTPUT_HANDLER_INTERNAL 0x0000
#define PHP_OUTPUT_HANDLER_USER 0x0001
/* handler ability flags */
#define PHP_OUTPUT_HANDLER_CLEANABLE 0x0010
#define PHP_OUTPUT_HANDLER_FLUSHABLE 0x0020
#define PHP_OUTPUT_HANDLER_REMOVABLE 0x0040
#define PHP_OUTPUT_HANDLER_STDFLAGS 0x0070
/* handler status flags */
#define PHP_OUTPUT_HANDLER_STARTED 0x1000
#define PHP_OUTPUT_HANDLER_DISABLED 0x2000
#define PHP_OUTPUT_HANDLER_PROCESSED 0x4000

@haszi haszi requested a review from bukka as a code owner January 7, 2024 21:28
@haszi
Copy link
Contributor Author

haszi commented Jan 9, 2024

@Girgias: pinging you because you reviewed all the applicable doc changes.

TL;DR: this change will clear all user supplied flags that are supposed to be set internally only.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

I think the change makes sense, will wait for @bukka's opinion.

This might also require an email to the internals mailing list, just to ensure nobody is opposed.

@haszi haszi requested a review from Girgias January 10, 2024 12:02
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

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

I think it makes sense. Allowing setting those status flags seems like unintended and it's also not documented. I think we should add a line to UPGRADING in case someone uses it. Even if there's some use case, we should probably introduce better and documented API.

@haszi haszi requested a review from bukka January 12, 2024 21:48
@bukka bukka closed this in 3ce7bf2 Feb 9, 2024
@haszi haszi deleted the Clear-handler-status-flag-in-handler-init branch February 9, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants