Skip to content

Sanitize inputs, fix gallery/transcoder tweaks#1722

Open
sabbir1991 wants to merge 1 commit intodevelopfrom
fix/random-tweaks
Open

Sanitize inputs, fix gallery/transcoder tweaks#1722
sabbir1991 wants to merge 1 commit intodevelopfrom
fix/random-tweaks

Conversation

@sabbir1991
Copy link
Copy Markdown
Member

@sabbir1991 sabbir1991 commented Mar 12, 2026

Here are some fixes done in this PR

  • Sanitized and escaped form inputs for CF7, Gravity Forms, and SureForms.
  • Fixed MetForm shortcode attribute formatting for form ID.
  • Corrected inverted tag condition logic.
  • Removed invalid CORS credentials header with wildcard origin.
  • Escaped shortcode output for MetForm and Ninja Forms render paths.
  • Optimized SEO schema aggregation to avoid repeated merge overhead in loops.
  • Updated transcoder error handling so all non-200 upload responses are treated as failures.
  • Tightened retranscode eligibility check to require explicit Active status.
  • Converted poll list query to prepared SQL format.
  • Added meta cache priming before gallery loops to reduce per-item DB/meta lookups.
  • Removed repeated option fetch inside gallery fallback URL generation.
  • Restored missing $video_id initialization in dynamic gallery loop.

Copilot AI review requested due to automatic review settings March 12, 2026 09:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR focuses on hardening/sanitizing several REST/shortcode inputs, improving gallery rendering performance, and tightening transcoder-related logic.

Changes:

  • Sanitize/escape REST request params for multiple form integrations (GF, CF7, Sureforms) and tweak Metform shortcode usage.
  • Prime post meta caches in gallery renderers and simplify gallery permalink fallback logic.
  • Fix transcoder/tag handling edge cases and tighten failure handling/status checks.

Reviewed changes

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

Show a summary per file
File Description
inc/classes/shortcodes/class-godam-video-gallery.php Primes meta cache for gallery attachments; simplifies fallback URL construction.
inc/classes/rest-api/class-sureforms.php Sanitizes id param with absint().
inc/classes/rest-api/class-polls.php Switches custom-table query to use $wpdb->prepare() with identifier placeholder.
inc/classes/rest-api/class-gf.php Sanitizes id and theme params and escapes theme in shortcode.
inc/classes/rest-api/class-dynamic-gallery.php Formatting cleanups, meta-cache priming, and simplified fallback URL construction.
inc/classes/rest-api/class-cf7.php Sanitizes id and theme params and escapes theme in shortcode.
inc/classes/rest-api/class-ads.php Removes invalid CORS credential header when allowing * origin.
inc/classes/ninja-forms/class-ninja-forms-integration.php Wraps shortcode output in wp_kses_post().
inc/classes/metform/class-metform-rest-api.php Quotes shortcode attribute value for form_id.
inc/classes/metform/class-metform-integration.php Wraps shortcode output in wp_kses_post().
inc/classes/class-seo.php Reduces repeated array_merge() calls by chunking results before merging.
admin/godam-transcoder-functions.php Fixes tag-array conditional so tags are actually collected.
admin/class-rtgodam-transcoder-handler.php Treats any non-200 transcoder response as failure (vs only 5xx).
admin/class-rtgodam-retranscodemedia.php Requires explicit 'Active' usage status before allowing retranscode.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 158 to 160
if ( 'ninja-forms' === $layer && ! empty( $layer_id ) ) {
echo do_shortcode( "[ninja_form id='{$layer_id}']" ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
echo wp_kses_post( do_shortcode( "[ninja_form id='{$layer_id}']" ) );
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Using wp_kses_post() around the Ninja Forms shortcode output will strip form-related tags/attributes (e.g.,

, , ), which can break the form rendering inside the video editor iframe. Also, $layer_id is interpolated into the shortcode without sanitization, which allows shortcode-attribute injection if $layer_id is not strictly numeric. Consider sanitizing $layer_id with absint() (and bailing if empty) and either (a) outputting the trusted shortcode result without KSES (with an explicit PHPCS ignore + rationale), or (b) using wp_kses() with a custom allowlist that includes the required form elements/attributes.

Copilot uses AI. Check for mistakes.
Comment on lines 129 to 131
if ( 'metform' === $layer && ! empty( $layer_id ) ) {
echo do_shortcode( "[metform form_id='{$layer_id}']" ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
echo wp_kses_post( do_shortcode( "[metform form_id='{$layer_id}']" ) );
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Using wp_kses_post() around the Metform shortcode output will strip form-related markup (WordPress' 'post' KSES set typically removes

// etc.), which is likely to break the embedded form rendering in the video editor iframe. Also, $layer_id is interpolated into the shortcode without sanitization, which can allow shortcode-attribute injection if $layer_id is not guaranteed numeric. Sanitize $layer_id (e.g., absint + early return) and either output the trusted shortcode result unfiltered (with an explicit PHPCS ignore + comment) or use wp_kses() with a custom allowlist that permits the necessary form tags/attributes.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
16 0 16

⚠️ Warnings (16)

📁 composer.json (1 warning)
📍 Line 🔖 Check 💬 Message
0 missing_composer_json_file The "/vendor" directory using composer exists, but "composer.json" file is missing.
📁 readme.txt (2 warnings)
📍 Line 🔖 Check 💬 Message
0 mismatched_plugin_name Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0 trademarked_term The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.
📁 assets/build/css/main.css (1 warning)
📍 Line 🔖 Check 💬 Message
0 EnqueuedStylesScope This style is being loaded in all contexts.
📁 assets/src/libs/analytics.min.js (6 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/03/12/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/?godam-video=demo-godam-video-post (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
📁 assets/build/js/main.min.js (6 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/03/12/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/?godam-video=demo-godam-video-post (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

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.

2 participants