Skip to content

pr: [Nightly Fix] - Null Safety - Guard External Cache Interval#1

Open
jewel-claw wants to merge 1 commit intomasterfrom
nightly-fix/cache-interval-guard
Open

pr: [Nightly Fix] - Null Safety - Guard External Cache Interval#1
jewel-claw wants to merge 1 commit intomasterfrom
nightly-fix/cache-interval-guard

Conversation

@jewel-claw
Copy link

What

  • fix the external cache helpers to return early when caching_interval is empty or missing

Why

  • the previous condition dereferenced caching_interval even when it was unset, which can trigger notices and skip the intended early return

Fix

  • replace the contradictory isset check with an empty guard in both cache helper functions

Confidence

  • linted boot/ninja-tables-global-function.php with php -l

@greptile-apps
Copy link

greptile-apps bot commented Mar 10, 2026

PR author is not in the allowed authors list.

@qodo-code-review
Copy link

Review Summary by Qodo

Fix null safety in external cache interval validation

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix null safety issue in external cache interval guards
• Replace contradictory isset/AND logic with empty check
• Prevent undefined index notices in cache helper functions
• Remove trailing whitespace from file
Diagram
flowchart LR
  A["Old Logic: isset AND dereference"] -->|"Contradictory condition"| B["Potential undefined notices"]
  C["New Logic: empty check"] -->|"Safe guard"| D["Early return on empty interval"]
  B -->|"Fix applied"| D
Loading

Grey Divider

File Changes

1. boot/ninja-tables-global-function.php 🐞 Bug fix +2/-3

Guard external cache interval with empty check

• Replace contradictory `! isset($tableSettings['caching_interval']) &&
 $tableSettings['caching_interval'] with empty($tableSettings['caching_interval'])` in
 ninjaTableGetExternalCachedData()
• Apply same fix to ninjaTableSetExternalCacheData() function
• Remove trailing whitespace at end of file

boot/ninja-tables-global-function.php


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Mar 10, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Unguarded settings array access 🐞 Bug ⛯ Reliability
Description
ninjaTableGetExternalCachedData() and ninjaTableSetExternalCacheData() call
empty($tableSettings['caching_interval']) immediately after get_post_meta() without verifying
$tableSettings is an array. If _ninja_table_settings is missing/falsy/non-array, the array-offset
access can throw PHP warnings/errors before the intended early return.
Code

boot/ninja-tables-global-function.php[R395-397]

+    if (empty($tableSettings['caching_interval'])) {
        return false;
    }
Evidence
Both external cache helpers directly index into $tableSettings after get_post_meta() (no
type/emptiness normalization), while other code paths in this repo explicitly guard get_post_meta()
results and fall back to defaults when settings are falsy. Additionally, default settings don’t
include caching_interval, so the key/meta being absent is a normal state that must be handled
safely.

boot/ninja-tables-global-function.php[392-400]
boot/ninja-tables-global-function.php[412-417]
boot/ninja-tables-global-function.php[24-27]
boot/ninja-tables-global-function.php[36-40]
boot/ninja-tables-global-function.php[64-85]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ninjaTableGetExternalCachedData()` and `ninjaTableSetExternalCacheData()` use `empty($tableSettings['caching_interval'])` right after `get_post_meta()`. If `_ninja_table_settings` is missing/falsy/non-array, the array-offset access can error before the early-return executes.

### Issue Context
The codebase already treats `get_post_meta()` results as potentially falsy/non-array and normalizes settings to defaults in `ninja_table_get_table_settings()`. Also, default settings do not define `caching_interval`, so this key can be absent.

### Fix Focus Areas
- boot/ninja-tables-global-function.php[392-401]
- boot/ninja-tables-global-function.php[412-417]

### Implementation notes
Use one of:
- `if (!is_array($tableSettings) || empty($tableSettings['caching_interval'])) return false;`
- or replace `get_post_meta(... '_ninja_table_settings' ...)` with `ninja_table_get_table_settings(...)` (ensures an array), then `empty($tableSettings['caching_interval'])` is safe.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@kilo-code-bot
Copy link

kilo-code-bot bot commented Mar 10, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0

Analysis

The PR correctly fixes a logic bug in the original code:

  • Original (buggy): if ( ! isset($tableSettings['caching_interval']) && $tableSettings['caching_interval'])

    • This condition is contradictory: it checks "if NOT set AND truthy" - which can never be true
    • Would cause undefined index notices and never properly guard the cache functions
  • Fixed: if (empty($tableSettings['caching_interval']))

    • Correctly checks if the value is not set, null, 0, empty string, or false
    • Properly guards both ninjaTableGetExternalCachedData() and ninjaTableSetExternalCacheData() functions

The fix is valid and addresses a null safety issue in the external cache interval validation.

Files Reviewed (1 file)
  • boot/ninja-tables-global-function.php - Bug fix (correct)

Comment on lines +395 to 397
if (empty($tableSettings['caching_interval'])) {
return false;
}

Choose a reason for hiding this comment

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

Action required

1. Unguarded settings array access 🐞 Bug ⛯ Reliability

ninjaTableGetExternalCachedData() and ninjaTableSetExternalCacheData() call
empty($tableSettings['caching_interval']) immediately after get_post_meta() without verifying
$tableSettings is an array. If _ninja_table_settings is missing/falsy/non-array, the array-offset
access can throw PHP warnings/errors before the intended early return.
Agent Prompt
### Issue description
`ninjaTableGetExternalCachedData()` and `ninjaTableSetExternalCacheData()` use `empty($tableSettings['caching_interval'])` right after `get_post_meta()`. If `_ninja_table_settings` is missing/falsy/non-array, the array-offset access can error before the early-return executes.

### Issue Context
The codebase already treats `get_post_meta()` results as potentially falsy/non-array and normalizes settings to defaults in `ninja_table_get_table_settings()`. Also, default settings do not define `caching_interval`, so this key can be absent.

### Fix Focus Areas
- boot/ninja-tables-global-function.php[392-401]
- boot/ninja-tables-global-function.php[412-417]

### Implementation notes
Use one of:
- `if (!is_array($tableSettings) || empty($tableSettings['caching_interval'])) return false;`
- or replace `get_post_meta(... '_ninja_table_settings' ...)` with `ninja_table_get_table_settings(...)` (ensures an array), then `empty($tableSettings['caching_interval'])` is safe.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

1 participant