Skip to content

Conversation

@subodhr258
Copy link
Collaborator

Issue - #1230

This pull request improves input validation and user guidance for setting the duration of a hotspot layer in the video editor. The main changes ensure that the duration is safely parsed, validated, and clearly communicated to the user, with a new maximum limit.

Input validation and user experience improvements:

  • Added a parseSafeDuration function to sanitize and validate the duration input, ensuring it is a number between 1 and 36,000 seconds (10 hours), and handling invalid inputs gracefully.
  • Updated the duration input field to use the new validation function, set the maximum allowed value to 36,000 seconds, and improved the help text to clearly state the maximum duration limit for hotspot layers.

Recording:

Screen.Recording.2025-12-31.at.5.57.54.PM.mov

Copy link
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 fixes a critical input validation bug in the video editor where entering extremely large values for hotspot layer duration caused integer overflow issues. The fix introduces proper validation to ensure duration values are safely bounded between 1 and 36,000 seconds (10 hours).

  • Added parseSafeDuration function that sanitizes input, validates numeric values, and enforces min/max bounds
  • Updated the duration input field to include a maximum attribute and improved help text that clearly communicates the 10-hour limit
  • Replaces unsafe parseInt() usage with a robust validation approach that handles edge cases like infinity, NaN, and overflow

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

* Safely parse and validate duration value.
*
* @param {string|number} val - The input value to parse.
* @return {number} Validated duration value between 1 and 3600 seconds.
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The JSDoc comment states the return value is "between 1 and 3600 seconds", but the actual implementation uses 36000 seconds (10 hours) as the maximum. The documentation should be updated to match the implementation.

Suggested change
* @return {number} Validated duration value between 1 and 3600 seconds.
* @return {number} Validated duration value between 1 and 36000 seconds (up to 10 hours).

Copilot uses AI. Check for mistakes.
<TextControl
label={ __( 'Layer Duration (seconds)', 'godam' ) }
className="godam-input"
type="number"
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

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

The min="1" attribute was removed from the input field. While the parseSafeDuration function enforces a minimum of 1, removing this HTML attribute means the browser's native validation no longer prevents users from entering values less than 1 (like 0 or negative numbers). Consider keeping the min attribute for better user experience, as it provides immediate visual feedback in the browser UI.

Suggested change
type="number"
type="number"
min="1"

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

🔍 WordPress Plugin Check Report

❌ Status: Failed

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
29 12 17

❌ Errors (12)

📁 inc/classes/wpforms/wpforms-field-godam-record-frontend.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/wpforms/wpforms-field-godam-record-entry-view.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/wpforms/wpforms-field-godam-record-entry-edit.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/class-rewrite.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/class-elementor-widgets.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/everest-forms/everest-forms-field-godam-record-frontend.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/assets/class-ima-assets.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/assets/class-jetpack-form-assets.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/class-video-preview.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/lifter-lms/class-lifter-lms.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/fluentforms/class-form-submit.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;
📁 inc/classes/class-video-engagement.php (1 error)
📍 Line 🔖 Check 💬 Message
0 missing_direct_file_access_protection PHP file should prevent direct access. Add a check like: if ( ! defined( 'ABSPATH' ) ) exit;

⚠️ Warnings (17)

📁 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.
📁 inc/classes/media-library/class-media-folder-utils.php (1 warning)
📍 Line 🔖 Check 💬 Message
91 PluginCheck.Security.DirectDB.UnescapedDBParameter Unescaped parameter $where_clause used in $wpdb->get_var($wpdb->prepare(\n\t\t\t\t"\n\t\t\tSELECT COUNT(DISTINCT p.ID)\n\t\t\tFROM {$wpdb->posts} p\n\t\t\tINNER JOIN {$wpdb->term_relationships} tr ON p.ID = tr.object_id\n\t\t\tINNER JOIN {$wpdb->term_taxonomy} tt ON tr.term_taxonomy_id = tt.term_taxonomy_id\n\t\t\tWHERE {$where_clause}\n\t\t",\n\t\t\t\t$query_params\n\t\t\t))\n$where_clause assigned unsafely at line 82:\n $where_clause .= ' AND p.post_mime_type LIKE %s'\n$query_params[] used without escaping.\n$mime_type used without escaping.
📁 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/?p=1 (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/?page_id=2 (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/?attachment_id=6 (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/?p=1 (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/?page_id=2 (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/?attachment_id=6 (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