Enhanced color sanitization to handle non-string values

[girishpanchal30]

Summary

Checked that the color sanitization function handles non-string values gracefully, returning an empty string instead of causing a fatal error.

Check before Pull Request is ready:

• I have written a test and included it in this PR
• I have run all tests and they pass
• The code passes when running the PHP CodeSniffer
• Code meets WordPress Coding Standards for PHP, HTML, CSS and JS
• Security and Sanitization requirements have been followed
• I have assigned a reviewer or two to review this PR

Closes #4567

[pirate-bot]

Plugin build for 82b3b04 is ready 🛎️!

• Download build

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR hardens Neve’s color/background sanitization against non-string inputs and adds tests to prevent fatals and null returns during sanitization.

Changes:

• Add a PHPUnit test covering neve_sanitize_colors() behavior for arrays, objects, booleans, null, invalid strings, and valid colors.
• Update neve_sanitize_colors() to early-return for non-scalar inputs and normalize sanitize_hex_color() null to ''.
• Add input-type guarding in neve_sanitize_rgba() and make neve_sanitize_background() more defensive for missing keys.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/test-neve-sanitization.php	Adds regression tests ensuring color sanitization is resilient to non-string values.
globals/sanitize-functions.php	Makes sanitizers safer with type checks and fallback defaults (avoids fatals/notices and null returns).

Suppressed comments (1)

globals/sanitize-functions.php:69

• The default channel values are initialized to the string 'rgba(0,0,0,0)', so if sscanf() fails to match (e.g. malformed rgba(...)), the function can return an invalid string like rgba(rgba(0,0,0,0),...). Initialize $red/$green/$blue to integer 0 and $alpha to float 0 (or 0.0), and check the return value of sscanf(); if it doesn’t parse all 4 components, return 'rgba(0,0,0,0)'.

function neve_sanitize_rgba( $value ) {
	if ( ! is_string( $value ) ) {
		return 'rgba(0,0,0,0)';
	}

	$red   = 'rgba(0,0,0,0)';
	$green = 'rgba(0,0,0,0)';
	$blue  = 'rgba(0,0,0,0)';
	$alpha = 'rgba(0,0,0,0)';   // If empty or an array return transparent

	// By now we know the string is formatted as an rgba color so we need to further sanitize it.
	$value = str_replace( ' ', '', $value );
	sscanf( $value, 'rgba(%d,%d,%d,%f)', $red, $green, $blue, $alpha );

	return 'rgba(' . $red . ',' . $green . ',' . $blue . ',' . $alpha . ')';
}

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

globals/sanitize-functions.php:56

• neve_sanitize_colors() now normalizes invalid inputs to '', but neve_sanitize_rgba() returns 'rgba(0,0,0,0)' for non-string inputs. This introduces inconsistent “invalid value” semantics between the two public sanitizers. Consider aligning the behavior (or explicitly documenting the difference) so callers don’t have to handle two different invalid-sentinel values for closely related sanitizers.

function neve_sanitize_rgba( $value ) {
	if ( ! is_string( $value ) ) {
		return 'rgba(0,0,0,0)';
	}

globals/sanitize-functions.php:185

• The repeated isset(...) ? ... : ... pattern makes this block harder to scan and maintain. If the project’s minimum PHP version supports it, using the null coalescing operator (??) would simplify the expressions and reduce duplication while preserving the same behavior.

	$value['imageUrl']          = esc_url( isset( $value['imageUrl'] ) ? $value['imageUrl'] : '' );
	$value['colorValue']        = neve_sanitize_colors( isset( $value['colorValue'] ) ? $value['colorValue'] : '' );
	$value['overlayColorValue'] = neve_sanitize_colors( isset( $value['overlayColorValue'] ) ? $value['overlayColorValue'] : '' );


	$value['overlayOpacity'] = isset( $value['overlayOpacity'] ) ? (int) $value['overlayOpacity'] : 0;

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (4)

globals/sanitize-functions.php:40

• The docblock for neve_sanitize_colors() is now inaccurate: the function accepts CSS variables (var(...)) and gradients in addition to hex/RGBA. Update the description and @param text to reflect supported formats (hex, rgba/hsl functions via neve_sanitize_rgba, CSS var(...), and gradient strings).

/**
 * Function to sanitize alpha color.
 *
 * @param mixed $value Hex or RGBA color.
 *
 * @return string
 */
function neve_sanitize_colors( $value ) {

globals/sanitize-functions.php:30

• neve_is_css_var() relies on a recursive PCRE pattern ((?P>nv_var)), which can be abused with very deep nesting to trigger high CPU usage or recursion/backtracking limits (potential DoS) if the input is user-controlled. Add a cheap guard before preg_match (e.g., reject values exceeding a reasonable max length and/or cap nesting by disallowing excessive var( occurrences).

	$hex      = '#(?:[0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})';
	$color_fn = '(?:rgb|rgba|hsl|hsla)\(\s*[0-9a-z.,%\/\s-]+\)';
	$keyword  = '[a-z]+(?:-[a-z]+)*';
	$fallback = '(?:(?P>nv_var)|' . $hex . '|' . $color_fn . '|' . $keyword . ')';
	$pattern  = '/^(?P<nv_var>var\(\s*--[a-z0-9_-]+\s*(?:,\s*' . $fallback . '\s*)?\))$/i';

	return (bool) preg_match( $pattern, trim( $value ) );

globals/sanitize-functions.php:204

• The repeated isset(...) ? ... : ... pattern makes this block harder to scan and easier to get wrong as fields evolve. Prefer null coalescing ($value['key'] ?? '') (and then cast where needed) to reduce duplication and improve readability.

	$value['imageUrl']          = esc_url( isset( $value['imageUrl'] ) ? $value['imageUrl'] : '' );
	$value['colorValue']        = neve_sanitize_colors( isset( $value['colorValue'] ) ? $value['colorValue'] : '' );
	$value['overlayColorValue'] = neve_sanitize_colors( isset( $value['overlayColorValue'] ) ? $value['overlayColorValue'] : '' );


	$value['overlayOpacity'] = isset( $value['overlayOpacity'] ) ? (int) $value['overlayOpacity'] : 0;

globals/sanitize-functions.php:45

• neve_sanitize_colors() now rejects non-string/non-numeric inputs early. This is good for arrays/objects, but it also changes behavior for “stringable” objects (objects implementing __toString()), which previously would have been coerced to string by strpos()/casting in older code paths. If backwards compatibility matters here, consider treating Stringable/__toString() objects as acceptable input and casting them to string before validation.

	if ( ! is_string( $value ) && ! is_numeric( $value ) ) {
		return '';
	}

	$value = (string) $value;

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

globals/sanitize-functions.php:55

• Returning the raw (trimmed) CSS var string removes the previous sanitize_text_field() defense-in-depth for this branch. Even with a restrictive regex, it’s safer and more consistent to additionally run the validated value through sanitize_text_field() (e.g., on the trimmed string) to strip control characters/newlines and align with prior behavior.

	if ( neve_is_css_var( $value ) ) {
		return trim( $value );
	}

globals/sanitize-functions.php:28

• The 200 max-length threshold is a magic number. Consider extracting it into a named constant (or at least adding a brief comment explaining why 200 is the chosen limit) so future changes to the regex/recursion have a clear rationale and don’t accidentally weaken/overconstrain validation.

	if ( $value === '' || strlen( $value ) > 200 ) {
		return false;
	}