Allow quotes to be correctly handled by october:env (#4986)

This fix will apply quotes around string environment variables which contain either a single, or double, quote as well as any variables with a hash symbol - escaping any double-quotes encountered. When artisan october:env is run, this should correctly transfer all configuration values from the config files to the .env file.

Fixes #4979.

modules/system/console/OctoberEnv.php

@@ -203,7 +203,7 @@ class OctoberEnv extends Command
     private function saveEnvSettings($key, $value)
     {
         if (! $this->envKeyExists($key)) {
-            $line = sprintf("%s=%s\n", $key, $this->stripQuotes($value));
+            $line = sprintf("%s=%s\n", $key, $value);
 
             if ($this->config == 'database' && $key != 'DB_CONNECTION') {
                 $this->writeDbEnvSettings($line);
@@ -266,7 +266,11 @@ class OctoberEnv extends Command
     private function normalize($value)
     {
         if (is_string($value)) {
-            return "'$value'";
+            if (preg_match('/["\'#]/', $value)) {
+                return '"' . str_replace('"', '\\"', $value) . '"';
+            } else {
+                return $value;
+            }
         } elseif (is_bool($value)) {
             return $value ? 'true' : 'false';
         } elseif ($value === null) {
@@ -276,15 +280,6 @@ class OctoberEnv extends Command
         return $value;
     }
 
-    /**
-     * @param $string
-     * @return string
-     */
-    private function stripQuotes($string)
-    {
-        return strtr($string, ['"' => '', "'" => '']);
-    }
-
     /**
      * @param $matches
      * @return bool

tests/fixtures/config/app.php

@@ -0,0 +1,9 @@
+<?php
+// Fixture used for `october:env` unit tests in `tests/unit/system/console/OctoberEnvTest.php
+
+return [
+    'debug' => true,
+    'url' => 'https://localhost',
+    'key' => 'CHANGE_ME!!!!!!!!!!!!!!!!!!!!!!!',
+    'timezone' => 'UTC',
+];

tests/fixtures/config/cache.php

@@ -0,0 +1,6 @@
+<?php
+// Fixture used for `october:env` unit tests in `tests/unit/system/console/OctoberEnvTest.php
+
+return [
+    'default' => 'file',
+];

tests/fixtures/config/cms.php

@@ -0,0 +1,10 @@
+<?php
+// Fixture used for `october:env` unit tests in `tests/unit/system/console/OctoberEnvTest.php
+
+return [
+    'enableRoutesCache' => false,
+    'enableAssetCache' => false,
+    'databaseTemplates' => false,
+    'linkPolicy' => 'detect',
+    'enableCsrfProtection' => true,
+];

tests/fixtures/config/database.php

@@ -0,0 +1,16 @@
+<?php
+// Fixture used for `october:env` unit tests in `tests/unit/system/console/OctoberEnvTest.php
+
+return [
+    'default' => 'mysql',
+    'connections' => [
+        'mysql' => [
+            'host'       => 'localhost',
+            'port'       => 3306,
+            'database'   => 'data#base',
+            'username'   => 'teal\'c',
+            'password'   => 'test"quotes\'test',
+        ],
+    ],
+    'useConfigForTesting' => false,
+];

tests/fixtures/config/mail.php

@@ -0,0 +1,11 @@
+<?php
+// Fixture used for `october:env` unit tests in `tests/unit/system/console/OctoberEnvTest.php
+
+return [
+    'driver' => 'smtp',
+    'host' => 'smtp.mailgun.org',
+    'port' => 587,
+    'encryption' => 'tls',
+    'username' => null,
+    'password' => null,
+];

tests/fixtures/config/queue.php

@@ -0,0 +1,6 @@
+<?php
+// Fixture used for `october:env` unit tests in `tests/unit/system/console/OctoberEnvTest.php
+
+return [
+    'default' => 'sync',
+];

tests/fixtures/config/session.php

@@ -0,0 +1,6 @@
+<?php
+// Fixture used for `october:env` unit tests in `tests/unit/system/console/OctoberEnvTest.php
+
+return [
+    'driver' => 'file',
+];

tests/unit/system/console/OctoberEnvTest.php

@@ -0,0 +1,130 @@
+<?php
+
+use October\Rain\Foundation\Bootstrap\LoadConfiguration;
+use Symfony\Component\Console\Input\ArrayInput;
+use Symfony\Component\Console\Output\NullOutput;
+use System\Console\OctoberEnv;
+
+class OctoberEnvTest extends TestCase
+{
+    /** @var bool If the config fixtures have been copied */
+    public static $fixturesCopied = false;
+
+    /** @var string Stores the original config path from the app container */
+    public static $origConfigPath;
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        $this->setUpConfigFixtures();
+        $this->stubOutEnvFile();
+    }
+
+    public function testCommand()
+    {
+        $command = new OctoberEnv();
+        $command->setLaravel($this->app);
+        $command->run(new ArrayInput([]), new NullOutput);
+
+        // Check environment file
+        $envFile = file_get_contents(base_path('.env'));
+
+        // Forward compatible assertions
+        // @TODO: Use only `assertStringContainsString` after L6 upgrade
+
+        if (method_exists($this, 'assertStringContainsString')) {
+            $this->assertStringContainsString('APP_DEBUG=true', $envFile);
+            $this->assertStringContainsString('APP_URL=https://localhost', $envFile);
+            $this->assertStringContainsString('DB_CONNECTION=mysql', $envFile);
+            $this->assertStringContainsString('DB_DATABASE="data#base"', $envFile);
+            $this->assertStringContainsString('DB_USERNAME="teal\'c"', $envFile);
+            $this->assertStringContainsString('DB_PASSWORD="test\\"quotes\'test"', $envFile);
+            $this->assertStringContainsString('DB_PORT=3306', $envFile);
+        } else {
+            $this->assertContains('APP_DEBUG=true', $envFile);
+            $this->assertContains('APP_URL=https://localhost', $envFile);
+            $this->assertContains('DB_CONNECTION=mysql', $envFile);
+            $this->assertContains('DB_DATABASE="data#base"', $envFile);
+            $this->assertContains('DB_USERNAME="teal\'c"', $envFile);
+            $this->assertContains('DB_PASSWORD="test\\"quotes\'test"', $envFile);
+            $this->assertContains('DB_PORT=3306', $envFile);
+        }
+    }
+
+    protected function tearDown()
+    {
+        $this->tearDownConfigFixtures();
+        $this->restoreEnvFile();
+
+        parent::tearDown();
+    }
+
+    protected function setUpConfigFixtures()
+    {
+        // Mock config path and copy fixtures
+        if (!is_dir(storage_path('temp/tests/config'))) {
+            mkdir(storage_path('temp/tests/config'), 0777, true);
+        }
+
+        foreach (glob(base_path('tests/fixtures/config/*.php')) as $file) {
+            $path = pathinfo($file);
+            copy($file, storage_path('temp/tests/config/' . $path['basename']));
+        }
+
+        static::$fixturesCopied = true;
+
+        // Store original config path
+        static::$origConfigPath = $this->app->make('path.config');
+
+        $this->app->instance('path.config', storage_path('temp/tests/config'));
+
+        // Re-load configuration
+        $configBootstrap = new LoadConfiguration;
+        $configBootstrap->bootstrap($this->app);
+    }
+
+    protected function tearDownConfigFixtures()
+    {
+        // Remove copied config fixtures
+        if (static::$fixturesCopied) {
+            foreach (glob(storage_path('temp/tests/config/*.php')) as $file) {
+                unlink($file);
+            }
+            rmdir(storage_path('temp/tests/config'));
+            rmdir(storage_path('temp/tests'));
+
+            static::$fixturesCopied = false;
+        }
+
+        // Restore config path
+        if (self::$origConfigPath) {
+            $this->app->instance('path.config', static::$origConfigPath);
+
+            static::$origConfigPath = null;
+        }
+
+        // Re-load configuration
+        $configBootstrap = new LoadConfiguration;
+        $configBootstrap->bootstrap($this->app);
+    }
+
+    protected function stubOutEnvFile()
+    {
+        if (file_exists(base_path('.env.stub'))) {
+            unlink(base_path('.env.stub'));
+        }
+        if (file_exists(base_path('.env'))) {
+            rename(base_path('.env'), base_path('.env.stub'));
+        }
+    }
+
+    protected function restoreEnvFile()
+    {
+        unlink(base_path('.env'));
+
+        if (file_exists(base_path('.env.stub'))) {
+            rename(base_path('.env.stub'), base_path('.env'));
+        }
+    }
+}