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

Add header_register_callback builtin support #1101

Merged
merged 11 commits into from
Oct 3, 2024

Conversation

PetrShumilov
Copy link
Contributor

@PetrShumilov PetrShumilov commented Sep 12, 2024

Add compatible with PHP 7.4.0 support of header_register_callback builtin.

Related issues: KPHP-1860

NOTE:

  • After discussion with @DrDet we've decided to prohibit exceptions throwing from callback
  • f$send_http_103_early_hints shouldn't be a cause of callback invocation

@PetrShumilov PetrShumilov added enhancement New feature or request runtime Feature related to runtime missing functionality Missing PHP functions, classes, constants and etc labels Sep 12, 2024
@PetrShumilov PetrShumilov added this to the next milestone Sep 12, 2024
@PetrShumilov PetrShumilov self-assigned this Sep 12, 2024
runtime/interface.cpp Outdated Show resolved Hide resolved
runtime/interface.cpp Outdated Show resolved Hide resolved
runtime/interface.cpp Outdated Show resolved Hide resolved
@PetrShumilov PetrShumilov force-pushed the pshumilov/add_header_register_callback branch 3 times, most recently from e556b80 to 6759d6d Compare September 16, 2024 15:21
astrophysik
astrophysik previously approved these changes Sep 17, 2024
runtime/interface.cpp Outdated Show resolved Hide resolved
runtime/interface.cpp Outdated Show resolved Hide resolved
astrophysik
astrophysik previously approved these changes Sep 18, 2024
Copy link
Contributor

@DrDet DrDet left a comment

Choose a reason for hiding this comment

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

The main idea behind header_register_callback is to give opportunity to modify headers before any output is sent.
Output can be sent in the middle of script in some cases. Off the top of my head, I can remember at least 2 such cases:

  • f$flush
    There's a test for that case, but for some reason it's not correct
  • f$send_http_103_early_hints

Now these cases are skipped, and we invoke callback only in the end of the script. Now it behaves just like register_shutdown_function and it's incorrect

For example:

<?php

function main() {
    header_register_callback(function() {
        fprintf(STDERR, "header_callback invoked\n");
        header("Qwerty: hello");
    });
    fprintf(STDERR, "before flush()\n");
    echo "111111111\n";
    flush();
    echo "222222222\n";
    fprintf(STDERR, "after flush()\n");
}

main();

Expected behaviour here is to have Qwerty: hello response header, but that's not really the case.

Let's do the following:

  • find and fix all skipped cases like that
  • add test with callback, which sends some RPC to check swapcontext correctness

tests/python/tests/http_server/php/index.php Outdated Show resolved Hide resolved
@PetrShumilov PetrShumilov force-pushed the pshumilov/add_header_register_callback branch 2 times, most recently from a4ec3eb to 3ad05b8 Compare September 25, 2024 12:39
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
runtime/interface.cpp Outdated Show resolved Hide resolved
runtime/interface.cpp Outdated Show resolved Hide resolved
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
@PetrShumilov PetrShumilov force-pushed the pshumilov/add_header_register_callback branch from 3ad05b8 to 062d5f0 Compare September 30, 2024 11:29
Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
@PetrShumilov PetrShumilov merged commit 2206f5e into master Oct 3, 2024
5 checks passed
@PetrShumilov PetrShumilov deleted the pshumilov/add_header_register_callback branch October 3, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request missing functionality Missing PHP functions, classes, constants and etc runtime Feature related to runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants