-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add svg support #21
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
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR adds SVG support as a premium feature, introducing a new Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin UI<br/>(React)
participant API as REST API<br/>(PHP)
participant Settings as Settings<br/>Manager
participant WP as WordPress<br/>Hooks
Admin->>API: Load settings (GET)
API->>Settings: Fetch svg_upload & svg_optimization_enabled
Settings->>API: Return current settings
API->>Admin: Return cimoOptions with SVG fields
Admin->>Admin: Parse & populate form state
Admin->>Admin: User toggles SVG options
Admin->>API: Save settings (POST cimo_options)
API->>Settings: Sanitize & persist svg_upload & svg_optimization_enabled
Settings->>WP: Hook: maybe_enable_svg_upload
WP->>WP: Add image/svg+xml to MIME types if enabled
API->>Admin: Confirm save
Admin->>Admin: Update UI state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/admin/class-admin.php`:
- Around line 365-378: The maybe_enable_svg_upload method currently only adds
the SVG MIME type and does not sanitize SVG content, creating an XSS risk; to
fix, integrate server-side SVG sanitization by adding a
wp_handle_upload_prefilter (or wp_handle_upload) hook that intercepts SVG
uploads, passes the uploaded file contents through a sanitizer such as
enshrined/svg-sanitize (sanitizeSvg or SVGSanitize class from that library),
replaces the temporary upload file with the sanitized output (or rejects the
upload if sanitization fails), and keep maybe_enable_svg_upload as-is for MIME
enabling while ensuring all code paths that accept SVGs call your new prefilter
to remove scripts, event attributes, and dangerous elements before saving.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/admin/class-admin.phpsrc/admin/class-script-loader.phpsrc/admin/js/page/admin-settings.js
🧰 Additional context used
🧬 Code graph analysis (2)
src/admin/class-admin.php (1)
src/admin/js/page/admin-settings.js (1)
settings(17-42)
src/admin/class-script-loader.php (1)
src/admin/js/page/admin-settings.js (1)
settings(17-42)
🔇 Additional comments (9)
src/admin/class-script-loader.php (1)
94-95: LGTM - SVG settings properly localized.The new SVG settings follow the same pattern as existing video/audio settings, with appropriate default values (
svgUpload: 0,svgOptimizationEnabled: 1) that match the frontend defaults.src/admin/class-admin.php (2)
116-123: LGTM - REST schema properly extended.The SVG settings schema follows the established pattern for other feature toggles, using integer types consistent with existing settings like
video_optimization_enabled.
294-301: LGTM - Settings sanitization follows established patterns.The sanitization logic for SVG options is consistent with other boolean-like settings in the file.
src/admin/js/page/admin-settings.js (5)
38-41: LGTM - SVG settings initialized with correct defaults.The initial state values match the backend defaults:
svgUpload: 0(disabled by default) andsvgOptimizationEnabled: 1(enabled when uploads are allowed).
98-101: LGTM - Settings loading follows established pattern.The API response mapping correctly converts snake_case keys to camelCase state properties, consistent with other settings.
209-217: LGTM - Reset function follows established pattern.The
applySVGDefaultSettingsfunction is consistent with other reset functions in the file (applyImageDefaultSettings,applyVideoDefaultSettings, etc.).
270-273: LGTM - Save payload correctly includes SVG settings.The snake_case keys match the PHP REST schema, and the values are passed consistently with other boolean-like settings.
791-847: LGTM - SVG settings UI properly implemented.The UI section follows the established pattern from Audio/Video optimization sections:
- Premium feature properly gated behind
buildType === 'premium'- SVG optimization toggle conditionally shown only when uploads are enabled
- Reset button calls the appropriate default settings function
package.json (1)
40-41: SVGO dependency correctly added at latest stable version.The SVGO library is the standard choice for SVG optimization. Version 4.0.0 is the latest stable release, and the caret range
^4.0.0appropriately allows compatible minor and patch updates.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| /** | ||
| * Enable SVG upload if setting is enabled | ||
| */ | ||
| public function maybe_enable_svg_upload( $mimes ) { | ||
| $settings = get_option( 'cimo_options', [] ); | ||
|
|
||
| // If svg_upload is enabled, add svg to allowed mime types | ||
| if ( isset( $settings['svg_upload'] ) && $settings['svg_upload'] === 1 ) { | ||
| $mimes['svg'] = 'image/svg+xml'; | ||
| } | ||
|
|
||
| return $mimes; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Verification inconclusive
Script executed:
# Search for sanitization related to SVG uploads
rg -i "sanitize.*svg|svg.*sanitize" --type phpRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
src/admin/class-admin.php: // Sanitize svg_upload
src/admin/class-admin.php: $sanitized['svg_upload'] = $options['svg_upload'] ? 1 : 0;
src/admin/class-admin.php: // Sanitize svg_optimization_enabled
src/admin/class-admin.php: $sanitized['svg_optimization_enabled'] = $options['svg_optimization_enabled'] ? 1 : 0;
Script executed:
# Search for upload filter hooks and SVG handling
rg -i "wp_handle_upload|upload_mimes|wp_handle_upload_prefilter" --type phpRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
src/admin/class-admin.php: add_filter( 'upload_mimes', [ $this, 'maybe_enable_svg_upload'] );
Script executed:
# Check the full context around the SVG upload method and related code
fd "class-admin.php" --type f | head -1 | xargs wc -lRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
382 src/admin/class-admin.php
Script executed:
# Get the SVG upload filter implementation
fd "class-admin.php" --type f | head -1 | xargs cat -n | sed -n '25,35p'Repository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
25 add_filter( 'intermediate_image_sizes', [ $this, 'disable_thumbnail_generation' ] );
26
27 // Handle WordPress automatic image scaling
28 add_filter( 'big_image_size_threshold', [ $this, 'maybe_disable_wp_scaling' ] );
29
30 // Handle enabling of WordPress SVG upload
31 add_filter( 'upload_mimes', [ $this, 'maybe_enable_svg_upload'] );
32 }
33
34 /**
35 * Add admin menu under Settings
Script executed:
# Check for any sanitization of uploaded SVG files
rg "maybe_enable_svg_upload|sanitize|filter" src/admin/class-admin.php -A 5 -B 5Repository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
add_action( 'admin_menu', [ $this, 'add_admin_menu' ] );
add_action( 'admin_enqueue_scripts', [ $this, 'enqueue_admin_scripts' ] );
}
// Disable thumbnail generation
add_filter( 'intermediate_image_sizes', [ $this, 'disable_thumbnail_generation' ] );
// Handle WordPress automatic image scaling
add_filter( 'big_image_size_threshold', [ $this, 'maybe_disable_wp_scaling' ] );
// Handle enabling of WordPress SVG upload
add_filter( 'upload_mimes', [ $this, 'maybe_enable_svg_upload'] );
}
/**
* Add admin menu under Settings
*/
--
'cimo_options',
'cimo_options',
[
'type' => 'object',
'description' => __( 'Cimo Image Optimizer Settings', 'cimo-image-optimizer' ),
'sanitize_callback' => [ $this, 'sanitize_options' ],
'show_in_rest' => [
'schema' => [
'type' => 'object',
'properties' => [
'webp_quality' => [
--
'cimo_rating',
'cimo_rating_dismissed',
[
'type' => 'string',
'description' => __( 'Tracks if the rating notice has been dismissed.', 'cimo-image-optimizer' ),
'sanitize_callback' => [ $this, 'sanitize_rating_dismissed' ],
'show_in_rest' => [
'schema' => [
'type' => 'string',
'enum' => [ '0', '1' ],
],
--
// Enqueue WordPress component styles
wp_enqueue_style( 'wp-components' );
// This should return the dependencies for both css and js, these will be merged with the default dependencies.
$dependencies = apply_filters( 'cimo/admin/enqueue_admin_scripts', ['css' => [], 'js' => []] );
// Get the build files
$build_dir = plugin_dir_path( CIMO_FILE ) . 'build/admin/';
$build_url = plugin_dir_url( CIMO_FILE ) . 'build/admin/';
--
/**
* Sanitize options
*/
public function sanitize_options( $options ) {
// Load up the complete options so we don't lose any existing settings.
$current = get_option( 'cimo_options', [] );
$sanitized = is_array( $current ) ? $current : [];
// Sanitize webp quality
if ( isset( $options['webp_quality'] ) ) {
$quality = absint( $options['webp_quality'] );
// 0 means disabled/not set
$sanitized['webp_quality'] = $quality > 0 ? max( 1, min( 100, $quality ) ) : 0;
}
// Sanitize max image dimension
if ( isset( $options['max_image_dimension'] ) ) {
$dimension = absint( $options['max_image_dimension'] );
// 0 means disabled/not set
$sanitized['max_image_dimension'] = $dimension;
}
// Sanitize disable_wp_scaling
if ( isset( $options['disable_wp_scaling'] ) ) {
$sanitized['disable_wp_scaling'] = $options['disable_wp_scaling'] ? 1 : 0;
}
// Sanitize disable_thumbnail_generation
if ( isset( $options['disable_thumbnail_generation'] ) ) {
$sanitized['disable_thumbnail_generation'] = $options['disable_thumbnail_generation'] ? 1 : 0;
}
// Sanitize thumbnail sizes
if ( isset( $options['thumbnail_sizes'] ) && is_array( $options['thumbnail_sizes'] ) ) {
$sanitized['thumbnail_sizes'] = array_map( 'sanitize_text_field', $options['thumbnail_sizes'] );
}
// Sanitize lqip_enabled
if ( isset( $options['lqip_enabled'] ) ) {
$sanitized['lqip_enabled'] = $options['lqip_enabled'] ? 1 : 0;
}
if ( isset( $options['lqip_pulse_speed'] ) ) {
$sanitized['lqip_pulse_speed'] = floatval( $options['lqip_pulse_speed'] );
}
if ( isset( $options['lqip_brightness'] ) ) {
$sanitized['lqip_brightness'] = floatval( $options['lqip_brightness'] );
}
if ( isset( $options['lqip_fade_duration'] ) ) {
$sanitized['lqip_fade_duration'] = floatval( $options['lqip_fade_duration'] );
}
// Sanitize video quality
if ( isset( $options['video_optimization_enabled'] ) ) {
$sanitized['video_optimization_enabled'] = $options['video_optimization_enabled'] ? 1 : 0;
}
if ( isset( $options['video_quality'] ) ) {
$sanitized['video_quality'] = intval( $options['video_quality'] );
}
if ( isset( $options['video_max_resolution'] ) ) {
$sanitized['video_max_resolution'] = sanitize_text_field( $options['video_max_resolution'] );
}
// Sanitize audio quality
if ( isset( $options['audio_optimization_enabled'] ) ) {
$sanitized['audio_optimization_enabled'] = $options['audio_optimization_enabled'] ? 1 : 0;
}
if ( isset( $options['audio_quality'] ) ) {
$sanitized['audio_quality'] = intval( $options['audio_quality'] );
}
// Sanitize svg_upload
if ( isset( $options['svg_upload'] ) ) {
$sanitized['svg_upload'] = $options['svg_upload'] ? 1 : 0;
}
// Sanitize svg_optimization_enabled
if ( isset( $options['svg_optimization_enabled'] ) ) {
$sanitized['svg_optimization_enabled'] = $options['svg_optimization_enabled'] ? 1 : 0;
}
return $sanitized;
}
/**
* Sanitize rating dismissed flag
*/
public function sanitize_rating_dismissed( $value ) {
return '1' === (string) $value ? '1' : '0';
}
/**
* Get all registered image sizes
--
}
/**
* Enable SVG upload if setting is enabled
*/
public function maybe_enable_svg_upload( $mimes ) {
$settings = get_option( 'cimo_options', [] );
// If svg_upload is enabled, add svg to allowed mime types
if ( isset( $settings['svg_upload'] ) && $settings['svg_upload'] === 1 ) {
$mimes['svg'] = 'image/svg+xml';
Script executed:
# Search for SVG file processing in the codebase
rg -i "svg" --type php src/ | grep -i "optimize\|process\|filter\|sanitize\|handle" | head -20Repository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
src/admin/class-admin.php: // Handle enabling of WordPress SVG upload
src/admin/class-admin.php: add_filter( 'upload_mimes', [ $this, 'maybe_enable_svg_upload'] );
src/admin/class-admin.php: // Sanitize svg_upload
src/admin/class-admin.php: $sanitized['svg_upload'] = $options['svg_upload'] ? 1 : 0;
src/admin/class-admin.php: // Sanitize svg_optimization_enabled
src/admin/class-admin.php: $sanitized['svg_optimization_enabled'] = $options['svg_optimization_enabled'] ? 1 : 0;
Script executed:
# Check if there's any SVG optimization functionality
fd "*.php" src/ -x grep -l "svg" | xargs grep -l "optimize" 2>/dev/nullRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 123
stderr:
[fd error]: regex parse error:
*.php
^
error: repetition operator missing expression
Note: You can use the '--fixed-strings' option to search for a literal string instead of a regular expression. Alternatively, you can also use the '--glob' option to match on a glob pattern.
Script executed:
# Look for any wp_handle_upload or file upload hooks
rg "wp_handle_upload|wp_check_filetype|before_delete_post|wp_insert_post" --type phpRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 1
Script executed:
# Search the entire plugin for any reference to file processing or sanitization libraries
rg "enshrined|sanitize.*upload|svg.*sanitize" --type phpRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
src/admin/class-admin.php: $sanitized['svg_upload'] = $options['svg_upload'] ? 1 : 0;
Script executed:
# Check for any SVG-related constants or configuration
rg "svg" --type php -i src/ | head -30Repository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
src/admin/class-script-loader.php: 'svgUpload' => isset( $settings['svg_upload'] ) ? (int) $settings['svg_upload'] : 0,
src/admin/class-script-loader.php: 'svgOptimizationEnabled' => isset( $settings['svg_optimization_enabled'] ) ? (int) $settings['svg_optimization_enabled'] : 1,
src/admin/class-admin.php: // Handle enabling of WordPress SVG upload
src/admin/class-admin.php: add_filter( 'upload_mimes', [ $this, 'maybe_enable_svg_upload'] );
src/admin/class-admin.php: // SVG Optimization settings
src/admin/class-admin.php: 'svg_upload' => [
src/admin/class-admin.php: 'svg_optimization_enabled' => [
src/admin/class-admin.php: // Sanitize svg_upload
src/admin/class-admin.php: if ( isset( $options['svg_upload'] ) ) {
src/admin/class-admin.php: $sanitized['svg_upload'] = $options['svg_upload'] ? 1 : 0;
src/admin/class-admin.php: // Sanitize svg_optimization_enabled
src/admin/class-admin.php: if ( isset( $options['svg_optimization_enabled'] ) ) {
src/admin/class-admin.php: $sanitized['svg_optimization_enabled'] = $options['svg_optimization_enabled'] ? 1 : 0;
src/admin/class-admin.php: * Enable SVG upload if setting is enabled
src/admin/class-admin.php: public function maybe_enable_svg_upload( $mimes ) {
src/admin/class-admin.php: // If svg_upload is enabled, add svg to allowed mime types
src/admin/class-admin.php: if ( isset( $settings['svg_upload'] ) && $settings['svg_upload'] === 1 ) {
src/admin/class-admin.php: $mimes['svg'] = 'image/svg+xml';
src/admin/class-meta-box.php: echo '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 672 672" height="20" width="20"><path d="M132.5 132.5C182.4 82.5 253 56 336 56C419 56 489.6 82.5 539.5 132.5C589.4 182.5 616 253 616 336C616 419 589.5 489.6 539.5 539.5C489.5 589.4 419 616 336 616C253 616 182.4 589.5 132.5 539.5C82.6 489.5 56 419 56 336C56 253 82.5 182.4 132.5 132.5zM465.5 273.9C477.6 264.2 479.5 246.6 469.9 234.5C460.3 222.4 442.6 220.5 430.5 230.1C378 272.1 330.3 341.9 306.7 379.4C291.4 359.3 267.2 331.1 239.5 312.6C226.6 304 209.2 307.5 200.7 320.4C192.2 333.3 195.6 350.7 208.5 359.2C237.4 378.5 264.1 415.1 274.1 429.9C281.5 440.9 294 447.9 307.9 447.9C322.3 447.9 335.5 440.3 342.8 428C357.2 403.5 410 318.3 465.6 273.8z"/></svg>';
Script executed:
# Check if plugin has any hooks or filters for upload security
rg "add_filter|apply_filters" --type php src/admin/class-admin.php | grep -i uploadRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
add_filter( 'upload_mimes', [ $this, 'maybe_enable_svg_upload'] );
Script executed:
# Search package.json or composer.json for any SVG-related dependencies
fd "composer.json|package.json" | xargs cat 2>/dev/null | grep -i svgRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
"svgo": "^4.0.0"
Script executed:
# Check if SVGO is used in the upload process
rg -i "svgo" --type php --type js --type json src/ includes/ build/ 2>/dev/null | head -20Repository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
src/admin/class-script-loader.php: 'svgOptimizationEnabled' => isset( $settings['svg_optimization_enabled'] ) ? (int) $settings['svg_optimization_enabled'] : 1,
src/admin/js/page/admin-settings.js: svgOptimizationEnabled: 1,
src/admin/js/page/admin-settings.js: svgOptimizationEnabled: cimoOptions.svg_optimization_enabled !== undefined ? cimoOptions.svg_optimization_enabled : 1,
src/admin/js/page/admin-settings.js: svgOptimizationEnabled: 1,
src/admin/js/page/admin-settings.js: svg_optimization_enabled: settings.svgOptimizationEnabled,
src/admin/js/page/admin-settings.js: checked={ settings.svgOptimizationEnabled === 1 }
src/admin/js/page/admin-settings.js: onChange={ checked => handleInputChange( 'svgOptimizationEnabled', checked ? 1 : 0 ) }
Script executed:
# Look for any file upload handling or processing
rg "wp_handle_upload|upload_prefilter|image/svg" --type phpRepository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
stdout:
src/admin/class-admin.php: $mimes['svg'] = 'image/svg+xml';
Script executed:
# Check if there's any frontend SVG upload processing
fd "*.js" build/ src/ 2>/dev/null | xargs grep -l "svg" 2>/dev/null | head -5Repository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
Script executed:
# Check the README or documentation for SVG security information
cat README.md 2>/dev/null | grep -i "svg\|security\|sanitize" -A 2 -B 2 | head -30Repository: gambitph/Cimo
Repository: gambitph/Cimo
Exit code: 0
SVG uploads enabled without content sanitization—creates XSS vulnerability.
The maybe_enable_svg_upload() method only adds the SVG MIME type to allowed uploads without validating SVG file content. SVG files can contain embedded <script> tags, event handlers (onload, onclick), and other malicious payloads that execute when the file is rendered, creating an XSS vulnerability.
The plugin's settings sanitization (lines 294–301) only validates the svg_upload boolean setting, not the uploaded SVG file content. SVGO (in dependencies) is a Node.js optimization tool and does not provide upload-time security validation.
Implement SVG content sanitization before upload:
- Use a library like
enshrined/svg-sanitizeto strip dangerous elements - Add the
wp_handle_upload_prefilterhook to sanitize SVG content before saving
🤖 Prompt for AI Agents
In `@src/admin/class-admin.php` around lines 365 - 378, The
maybe_enable_svg_upload method currently only adds the SVG MIME type and does
not sanitize SVG content, creating an XSS risk; to fix, integrate server-side
SVG sanitization by adding a wp_handle_upload_prefilter (or wp_handle_upload)
hook that intercepts SVG uploads, passes the uploaded file contents through a
sanitizer such as enshrined/svg-sanitize (sanitizeSvg or SVGSanitize class from
that library), replaces the temporary upload file with the sanitized output (or
rejects the upload if sanitization fails), and keep maybe_enable_svg_upload
as-is for MIME enabling while ensuring all code paths that accept SVGs call your
new prefilter to remove scripts, event attributes, and dangerous elements before
saving.
fixes #16
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.