-
Notifications
You must be signed in to change notification settings - Fork 154
fix: file upload field allowed extension #1764
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
fix: file upload field allowed extension #1764
Conversation
WalkthroughFile extension comparisons are normalized to lowercase using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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: 0
🧹 Nitpick comments (1)
wpuf-functions.php (1)
1037-1056: Good fix for case-insensitive extension matching.The normalization logic correctly handles file extensions regardless of case:
- Converts the file extension to lowercase (line 1037)
- Normalizes allowed extensions to lowercase and trims whitespace (lines 1042-1049)
- Uses strict comparison with normalized values (line 1054)
This will properly recognize files like
.MP3,.Mp3, or.mp3as playable media.Optional performance optimization:
Consider moving the extension normalization (lines 1041-1052) outside the
foreachloop at line 1027, since the same processing repeats for every attachment:foreach ( $field_value as $attachment_id ) { + if ( ! isset( $allowed_extensions ) ) { + $wpuf_allowed_extensions = wpuf_allowed_extensions(); + $allowed_audio_extensions = array_map( + 'strtolower', + array_map( 'trim', explode( ',', $wpuf_allowed_extensions['audio']['ext'] ) ) + ); + $allowed_video_extensions = array_map( + 'strtolower', + array_map( 'trim', explode( ',', $wpuf_allowed_extensions['video']['ext'] ) ) + ); + $allowed_extensions = array_merge( + $allowed_audio_extensions, $allowed_video_extensions + ); + } if ( 'image_upload' === $attr['input_type'] ) { $image_size = wpuf_get_option( 'insert_photo_size', 'wpuf_frontend_posting', 'thumbnail' ); $thumb = wp_get_attachment_image( $attachment_id, $image_size ); } else { $thumb = get_post_field( 'post_title', $attachment_id ); } $full_size = wp_get_attachment_url( $attachment_id ); $path = parse_url( $full_size, PHP_URL_PATH ); $extension = strtolower( pathinfo( $path, PATHINFO_EXTENSION ) ); if ( $thumb ) { $playable = isset( $attr['playable_audio_video'] ) ? $attr['playable_audio_video'] : 'no'; - $wpuf_allowed_extensions = wpuf_allowed_extensions(); - $allowed_audio_extensions = array_map( - 'strtolower', - array_map( 'trim', explode( ',', $wpuf_allowed_extensions['audio']['ext'] ) ) - ); - $allowed_video_extensions = array_map( - 'strtolower', - array_map( 'trim', explode( ',', $wpuf_allowed_extensions['video']['ext'] ) ) - ); - $allowed_extensions = array_merge( - $allowed_audio_extensions, $allowed_video_extensions - ); if ( 'yes' === $playable && in_array( $extension, $allowed_extensions, true ) ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wpuf-functions.php(4 hunks)
🧰 Additional context used
🪛 GitHub Actions: Inspections
wpuf-functions.php
[warning] 1-1: The method parameter $post_id is never used
[error] 1-1: Processing form data without nonce verification.
🔇 Additional comments (2)
wpuf-functions.php (2)
531-568: LGTM: Formatting cleanup of extension lists.The trailing comma removals in the extension strings are purely cosmetic changes with no functional impact.
4076-4087: LGTM: Array dependency support added with backward compatibility.The implementation correctly handles both array and string dependencies:
- Array dependencies are JSON-encoded and placed in
data-depends-on(line 4081)- String dependencies retain existing behavior (line 4084)
- Both are properly escaped with
esc_attr()- Single quotes in the HTML attribute (line 4087) safely contain JSON with double quotes
This maintains backward compatibility while adding new functionality.
file upload field allowed extension
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.