Skip to content
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/only allow svg when uploading #228

Merged
merged 8 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
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
47 changes: 45 additions & 2 deletions safe-svg.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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.
*
Expand Down Expand Up @@ -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 ) {
Expand All @@ -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
*
Expand Down
8 changes: 7 additions & 1 deletion tests/cypress/e2e/safe-svg.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
16 changes: 15 additions & 1 deletion tests/cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
});
});

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);
})
});
1 change: 1 addition & 0 deletions tests/unit/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
8 changes: 6 additions & 2 deletions tests/unit/test-safe-svg.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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
Expand Down
Loading