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

Usage of get_current_screen() without checking its availability leaading to potential fatal error #2515

Closed
maximus80 opened this issue Mar 29, 2023 · 3 comments
Labels
[Priority] Low This issue/pull request is not immediate [Type] Bug Something that is not working as expected
Milestone

Comments

@maximus80
Copy link

Describe the bug:

Possible fatal error, due to function get_current_screen() not always being defined here: https://github.com/godaddy-wordpress/coblocks/blob/master/includes/block-migrate/loader.php#L30

The problem is, that the code automatically assumes that the function is defined, which is not always the case. Only if screen.php or admin.php is loaded, it will be defined.

Since the code gets executed on the_post action, it can happen during any 3rd party plugin request that loops through posts using the_post().

To reproduce:

Create a custom query from a plugin that uses the_post() that runs without screen.php or admin.php being loaded.

Expected behavior:

  • CoBlocks should not use an anonymous function here, which can't be unhooked easily.
  • CoBlocks should checks if get_current_screen() is actually defined.
  • CoBlocks should makes sure screen.php or admin.php are included:
    require_once ABSPATH . 'wp-admin/includes/screen.php';
    require_once ABSPATH . 'wp-admin/admin.php';
  • Additionally, CoBlocks should not access member variable is_block_editor without check.

Isolating the problem:

  • [x ] I can reproduce this bug consistently using the steps above

WordPress version:

6.1.1+

@maximus80 maximus80 added [Priority] Low This issue/pull request is not immediate [Type] Bug Something that is not working as expected labels Mar 29, 2023
@AnthonyLedesma
Copy link
Member

@maximus80 Thank you for taking the time to make this issue! We just merged in a fix for the missing dependency in some situations and will be pushing out a patch release.

@AnthonyLedesma
Copy link
Member

handled with #2514

@AnthonyLedesma AnthonyLedesma added this to the 3.0.2 milestone Mar 29, 2023
@maximus80
Copy link
Author

Glad to hear that, thanks for the quick action 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Priority] Low This issue/pull request is not immediate [Type] Bug Something that is not working as expected
Projects
None yet
Development

No branches or pull requests

2 participants