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

PHP warning thrown visiting non-existent connection pull screen. #1157

Closed
1 task done
peterwilsoncc opened this issue Nov 21, 2023 · 1 comment · Fixed by #1225
Closed
1 task done

PHP warning thrown visiting non-existent connection pull screen. #1157

peterwilsoncc opened this issue Nov 21, 2023 · 1 comment · Fixed by #1225
Assignees
Labels
help wanted needs:engineering This requires engineering to resolve. type:bug Something isn't working.
Milestone

Comments

@peterwilsoncc
Copy link
Collaborator

peterwilsoncc commented Nov 21, 2023

Describe the bug

A PHP warning (notice in PHP 7.x and earlier) can be thrown visiting the pull content screen for an external connection that does not exist.

The error is caused by this block of code assuming than if a connection screen is internal if no valid external connection is found:

if ( is_a( $connection_now, '\Distributor\ExternalConnection' ) ) {
$error_key = "external_{$connection_now->id}";
} else {
$error_key = "internal_{$connection_now->site->blog_id}";
}

As the $connection object isn't an object, the site->blog_id property throws a notice.

The issue can be resolved by validating that the connection is an internal connection before accessing the property. If it's neither an internal or external connection then the function can return early.

Steps to Reproduce

  1. Visit the admin page for an external connection that does not exist /wp-admin/admin.php?page=pull&connection_type=external&connection_id=1 (this presumes Post 1 is not an external connection.
  2. A PHP notice will be thrown

Screenshots, screen recording, code snippet

Log

[21-Nov-2023 03:10:27 UTC] PHP Notice:  Trying to get property 'site' of non-object in /vagrant/content/plugins/distributor/includes/pull-ui.php on line 580
[21-Nov-2023 03:10:27 UTC] PHP Stack trace:
[21-Nov-2023 03:10:27 UTC] PHP   1. {main}() /vagrant/wp-build/wp-admin/admin.php:0
[21-Nov-2023 03:10:27 UTC] PHP   2. do_action($hook_name = 'distributor_page_pull') /vagrant/wp-build/wp-admin/admin.php:259
[21-Nov-2023 03:10:27 UTC] PHP   3. WP_Hook->do_action($args = [0 => '']) /vagrant/wp-build/wp-includes/plugin.php:517
[21-Nov-2023 03:10:27 UTC] PHP   4. WP_Hook->apply_filters($value = '', $args = [0 => '']) /vagrant/wp-build/wp-includes/class-wp-hook.php:348
[21-Nov-2023 03:10:27 UTC] PHP   5. Distributor\PullUI\dashboard('') /vagrant/wp-build/wp-includes/class-wp-hook.php:324
[21-Nov-2023 03:10:27 UTC] PHP   6. Distributor\PullUI\output_pull_errors() /vagrant/content/plugins/distributor/includes/pull-ui.php:527
[21-Nov-2023 03:10:27 UTC] PHP Notice:  Trying to get property 'blog_id' of non-object in /vagrant/content/plugins/distributor/includes/pull-ui.php on line 580
[21-Nov-2023 03:10:27 UTC] PHP Stack trace:
[21-Nov-2023 03:10:27 UTC] PHP   1. {main}() /vagrant/wp-build/wp-admin/admin.php:0
[21-Nov-2023 03:10:27 UTC] PHP   2. do_action($hook_name = 'distributor_page_pull') /vagrant/wp-build/wp-admin/admin.php:259
[21-Nov-2023 03:10:27 UTC] PHP   3. WP_Hook->do_action($args = [0 => '']) /vagrant/wp-build/wp-includes/plugin.php:517
[21-Nov-2023 03:10:27 UTC] PHP   4. WP_Hook->apply_filters($value = '', $args = [0 => '']) /vagrant/wp-build/wp-includes/class-wp-hook.php:348
[21-Nov-2023 03:10:27 UTC] PHP   5. Distributor\PullUI\dashboard('') /vagrant/wp-build/wp-includes/class-wp-hook.php:324
[21-Nov-2023 03:10:27 UTC] PHP   6. Distributor\PullUI\output_pull_errors() /vagrant/content/plugins/distributor/includes/pull-ui.php:527

Environment information

  • PHP 7.4.33
  • Web server: nginx 1.18.0 (Ubuntu)

WordPress information

WordPress 6.4.1
Distributor: Develop

Code of Conduct

  • I agree to follow this project's Code of Conduct
@peterwilsoncc peterwilsoncc added the type:bug Something isn't working. label Nov 21, 2023
@jeffpaul jeffpaul moved this from Incoming to To Do in Open Source Practice Nov 21, 2023
@jeffpaul jeffpaul added this to the 2.1.0 milestone Nov 21, 2023
@jeffpaul jeffpaul added help wanted needs:engineering This requires engineering to resolve. labels Nov 21, 2023
@peterwilsoncc peterwilsoncc changed the title PHP Notice thrown visiting non-existent connection pull screen. PHP warning thrown visiting non-existent connection pull screen. Nov 22, 2023
@peterwilsoncc
Copy link
Collaborator Author

It occurred to me that this is a warning in PHP 8.0 and later so I've updated the description accordingly. See https://3v4l.org/tfHlt

@kirtangajjar kirtangajjar moved this from To Do to Code Review in Open Source Practice Jun 8, 2024
@github-project-automation github-project-automation bot moved this from Code Review to Done in Open Source Practice Jun 11, 2024
@dkotter dkotter modified the milestones: 2.1.0, 2.0.5 Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted needs:engineering This requires engineering to resolve. type:bug Something isn't working.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants