From bbf40146fb9be392e02a9d59f733604839a1284b Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 23 Feb 2014 10:59:59 -0800 Subject: [PATCH] Recommend STRICT_ALL_TABLES for every install, not just development installs Summary: See D8308. Enabling STRICT_ALL_TABLES prevents this entire class of error, by fataling on truncation instead of truncating. We never want truncation; it is always bad and sometimes extremely bad. We've recommended this mode for developer installs for a long time, and some users run with it enabled, so it's very unlikely to cause any issues (I've had it enabled locally for at least 6-8 months, I think). Test Plan: - Disabled mode. - Saw warning. - Enabled mode. - No warning. {F117040} Reviewers: btrahan, chad Reviewed By: chad CC: chad, aran, arice Differential Revision: https://secure.phabricator.com/D8309 --- .../check/PhabricatorSetupCheckMySQL.php | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/src/applications/config/check/PhabricatorSetupCheckMySQL.php b/src/applications/config/check/PhabricatorSetupCheckMySQL.php index a4cefc46b0..9b4c4581a6 100644 --- a/src/applications/config/check/PhabricatorSetupCheckMySQL.php +++ b/src/applications/config/check/PhabricatorSetupCheckMySQL.php @@ -25,30 +25,39 @@ final class PhabricatorSetupCheckMySQL extends PhabricatorSetupCheck { ->setMessage($message); } - if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { - $mode_string = queryfx_one($conn_raw, "SELECT @@sql_mode"); - $modes = explode(',', $mode_string['@@sql_mode']); - if (!in_array('STRICT_ALL_TABLES', $modes)) { - $summary = pht( - "MySQL is not in strict mode, but should be for Phabricator ". - "development."); + $mode_string = queryfx_one($conn_raw, "SELECT @@sql_mode"); + $modes = explode(',', $mode_string['@@sql_mode']); + if (!in_array('STRICT_ALL_TABLES', $modes)) { + $summary = pht( + "MySQL is not in strict mode, but using strict mode is strongly ". + "encouraged."); - $message = pht( - "This install is in developer mode, but the global sql_mode is not ". - "set to 'STRICT_ALL_TABLES'. It is recommended that you set this ". - "mode while developing Phabricator. Strict mode will promote some ". - "query warnings to errors, and ensure you don't miss them during ". - "development. You can find more information about this mode (and ". - "how to configure it) in the MySQL manual."); + $message = pht( + "On your MySQL instance, the global sql_mode is not set to ". + "'STRICT_ALL_TABLES'. It is strongly encouraged that you enable this ". + "mode when running Phabricator.\n\n". + "By default, MySQL will fail silently and continue when certain ". + "error conditions occur. Sometimes contuining does the wrong thing. ". + "For example, inserting too much data into a column will cause ". + "silent truncation (and thus data loss) instead of failing in an ". + "obvious way that we can fix. These behaviors can also create ". + "security risks. Enabling strict mode raises an explicit error ". + "instead and prevents this entire class of problem from doing any ". + "damage.\n\n". + "You can find more information about this mode (and how to configure ". + "it) in the MySQL manual. Usually, it is sufficient to add this to ". + "your 'my.cnf' file:\n\n". + "%s\n". + "(Note that if you run other applications against the same database, ". + "they may not work in strict mode. Be careful about enabling it in ". + "these cases.)", + phutil_tag('pre', array(), 'sql-mode=STRICT_ALL_TABLES')); - $this->newIssue('mysql.mode') - ->setName(pht('MySQL STRICT_ALL_TABLES Mode Not Set')) - ->addRelatedPhabricatorConfig('phabricator.developer-mode') - ->setSummary($summary) - ->setMessage($message); - } + $this->newIssue('mysql.mode') + ->setName(pht('MySQL STRICT_ALL_TABLES Mode Not Set')) + ->setSummary($summary) + ->setMessage($message); } - } }