Fix: Handle MariaDB sandbox mode during db import #301
Conversation
Detects the sandbox mode directive in the SQL file and switches to a piped import to avoid MariaDB syntax errors.
Summary of ChangesHello @Evgenii-Zinner, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where database imports would fail when using MariaDB 10.11+ due to a specific 'sandbox mode' directive in the dump file. It introduces a more robust and memory-efficient method that detects this directive and dynamically switches to a piped streaming approach, ensuring successful imports while preserving existing optimization options and enhancing execution safety. This change significantly improves compatibility with newer MariaDB versions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a clever solution to handle MariaDB's sandbox mode directive during database imports by piping the SQL file and skipping the first line. The approach is memory-efficient and appears secure. The new functional test is a great addition. I've found a few critical issues in the implementation that need to be addressed for this feature to work correctly. Specifically, the detection string for the sandbox mode seems incorrect, and the way the external command is executed will lead to a fatal error. I've also included some suggestions to improve error reporting and code structure.
| // Check for MariaDB sandbox mode directive in the first line. | ||
| // This directive can interfere with standard imports by enabling a sandbox mode | ||
| // that restricts certain operations. We skip it by piping the file content | ||
| // and skipping the first line. | ||
| $fp = fopen( $result_file, 'r' ); | ||
| if ( ! $fp ) { | ||
| WP_CLI::error( sprintf( 'Unable to read import file: %s', $result_file ) ); | ||
| } | ||
| $first_line = fgets( $fp ); | ||
| fclose( $fp ); | ||
|
|
||
| if ( 0 === strpos( $first_line, '/*!999999\- enable the sandbox mode */' ) ) { | ||
| WP_CLI::log( 'MariaDB sandbox mode directive detected. Skipping it by piping the file content.' ); | ||
|
|
||
| $preamble = $this->get_sql_mode_query( $assoc_args ) . "\n"; | ||
| if ( ! Utils\get_flag_value( $assoc_args, 'skip-optimization' ) ) { | ||
| $preamble .= "SET autocommit = 0; SET unique_checks = 0; SET foreign_key_checks = 0;\n"; | ||
| } | ||
|
|
||
| $postamble = Utils\get_flag_value( $assoc_args, 'skip-optimization' ) ? '' : "\nCOMMIT;\n"; | ||
|
|
||
| // Use a shell pipeline to skip the first line and wrap the rest in transaction/optimizations. | ||
| $command = sprintf( | ||
| 'sh -c \'p="$1"; f="$2"; s="$3"; shift 3; ( printf "%%s" "$p"; tail -n +2 "$f"; printf "%%s" "$s" ) | %s %s --no-auto-rehash "$@"\' sh %s %s %s', | ||
| $this->get_mysql_command(), | ||
| $this->get_defaults_flag_string( $assoc_args ), | ||
| escapeshellarg( $preamble ), | ||
| escapeshellarg( $result_file ), | ||
| escapeshellarg( $postamble ) | ||
| ); | ||
|
|
||
| // Ensure we don't pass 'execute' which would conflict with STDIN. | ||
| unset( $mysql_args['execute'] ); | ||
|
|
||
| $result = self::run( $command, $mysql_args ); | ||
|
|
||
| if ( 0 === $result['exit_code'] ) { | ||
| WP_CLI::success( sprintf( "Imported from '%s'.", $result_file ) ); | ||
| } else { | ||
| WP_CLI::error( sprintf( "Failed to import from '%s'.", $result_file ) ); | ||
| } | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
This new logic for handling MariaDB sandbox dumps adds a significant amount of code to the import() method. To improve readability and maintainability, consider extracting this block into a new private helper method, for example handle_mariadb_sandbox_import(). The import() method would then just contain the detection logic and a call to this new method.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Evgenii-Zinner
force-pushed
the
fix/mariadb-sandbox-import
branch
from
0cf0f4b to
3e46997
Compare
MariaDB 10.11+ can output the sandbox header as /*M!999999\- enable the sandbox mode */. By checking for '999999' and 'sandbox mode' separately, we ensure compatibility across different MariaDB versions and escaping styles.
Evgenii-Zinner
force-pushed
the
fix/mariadb-sandbox-import
branch
from
3e46997 to
10e19a2
Compare
1 participant
Description
This PR addresses the MariaDB sandbox mode import failure (ref: #258) and provides a more robust, memory-efficient alternative to #294.
The Problem
Starting with MariaDB 10.5.24+, the
mariadb-dumputility often prepends the following directive to exports:/*!999999\- enable the sandbox mode */When WP-CLI attempts to import these files using the standard
SOURCEcommand via the--executeflag, MariaDB treats this specific comment as a syntax error or a restricted operation, causing the import process to terminate immediately.The Solution: Streamed Piping
Instead of using the MySQL
SOURCEcommand, this PR detects the sandbox directive within the first line and dynamically switches to a piped stream execution.Technical Highlights:
tail -n +2), we bypass the need to load potentially massive SQL files into PHP memory viafile_get_contents().sh -cwith positional parameters ($1,$2, etc.) to pass the preamble, file path, and postamble. This architecture avoids nested shell quoting complexities and mitigates command injection risks.--skip-optimizationby conditionally wrapping the stream in transaction controls:executeargument when piping to ensure themysqlclient correctly listens to the STDIN stream rather than ignoring it.Key Changes
(printf preamble; tail -n +2 file; printf postamble) | mysqlTesting
I have added a new functional test to
features/db-import.featureto cover this specific MariaDB edge case.New Test Scenario:
The test verifies that when an import file contains the sandbox directive on the first line:
Related Issues