Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 48 additions & 27 deletions includes/Ajax/Upload_Ajax.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,18 @@
$guest_post = true;
}
// check if the request coming from weForms & allow users to upload when require login option is disabled
if ( isset( $form_settings['require_login'] ) && $form_settings['require_login'] == 'false' ) {

Check warning on line 52 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Loose comparisons are not allowed. Expected: "==="; Found: "=="
$guest_post = true;
}
//if it is registration form, let the user upload the file
if ( get_post_type( $form_id ) == 'wpuf_profile' ) {

Check warning on line 56 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Loose comparisons are not allowed. Expected: "==="; Found: "=="
$guest_post = true;
}
if ( ! $guest_post ) {
die( 'error' );
}
}
$wpuf_file = isset( $_FILES['wpuf_file'] ) ? $_FILES['wpuf_file'] : []; // WPCS: sanitization ok.

Check failure on line 63 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Detected usage of a non-sanitized input variable: $_FILES['wpuf_file']
$file_name = pathinfo( $wpuf_file['name'], PATHINFO_FILENAME );
$file_extension = pathinfo( $wpuf_file['name'], PATHINFO_EXTENSION );
$upload = [
Expand Down Expand Up @@ -97,7 +97,7 @@
* @param string $field_type
*/
$image_type = apply_filters( 'wpuf_upload_response_image_type', $image_type, $form_id, $field_type );
if ( $image_type == 'link' ) {

Check warning on line 100 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Loose comparisons are not allowed. Expected: "==="; Found: "=="
$response['html'] = wp_get_attachment_link( $attach['attach_id'], $image_size );
} else {
$response['html'] = wp_get_attachment_image( $attach['attach_id'], $image_size );
Expand Down Expand Up @@ -147,24 +147,19 @@
/**
* Generic function to upload a file
*
* @param string $field_name file input field name
* @param array $upload_data file upload data
*
* @return bool|int attachment id on success, bool false instead
* @return array attachment result with success status and attach_id
*/
public function handle_upload( $upload_data ) {
$check_duplicate = $this->duplicate_upload( $upload_data );
if ( isset( $check_duplicate['duplicate'] ) && $check_duplicate['duplicate'] ) {
return [
'success' => true,
'attach_id' => $check_duplicate['duplicate'],
];
}
// Always make filenames unique to prevent conflicts between users
$upload_data['name'] = $this->wpuf_filename_unique( $upload_data['name'] );

Check failure on line 157 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Whitespace found at end of line
Comment on lines +155 to +157
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Filename uniqueness strategy deviates from PR goal; make every upload globally unique with a deterministic suffix

The current approach prefixes the base name with a user token (u{user_id} or guest-...), then relies on WordPress’ -1, -2… behavior for repeat uploads by the same user. The PR objective and summary specify a suffix pattern like original-wpuf-timestamp-uniqueid.ext to guarantee uniqueness per upload and avoid any reuse scenarios. Also, using only the last 4 chars of uniqid() for guests weakens entropy and increases collision risk under load.

Recommend switching to a consistent suffix strategy for both logged-in users and guests:

  • Base name stays intact.
  • Append “-wpuf-{UTC-timestamp}-{strong-unique-id}”.
  • Keep the filter to allow overrides.

This aligns tightly with the objective, avoids reintroducing filename collisions, and makes troubleshooting easier (“wpuf” token is visible).

🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[failure] 157-157:
Whitespace found at end of line

🤖 Prompt for AI Agents
In includes/Ajax/Upload_Ajax.php around lines 155-157, the current
wpuf_filename_unique strategy prefixes the basename with a user token and uses
fragile uniqid truncation for guests; change it to append a deterministic suffix
so every upload is globally unique: leave the original base name intact, append
"-wpuf-{UTC-timestamp}-{strong-unique-id}" before the extension for both
logged-in users and guests (use a stronger unique id source such as full
uniqid(true) or a hex from random_bytes for higher entropy), keep the existing
filter hook so callers can override the final filename, and ensure the result is
sanitized and respects filesystem length limits.

$uploaded_file = wp_handle_upload( $upload_data, [ 'test_form' => false ] );
// If the wp_handle_upload call returned a local path for the image
if ( isset( $uploaded_file['file'] ) ) {
$file_loc = $uploaded_file['file'];
$file_name = basename( $upload_data['name'] );
$upload_hash = md5( $upload_data['name'] . $upload_data['size'] );
$file_name = basename( $uploaded_file['file'] );
$file_type = wp_check_filetype( $file_name );
$attachment = [
'post_mime_type' => $file_type['type'],
Expand All @@ -175,7 +170,6 @@
$attach_id = wp_insert_attachment( $attachment, $file_loc );
$attach_data = wp_generate_attachment_metadata( $attach_id, $file_loc );
wp_update_attachment_metadata( $attach_id, $attach_data );
update_post_meta( $attach_id, 'wpuf_file_hash', $upload_hash );

return [
'success' => true,
Expand All @@ -189,9 +183,9 @@
];
}

public static function attach_html( $attach_id, $type = NULL, $form_id = NULL ) {

Check failure on line 186 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

TRUE, FALSE and NULL must be lowercase; expected "null" but found "NULL"

Check failure on line 186 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

TRUE, FALSE and NULL must be lowercase; expected "null" but found "NULL"
if ( ! $type ) {
$type = isset( $_GET['type'] ) ? sanitize_text_field( wp_unslash( $_GET['type'] ) ) : 'image';

Check warning on line 188 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Processing form data without nonce verification.

Check warning on line 188 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Processing form data without nonce verification.
}
$attachment = get_post( $attach_id );
if ( ! $attachment ) {
Expand Down Expand Up @@ -229,7 +223,7 @@
'<div class="attachment-name"><img src="%s" alt="%s" class="%s" /></div>', $image,
esc_attr( $attachment->post_title ), esc_attr( $attachment_class_names )
);
if ( wpuf_get_option( 'image_caption', 'wpuf_frontend_posting', 'off' ) == 'on' ) {

Check warning on line 226 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Loose comparisons are not allowed. Expected: "==="; Found: "=="
$html .= '<div class="wpuf-file-input-wrap">';
$html .= sprintf(
'<input type="text" name="wpuf_files_data[%d][title]" value="%s" placeholder="%s">', $attach_id,
Expand Down Expand Up @@ -274,9 +268,9 @@
}

// post author or editor role
if ( get_current_user_id() == absint( $attachment->post_author ) || current_user_can(

Check warning on line 271 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Loose comparisons are not allowed. Expected: "==="; Found: "=="
'delete_private_pages'

Check failure on line 272 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Multi-line function call not indented correctly; expected 12 spaces but found 16
) ) {

Check failure on line 273 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Multi-line function call not indented correctly; expected 8 spaces but found 12
$deleted = wp_delete_attachment( $attachment_id, true );
if ( $deleted ) {
wp_send_json_success( [ 'message' => __( 'Attachment deleted successfully.', 'wp-user-frontend' ) ] );
Expand All @@ -300,24 +294,51 @@
}

/**
* Check if duplicate file
* Make filename unique by adding user ID prefix to prevent conflicts between users
* while still allowing WordPress to handle duplicates for the same user
*
* @param array $file
* @since WPUF_SINCE
*

Check failure on line 301 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Whitespace found at end of line
* @param string $filename
*
* @return mixed
* @return string
*/
function duplicate_upload( $file ) {
global $wpdb;
$upload_hash = md5( $file['name'] . $file['size'] );

$match = $wpdb->get_var( $wpdb->prepare(
"SELECT post_id FROM $wpdb->postmeta m JOIN $wpdb->posts p ON p.ID = m.post_id WHERE m.meta_key = 'wpuf_file_hash' AND m.meta_value = %s AND p.post_status != 'trash' LIMIT 1;",
$upload_hash
) );
if ( $match ) {
$file['duplicate'] = $match;
private function wpuf_filename_unique( $filename ) {
$info = pathinfo( $filename );
$ext = empty( $info['extension'] ) ? '' : '.' . $info['extension'];
$name = basename( $filename, $ext );

Check failure on line 310 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Whitespace found at end of line
// Sanitize the base name
$name = sanitize_file_name( $name );

Check failure on line 313 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Whitespace found at end of line
// Get current user ID for user isolation
$user_id = get_current_user_id();

Check failure on line 316 in includes/Ajax/Upload_Ajax.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Whitespace found at end of line
// For logged-in users, add user ID prefix to prevent cross-user conflicts
// For guests, add a session-based or timestamp prefix
if ( $user_id > 0 ) {
// Add user ID prefix to isolate files between users
// Format: u123-filename.ext (WordPress will handle duplicates as u123-filename-1.ext)
$unique_prefix = 'u' . $user_id;
} else {
// For guest uploads, use timestamp to ensure uniqueness
// This prevents guests from overwriting each other's files
$unique_prefix = 'guest-' . time() . '-' . substr( uniqid(), -4 );
}

return $file;

// Combine prefix with filename
// This ensures user isolation while preserving WordPress duplicate handling
$new_filename = $unique_prefix . '-' . $name . $ext;

// Apply filter to allow customization of the unique filename
$new_filename = apply_filters( 'wpuf_upload_file_name', $new_filename, [
'original_name' => $filename,
'base_name' => $name,
'extension' => $ext,
'user_id' => $user_id,
'unique_prefix' => $unique_prefix,
] );

return $new_filename;
}
Comment on lines 296 to 343
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Implement a robust, suffix-based naming scheme with higher entropy; document the filter and update @SInCE

Refactor wpuf_filename_unique() to:

  • Always append a “-wpuf-{UTC timestamp}-{unique id}” suffix to the sanitized base name (not a prefix).
  • Use wp_generate_uuid4() where available (fallback to a longer uniqid with entropy) to improve uniqueness.
  • Keep passing context to the filter, but rename the key from unique_prefix to unique_suffix (clearer and matches the new strategy).
  • Update the docblock to reflect the actual behavior and set a concrete @SInCE version.
  • Remove trailing whitespace flagged by PHPCS.

Proposed diff:

-    /**
-     * Make filename unique by adding user ID prefix to prevent conflicts between users
-     * while still allowing WordPress to handle duplicates for the same user
-     *
-     * @since WPUF_SINCE
-     * 
-     * @param string $filename
-     *
-     * @return string
-     */
-    private function wpuf_filename_unique( $filename ) {
-        $info = pathinfo( $filename );
-        $ext  = empty( $info['extension'] ) ? '' : '.' . $info['extension'];
-        $name = basename( $filename, $ext );
-        
-        // Sanitize the base name
-        $name = sanitize_file_name( $name );
-        
-        // Get current user ID for user isolation
-        $user_id = get_current_user_id();
-        
-        // For logged-in users, add user ID prefix to prevent cross-user conflicts
-        // For guests, add a session-based or timestamp prefix
-        if ( $user_id > 0 ) {
-            // Add user ID prefix to isolate files between users
-            // Format: u123-filename.ext (WordPress will handle duplicates as u123-filename-1.ext)
-            $unique_prefix = 'u' . $user_id;
-        } else {
-            // For guest uploads, use timestamp to ensure uniqueness
-            // This prevents guests from overwriting each other's files
-            $unique_prefix = 'guest-' . time() . '-' . substr( uniqid(), -4 );
-        }
-        
-        // Combine prefix with filename
-        // This ensures user isolation while preserving WordPress duplicate handling
-        $new_filename = $unique_prefix . '-' . $name . $ext;
-        
-        // Apply filter to allow customization of the unique filename
-        $new_filename = apply_filters( 'wpuf_upload_file_name', $new_filename, [
-            'original_name' => $filename,
-            'base_name'     => $name,
-            'extension'     => $ext,
-            'user_id'       => $user_id,
-            'unique_prefix' => $unique_prefix,
-        ] );
-        
-        return $new_filename;
-    }
+    /**
+     * Generate a globally-unique filename by appending a deterministic suffix.
+     *
+     * Pattern: {basename}-wpuf-{UTC YmdHis}-{uniqueid}{.ext}
+     *
+     * @since 3.7.10
+     *
+     * @param string $filename Original filename (may include extension).
+     * @return string Unique filename with suffix applied.
+     */
+    private function wpuf_filename_unique( $filename ) {
+        $info = pathinfo( $filename );
+        $ext  = empty( $info['extension'] ) ? '' : '.' . strtolower( $info['extension'] );
+        $name = sanitize_file_name( wp_basename( $filename, $ext ) );
+
+        // Always use UTC to avoid timezone-dependent strings
+        $timestamp = gmdate( 'YmdHis' );
+
+        // Strong unique id: prefer UUIDv4; fall back to high-entropy uniqid
+        if ( function_exists( 'wp_generate_uuid4' ) ) {
+            $unique_id = substr( wp_generate_uuid4(), 0, 8 );
+        } else {
+            $unique_id = substr( uniqid( '', true ), -10 );
+        }
+
+        $unique_suffix = 'wpuf-' . $timestamp . '-' . $unique_id;
+        $new_filename  = sprintf( '%s-%s%s', $name, $unique_suffix, $ext );
+
+        // Filter to allow customization of the filename strategy.
+        // Context keys intentionally documented for developers extending this behavior.
+        $new_filename = apply_filters(
+            'wpuf_upload_file_name',
+            $new_filename,
+            [
+                'original_name' => $filename,
+                'base_name'     => $name,
+                'extension'     => $ext,
+                'unique_suffix' => $unique_suffix,
+            ]
+        );
+
+        return $new_filename;
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Check if duplicate file
* Make filename unique by adding user ID prefix to prevent conflicts between users
* while still allowing WordPress to handle duplicates for the same user
*
* @param array $file
* @since WPUF_SINCE
*
* @param string $filename
*
* @return mixed
* @return string
*/
function duplicate_upload( $file ) {
global $wpdb;
$upload_hash = md5( $file['name'] . $file['size'] );
$match = $wpdb->get_var( $wpdb->prepare(
"SELECT post_id FROM $wpdb->postmeta m JOIN $wpdb->posts p ON p.ID = m.post_id WHERE m.meta_key = 'wpuf_file_hash' AND m.meta_value = %s AND p.post_status != 'trash' LIMIT 1;",
$upload_hash
) );
if ( $match ) {
$file['duplicate'] = $match;
private function wpuf_filename_unique( $filename ) {
$info = pathinfo( $filename );
$ext = empty( $info['extension'] ) ? '' : '.' . $info['extension'];
$name = basename( $filename, $ext );
// Sanitize the base name
$name = sanitize_file_name( $name );
// Get current user ID for user isolation
$user_id = get_current_user_id();
// For logged-in users, add user ID prefix to prevent cross-user conflicts
// For guests, add a session-based or timestamp prefix
if ( $user_id > 0 ) {
// Add user ID prefix to isolate files between users
// Format: u123-filename.ext (WordPress will handle duplicates as u123-filename-1.ext)
$unique_prefix = 'u' . $user_id;
} else {
// For guest uploads, use timestamp to ensure uniqueness
// This prevents guests from overwriting each other's files
$unique_prefix = 'guest-' . time() . '-' . substr( uniqid(), -4 );
}
return $file;
// Combine prefix with filename
// This ensures user isolation while preserving WordPress duplicate handling
$new_filename = $unique_prefix . '-' . $name . $ext;
// Apply filter to allow customization of the unique filename
$new_filename = apply_filters( 'wpuf_upload_file_name', $new_filename, [
'original_name' => $filename,
'base_name' => $name,
'extension' => $ext,
'user_id' => $user_id,
'unique_prefix' => $unique_prefix,
] );
return $new_filename;
}
/**
* Generate a globally-unique filename by appending a deterministic suffix.
*
* Pattern: {basename}-wpuf-{UTC YmdHis}-{uniqueid}{.ext}
*
* @since 3.7.10
*
* @param string $filename Original filename (may include extension).
* @return string Unique filename with suffix applied.
*/
private function wpuf_filename_unique( $filename ) {
$info = pathinfo( $filename );
$ext = empty( $info['extension'] ) ? '' : '.' . strtolower( $info['extension'] );
$name = sanitize_file_name( wp_basename( $filename, $ext ) );
// Always use UTC to avoid timezone-dependent strings
$timestamp = gmdate( 'YmdHis' );
// Strong unique id: prefer UUIDv4; fall back to high-entropy uniqid
if ( function_exists( 'wp_generate_uuid4' ) ) {
$unique_id = substr( wp_generate_uuid4(), 0, 8 );
} else {
$unique_id = substr( uniqid( '', true ), -10 );
}
$unique_suffix = 'wpuf-' . $timestamp . '-' . $unique_id;
$new_filename = sprintf( '%s-%s%s', $name, $unique_suffix, $ext );
// Filter to allow customization of the filename strategy.
// Context keys intentionally documented for developers extending this behavior.
$new_filename = apply_filters(
'wpuf_upload_file_name',
$new_filename,
[
'original_name' => $filename,
'base_name' => $name,
'extension' => $ext,
'unique_suffix' => $unique_suffix,
]
);
return $new_filename;
}
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[failure] 316-316:
Whitespace found at end of line


[failure] 313-313:
Whitespace found at end of line


[failure] 310-310:
Whitespace found at end of line


[failure] 301-301:
Whitespace found at end of line

🤖 Prompt for AI Agents
In includes/Ajax/Upload_Ajax.php around lines 296 to 343, update
wpuf_filename_unique() to append (not prepend) a suffix of the form -wpuf-{UTC
timestamp}-{unique id} to the sanitized base name, use wp_generate_uuid4() when
available and otherwise fall back to a longer uniqid with entropy, rename the
context key passed to apply_filters from unique_prefix to unique_suffix
(containing the generated suffix), keep the rest of the context (original_name,
base_name, extension, user_id), update the method docblock to describe the
suffix-based behavior and set a concrete @since version, and remove any trailing
whitespace flagged by PHPCS; ensure the final returned filename is base + suffix
+ extension and that the filter allows overriding the full filename.

}
Loading