Skip to content

[Performance] Replace direct database queries with cached WP_Query implementations #40

@pbking

Description

@pbking

Issue: Direct Database Queries with Unsanitized Parameters

Description

The codebase uses direct database query functions (get_posts(), get_page_by_path()) with user-supplied parameters that lack proper sanitization and escaping. This presents both security and performance concerns.

Current Problems

1. Unsanitized Parameters

  • Location: class-pattern-builder-controller.php:28-33
  • Issue: get_posts() with $pattern->name derived parameters
  • Risk: Potential SQL injection if pattern names contain malicious content

2. Multiple Instances of Similar Issues

  • Line 46: get_page_by_path() with unsanitized pattern slug
  • Line 145: get_page_by_path() with unsanitized pattern slug
  • Line 500: get_page_by_path() with unsanitized pattern name
  • Line 508: get_page_by_path() with unsanitized pattern slug
  • Line 615: get_page_by_path() with unsanitized pattern name

3. Performance Issues

  • No caching of frequently accessed pattern queries
  • Repeated database hits for same pattern lookups
  • No use of transients for expensive operations

Security Risk

Medium-High - While WordPress functions provide some built-in sanitization, passing unsanitized user input directly to database query functions is not a best practice and could lead to issues.

Code Examples

Current Problematic Implementation

// Line 28-33: Direct get_posts() with unsanitized parameter
$posts = get_posts(
    array(
        'name'      => $path, // $path comes from unsanitized $pattern->name
        'post_type' => 'tbell_pattern_block',
    )
);

// Line 500: Direct get_page_by_path() with user input
$post = get_page_by_path( $pattern->name, OBJECT, 'wp_block' );

Recommended Solutions

1. Use WP_Query with Proper Escaping

$query = new WP_Query(array(
    'name' => sanitize_title($path),
    'post_type' => 'tbell_pattern_block',
    'posts_per_page' => 1,
    'no_found_rows' => true,
    'update_post_meta_cache' => false,
    'update_post_term_cache' => false,
));

2. Implement Query Caching

$cache_key = 'pattern_post_' . md5($pattern->name);
$post = get_transient($cache_key);

if (false === $post) {
    // Perform query
    $post = $this->get_pattern_post_safe($pattern);
    set_transient($cache_key, $post, HOUR_IN_SECONDS);
}

3. Add Input Sanitization

public function get_pattern_post_safe($pattern) {
    $sanitized_name = sanitize_title_with_dashes($pattern->name);
    $sanitized_name = wp_strip_all_tags($sanitized_name);
    
    // Use sanitized input in queries
}

Implementation Checklist

  • Replace all get_posts() calls with secured WP_Query implementations
  • Replace get_page_by_path() calls with sanitized alternatives
  • Add input sanitization for all pattern name parameters
  • Implement transient caching for frequently accessed patterns
  • Add proper error handling for failed queries
  • Update PHPDoc blocks to document security measures
  • Add unit tests for sanitization and caching

Performance Benefits

  • Reduced Database Load: Caching prevents repeated queries for same patterns
  • Optimized Queries: WP_Query allows fine-tuned query optimization
  • Better Scaling: Transient caching improves performance under load

Security Benefits

  • Input Sanitization: Proper escaping prevents potential injection issues
  • Validation: Explicit input validation catches malformed data
  • WordPress Standards: Follows WordPress coding standards for database access

Affected Files

  • includes/class-pattern-builder-controller.php

Testing Requirements

  • Verify all pattern queries work with sanitized inputs
  • Test caching functionality and cache invalidation
  • Performance testing to measure improvement
  • Security testing with various input patterns

Priority: High - Addresses both security and performance concerns

Estimated Effort: Medium - Requires refactoring multiple query methods

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions