Skip to content

Validate repo_info before accessing version#92

Open
apermo wants to merge 6 commits into
Zodiac1978:developfrom
apermo:fix/validate-repo-info
Open

Validate repo_info before accessing version#92
apermo wants to merge 6 commits into
Zodiac1978:developfrom
apermo:fix/validate-repo-info

Conversation

@apermo

@apermo apermo commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reject plugins_api() responses missing the version property before storing as repo_info (prevents caching invalid data)
  • Add isset( $report['repo_info']->version ) guard in the version column of render_table_row() (protects against stale cache entries)

This is consistent with how tested, last_updated, num_ratings, and rating are already guarded with isset() on lines 536, 546, 554.

Fixes #56

Context

plugins_api() can return a valid stdClass (not a WP_Error) that lacks expected properties. This happens when a premium plugin's slug collides with a wordpress.org entry that returns invalid data (e.g. Search & Filter Pro — see #56 comments). The object passes the ! is_wp_error() check but accessing ->version triggers PHP warnings.

Test in WordPress Playground

Both Playground instances include a mu-plugin that strips the version property from Classic Editor's plugins_api() response, reproducing the Search & Filter Pro scenario from #56.

Before (develop — shows the bug): Open in WordPress Playground

After (this branch — bug fixed): Open in WordPress Playground

Both instances include a mu-plugin that hooks plugins_api_result to strip the version property from Classic Editor's API response, reproducing the Search & Filter Pro scenario. In the Before instance, this causes PHP warnings that corrupt the AJAX response — the Classic Editor row fails to load. In the After instance, the invalid response is handled gracefully.

Pre-installed plugins:

Test plan

  • Open the Before link — Classic Editor row fails to load or shows PHP warnings
  • Open the After link — Classic Editor shows "No data available" cleanly, no PHP warnings
  • In the After link, verify other plugins (Apermo Admin Bar, Akismet, Hello Dolly) still show full version/update/rating data

@apermo

apermo commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

Fixed — moved the isset( $returned_object->version ) check inside the non-error branch so the else is only reached for WP_Error. PHPStan can now narrow the type correctly.

Comment thread rt-plugin-report.php Outdated
$report['repo_info'] = $returned_object;
// Cache the report.
set_site_transient( $cache_key, $report, self::CACHE_LIFETIME );
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Zodiac1978 Maybe we could add an else branch here to log the error? Your thoughts?

apermo added 2 commits April 14, 2026 15:42
plugins_api() can return a valid object (not WP_Error)
that lacks expected properties like `version`. This
happens with plugins like Search & Filter Pro whose
slug collides with a wp.org entry returning invalid
data.

Guard against this in two places:
- assemble_plugin_report(): reject API responses
  missing `version` before caching as repo_info
- render_table_row(): add isset() check on version,
  consistent with tested/rating/last_updated guards

Fixes Zodiac1978#56
Move the isset(version) check inside the non-error
branch so the else is only reached for WP_Error,
letting PHPStan narrow the type correctly.
@Zodiac1978 Zodiac1978 added the bug Something isn't working label Jun 21, 2026
@Zodiac1978 Zodiac1978 modified the milestones: 2.3, 2.2.4 Jun 21, 2026
@Zodiac1978

Copy link
Copy Markdown
Owner

@apermo I fixed the line endings and indentation issues I caused by the GitHub web editor in branch fix-line-endings-pr-92.

Could you cherry-pick commit 7305482 into this PR branch?

@apermo

apermo commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@Zodiac1978 Cherry-picked 7305482 onto the PR branch — it was a clean
fast-forward, so it's your exact commit and the file is consistent LF now. ✅

While looking at the red CI, I confirmed all three failing checks are
pre-existing and unrelated to the line-ending fix (that commit is
whitespace-only). Quick rundown so we know what we're dealing with:

WordPress Coding Standards (3 errors)

  • WordPress.Files.FileName.InvalidClassFileName (line 1) — WPCS expects the
    main file to be named class-rt-plugin-report.php. There's no suppression
    for it and it isn't auto-fixable, so this one alone keeps the check red.
  • Universal.ControlStructures.DisallowLonelyIf (lines 469 & 536) — the
    // phpcs:ignore comments are present, but they're on the if line while
    the sniff reports the violation on the else line just above, so they never
    take effect. Cleanest fix is to convert } else { if (...) { … } } to
    } elseif (...) { … } and drop the ignores.

Plugin Check (1 error + warnings)

  • Error hidden_files — flags tracked dotfiles/dirs (.github,
    .wordpress-org, .distignore, .gitignore, .wp-env.json).
  • Warnings only: trademark term "plugin" in the name/slug, unprefixed class
    RT_Plugin_Report, and update-checker detection.

I'm happy to fix the WPCS errors properly — either a small .phpcs.xml.dist
ruleset (set text-domain/prefixes and relax the single-file-plugin naming rule)
plus the elseif cleanup, or just the elseif cleanup if you'd rather keep the
config minimal. The Plugin Check hidden_files/trademark items are more of a
CI-config/policy call on your end.

Want me to push the fix into this PR, or would you prefer a separate one?

@Zodiac1978

Copy link
Copy Markdown
Owner

Let's do it in a separate one.

Thank you so much for working on those issues!

@apermo

apermo commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

I will prepare the PR as Draft, with the plan to rebase once the rest is merged.

@apermo

apermo commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

The CI coding-standards fixes are now in #97 (draft, branched from develop). It clears the WordPress Coding Standards errors via a project .phpcs.xml.dist ruleset plus the elseif cleanup. I've left it as a draft with a note to rebase once the other open PRs touching rt-plugin-report.php (including this one) are merged.

@Zodiac1978

Copy link
Copy Markdown
Owner

Looks like I need to look up how I can rebase. Never done that before 😬

@apermo

apermo commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Looks like I need to look up how I can rebase. Never done that before 😬

Will do that. I can also show you in Mannheim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PHP warnings when loading the report screen the second time

2 participants