diff --git a/safe-svg.php b/safe-svg.php index 38b86c15..30b17278 100644 --- a/safe-svg.php +++ b/safe-svg.php @@ -128,11 +128,16 @@ public function __construct() { $this->sanitizer = new Sanitizer(); $this->sanitizer->minify( true ); + // Allow SVG uploads from specific contexts. + add_action( 'load-upload.php', array( $this, 'allow_svg_from_upload' ) ); + add_action( 'load-post-new.php', array( $this, 'allow_svg_from_upload' ) ); + add_action( 'load-post.php', array( $this, 'allow_svg_from_upload' ) ); + add_action( 'load-site-editor.php', array( $this, 'allow_svg_from_upload' ) ); + + // Init all the things. add_action( 'init', array( $this, 'setup_blocks' ) ); - add_filter( 'upload_mimes', array( $this, 'allow_svg' ) ); add_filter( 'wp_handle_sideload_prefilter', array( $this, 'check_for_svg' ) ); add_filter( 'wp_handle_upload_prefilter', array( $this, 'check_for_svg' ) ); - add_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_svg' ), 75, 4 ); add_filter( 'wp_prepare_attachment_for_js', array( $this, 'fix_admin_preview' ), 10, 3 ); add_filter( 'wp_get_attachment_image_src', array( $this, 'one_pixel_fix' ), 10, 4 ); add_filter( 'admin_post_thumbnail_html', array( $this, 'featured_image_fix' ), 10, 3 ); @@ -145,6 +150,16 @@ public function __construct() { new safe_svg_settings(); } + /** + * Allow SVG uploads from the wp-admin/upload.php screen. Without this, you cannot upload SVGs from the media grid view. + * + * @return void + */ + public function allow_svg_from_upload() { + add_filter( 'upload_mimes', array( $this, 'allow_svg' ) ); + add_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_svg' ), 75, 4 ); + } + /** * Custom function to check if user can upload svg. * @@ -242,7 +257,18 @@ public function check_for_svg( $file ) { } $file_name = isset( $file['name'] ) ? $file['name'] : ''; + + // Allow SVGs to be uploaded when this function runs. + add_filter( 'upload_mimes', array( $this, 'allow_svg' ) ); + add_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_svg' ), 75, 4 ); + $wp_filetype = wp_check_filetype_and_ext( $file['tmp_name'], $file_name ); + + // Remove the SVG mime type after we've sanitized the file. + // We need to utilize the pre_move_uploaded_file filter to ensure we can remove the filters after the file has been full-processed. + // This is because wp_check_filetype_and_ext() is called multiple times during the upload process. + add_filter( 'pre_move_uploaded_file', array( $this, 'pre_move_uploaded_file' ) ); + $type = ! empty( $wp_filetype['type'] ) ? $wp_filetype['type'] : ''; if ( 'image/svg+xml' === $type ) { @@ -266,6 +292,23 @@ public function check_for_svg( $file ) { return $file; } + /** + * Remove the filters after the file has been processed. + * + * We need to utilize the pre_move_uploaded_file filter to ensure we can remove the filters after the file has been full-processed. + * This is because wp_check_filetype_and_ext() is called multiple times during the upload process. + * + * @param string $move_new_file The new file path. We don't touch this, just return it. + * + * @return string + */ + public function pre_move_uploaded_file( $move_new_file ) { + remove_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_svg' ), 75 ); + remove_filter( 'upload_mimes', array( $this, 'allow_svg' ) ); + + return $move_new_file; + } + /** * Sanitize the SVG * diff --git a/tests/cypress/e2e/safe-svg.cy.js b/tests/cypress/e2e/safe-svg.cy.js index 7f95718f..601ff02e 100644 --- a/tests/cypress/e2e/safe-svg.cy.js +++ b/tests/cypress/e2e/safe-svg.cy.js @@ -3,12 +3,18 @@ describe('Safe SVG Tests', () => { cy.login(); }); - it('Admin can upload SVG image', () => { + it('Admin can upload SVG image via add new media file', () => { cy.uploadMedia('.wordpress-org/icon.svg'); cy.get('.media-item .media-list-title, .media-item .title').should('exist').contains('icon'); cy.get('.media-item a.edit-attachment').should('exist').contains('Edit'); }); + it('Admin can upload SVG image via the media grid', () => { + cy.uploadMediaThroughGrid('.wordpress-org/icon.svg').then((attachmentId) => { + cy.get(`.attachments .attachment[data-id="${attachmentId}"]`).should('exist'); + }); + }); + /** * Flow for verify SVG sanitization. * diff --git a/tests/cypress/support/commands.js b/tests/cypress/support/commands.js index 5b5692aa..15959ae2 100644 --- a/tests/cypress/support/commands.js +++ b/tests/cypress/support/commands.js @@ -29,4 +29,18 @@ Cypress.Commands.add('uploadMedia', (filePath) => { cy.get('.drag-drop').should('exist'); cy.get('#drag-drop-area').should('exist'); cy.get('#drag-drop-area').selectFile(filePath, { action: 'drag-drop' }); -}); \ No newline at end of file +}); + +Cypress.Commands.add('uploadMediaThroughGrid', (filePath) => { + cy.visit('/wp-admin/upload.php?mode=grid'); + cy.get('.supports-drag-drop').should('exist'); + cy.get('.uploader-window').should('exist'); + // Intercept the upload request + cy.intercept('POST', '/wp-admin/async-upload.php').as('upload'); + cy.get('.supports-drag-drop').selectFile(filePath, { action: 'drag-drop', force: true, waitForAnimations: true }); + return cy.wait('@upload') + .then(({ request, response }) => { + cy.get('.uploader-window').trigger('dropzone:leave'); + return cy.wrap(response.headers['x-wp-upload-attachment-id'] ?? 0); + }) +}); diff --git a/tests/unit/bootstrap.php b/tests/unit/bootstrap.php index 24bf368f..703ac9cd 100644 --- a/tests/unit/bootstrap.php +++ b/tests/unit/bootstrap.php @@ -15,5 +15,6 @@ WP_Mock::bootstrap(); \WP_Mock::userFunction( 'plugin_dir_url' ); +\WP_Mock::userFunction( 'remove_filter' ); require TEST_PLUGIN_DIR . '/safe-svg.php'; diff --git a/tests/unit/test-safe-svg.php b/tests/unit/test-safe-svg.php index 061c175b..dd4881a6 100644 --- a/tests/unit/test-safe-svg.php +++ b/tests/unit/test-safe-svg.php @@ -41,9 +41,9 @@ public function tearDown(): void { * Test constructor. */ public function test_constructor() { - \WP_Mock::expectFilterAdded( 'upload_mimes', array( $this->instance, 'allow_svg' ) ); + \WP_Mock::expectActionAdded( 'load-upload.php', array( $this->instance, 'allow_svg_from_upload' ) ); \WP_Mock::expectFilterAdded( 'wp_handle_upload_prefilter', array( $this->instance, 'check_for_svg' ) ); - \WP_Mock::expectFilterAdded( 'wp_check_filetype_and_ext', array( $this->instance, 'fix_mime_type_svg' ), 75, 4 ); + \WP_Mock::expectFilterAdded( 'wp_handle_sideload_prefilter', array( $this->instance, 'check_for_svg' ) ); \WP_Mock::expectFilterAdded( 'wp_prepare_attachment_for_js', array( $this->instance, 'fix_admin_preview' ), 10, 3 ); \WP_Mock::expectFilterAdded( 'wp_get_attachment_image_src', array( $this->instance, 'one_pixel_fix' ), 10, 4 ); \WP_Mock::expectFilterAdded( 'admin_post_thumbnail_html', array( $this->instance, 'featured_image_fix' ), 10, 3 ); @@ -159,6 +159,10 @@ public function test_check_for_svg() { 'name' => 'svgTestOne.svg', ); + \WP_Mock::expectFilterAdded( 'upload_mimes', array( $this->instance, 'allow_svg' ) ); + \WP_Mock::expectFilterAdded( 'wp_check_filetype_and_ext', array( $this->instance, 'fix_mime_type_svg' ), 75, 4 ); + \WP_Mock::expectFilterAdded( 'pre_move_uploaded_file', array( $this->instance, 'pre_move_uploaded_file' ) ); + $this->instance->check_for_svg( $file ); // @phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents