From a8ede7b5f4850699203d4691a64236a0aafd0136 Mon Sep 17 00:00:00 2001 From: Ruud Boon Date: Wed, 22 Jan 2020 19:13:22 +0100 Subject: [PATCH 1/6] Without doing a file exists check we aren't able to detect if extension is set. I would like to make the users responsibility to not add the extension like it's described in the docs. Fixed #14756 --- phalcon/Config/ConfigFactory.zep | 5 +- .../config/config-with.in-file.name.ini | 58 +++++++++++++++++++ tests/unit/Config/ConfigFactory/LoadCest.php | 16 +++++ 3 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 tests/_data/assets/config/config-with.in-file.name.ini diff --git a/phalcon/Config/ConfigFactory.zep b/phalcon/Config/ConfigFactory.zep index 13ca91dd023..121c4f7d661 100644 --- a/phalcon/Config/ConfigFactory.zep +++ b/phalcon/Config/ConfigFactory.zep @@ -101,9 +101,8 @@ class ConfigFactory extends AbstractFactory first = config["filePath"], second = null; - if !strpos(first, ".") { - let first = first . "." . lcfirst(adapter); - } + + let first = first . "." . lcfirst(adapter); if "ini" === adapter { let second = Arr::get(config, "mode", 1); diff --git a/tests/_data/assets/config/config-with.in-file.name.ini b/tests/_data/assets/config/config-with.in-file.name.ini new file mode 100644 index 00000000000..63114d2a0af --- /dev/null +++ b/tests/_data/assets/config/config-with.in-file.name.ini @@ -0,0 +1,58 @@ +[annotations] +adapter = apcu +options.prefix = annotations +options.lifetime = 3600 + +[cache] +adapter = libmemcached +options.prefix = app-data + +[config] +adapter = ini +filePath = PATH_DATA"assets/config/config-with.in-file.name" +filePathExtension = PATH_DATA"assets/config/config-with.in-file.name.ini" + +[database] +adapter = mysql +options.host = DATA_MYSQL_HOST +options.username = DATA_MYSQL_USER +options.password = DATA_MYSQL_PASS +options.dbname = DATA_MYSQL_NAME +options.port = DATA_MYSQL_PORT +options.charset = DATA_MYSQL_CHARSET + +[image] +adapter = imagick +file = PATH_DATA"assets/images/phalconphp.jpg" + +[logger] +name = "my-logger" +options.adapters.0.adapter = stream +options.adapters.0.options.name = PATH_OUTPUT"tests/logs/factory.log" +options.adapters.1.adapter = stream +options.adapters.1.options.name = PATH_OUTPUT"tests/logs/factory2.log" + +[paginator] +adapter = queryBuilder +options.limit = 20 +options.page = 1 + +[translate] +adapter = gettext +options.locale = en_US.utf8 +options.defaultDomain = messages +options.directory = PATH_DATA"assets/translation/gettext" +options.category = LC_MESSAGES + + + + +[session] +uniqueId = my-private-app +host = 127.0.0.1 +port = 11211 +persistent = true +lifetime = 3600 +prefix = my_ +adapter = Files + diff --git a/tests/unit/Config/ConfigFactory/LoadCest.php b/tests/unit/Config/ConfigFactory/LoadCest.php index cd6aeec8c1f..8392dd5321e 100644 --- a/tests/unit/Config/ConfigFactory/LoadCest.php +++ b/tests/unit/Config/ConfigFactory/LoadCest.php @@ -53,6 +53,22 @@ public function configFactoryLoadConfig(UnitTester $I) Ini::class, $ini ); + + //Issue 14756 + $configFile = dataDir('assets/config/config-with.in-file.name.ini'); + $ini = new Ini($configFile, INI_SCANNER_NORMAL); + $I->assertInstanceOf( + Ini::class, + $ini + ); + + /** @var Ini $ini */ + $ini = (new ConfigFactory())->load($ini->config->toArray()); + + $I->assertInstanceOf( + Ini::class, + $ini + ); } /** From d1eaf72997d9d38ac7a04400bceea6a5414e188f Mon Sep 17 00:00:00 2001 From: Ruud Boon Date: Wed, 22 Jan 2020 19:17:34 +0100 Subject: [PATCH 2/6] Updated Changelog --- CHANGELOG-4.0.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG-4.0.md b/CHANGELOG-4.0.md index 1940697c1ef..3e7d3c0b4e4 100644 --- a/CHANGELOG-4.0.md +++ b/CHANGELOG-4.0.md @@ -9,6 +9,7 @@ - Fixed `Phalcon\Storage\Adapter\Stream::getKeys()` bug in the absence of a directory with a prefix name [#14725](https://github.com/phalcon/cphalcon/issues/14725), [#14721](https://github.com/phalcon/cphalcon/pull/14721) - Fixed `Phalcon\Debug::onUncaughtException` Should accept `\Throwable` instead of `\Exception` as an argument [#14738](https://github.com/phalcon/cphalcon/pull/14738) - Fixed `Phalcon\Mvc\Model\Binder` to now correctly call `has` and `set` on the cache object [#14743](https://github.com/phalcon/cphalcon/pull/14743) +- Fixed `Phalcon\Config\ConfigFactory` to always add the correct extension [#14756](https://github.com/phalcon/cphalcon/issues/14756) # [4.0.2](https://github.com/phalcon/cphalcon/releases/tag/v4.0.1) (2020-01-12) ## Fixed From 6c740ad7cfe7c250d9150d877c57de3e59839dfc Mon Sep 17 00:00:00 2001 From: Ruud Boon Date: Wed, 22 Jan 2020 21:45:02 +0100 Subject: [PATCH 3/6] Use filepath to build correct path --- phalcon/Config/ConfigFactory.zep | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phalcon/Config/ConfigFactory.zep b/phalcon/Config/ConfigFactory.zep index 121c4f7d661..323ad799c21 100644 --- a/phalcon/Config/ConfigFactory.zep +++ b/phalcon/Config/ConfigFactory.zep @@ -57,7 +57,7 @@ class ConfigFactory extends AbstractFactory */ public function load(config) -> object { - var adapter, extension, first, oldConfig, second; + var adapter, extension, first, oldConfig, second, pathParts; if typeof config === "string" { let oldConfig = config, @@ -101,8 +101,8 @@ class ConfigFactory extends AbstractFactory first = config["filePath"], second = null; - - let first = first . "." . lcfirst(adapter); + pathParts = pathinfo(first); + let first = pathParts["dirname"] . DIRECTORY_SEPARATOR . pathParts["filename"]; if "ini" === adapter { let second = Arr::get(config, "mode", 1); From 964a2f38c6e74d46fc6372366bd7c035c4f924da Mon Sep 17 00:00:00 2001 From: Ruud Boon Date: Wed, 22 Jan 2020 21:59:28 +0100 Subject: [PATCH 4/6] Missing let --- phalcon/Config/ConfigFactory.zep | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phalcon/Config/ConfigFactory.zep b/phalcon/Config/ConfigFactory.zep index 323ad799c21..09e9d8ddfd0 100644 --- a/phalcon/Config/ConfigFactory.zep +++ b/phalcon/Config/ConfigFactory.zep @@ -101,7 +101,7 @@ class ConfigFactory extends AbstractFactory first = config["filePath"], second = null; - pathParts = pathinfo(first); + let pathParts = pathinfo(first); let first = pathParts["dirname"] . DIRECTORY_SEPARATOR . pathParts["filename"]; if "ini" === adapter { From c17aa9f6021e5b1b272a8743f8d864b471c78344 Mon Sep 17 00:00:00 2001 From: Ruud Boon Date: Wed, 22 Jan 2020 22:40:12 +0100 Subject: [PATCH 5/6] Replaced implementation --- phalcon/Config/ConfigFactory.zep | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/phalcon/Config/ConfigFactory.zep b/phalcon/Config/ConfigFactory.zep index 09e9d8ddfd0..b93900b56dc 100644 --- a/phalcon/Config/ConfigFactory.zep +++ b/phalcon/Config/ConfigFactory.zep @@ -57,7 +57,7 @@ class ConfigFactory extends AbstractFactory */ public function load(config) -> object { - var adapter, extension, first, oldConfig, second, pathParts; + var adapter, extension, first, oldConfig, second; if typeof config === "string" { let oldConfig = config, @@ -101,8 +101,9 @@ class ConfigFactory extends AbstractFactory first = config["filePath"], second = null; - let pathParts = pathinfo(first); - let first = pathParts["dirname"] . DIRECTORY_SEPARATOR . pathParts["filename"]; + if (empty(pathinfo(first, PATHINFO_EXTENSION))) { + let first = first . "." . lcfirst(adapter); + } if "ini" === adapter { let second = Arr::get(config, "mode", 1); From dec638cd01ea0114b5340129c81eca1b6a7c58b6 Mon Sep 17 00:00:00 2001 From: Ruud Boon Date: Thu, 23 Jan 2020 07:21:36 +0100 Subject: [PATCH 6/6] Fixed asset --- tests/_data/assets/config/config-with.in-file.name.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/_data/assets/config/config-with.in-file.name.ini b/tests/_data/assets/config/config-with.in-file.name.ini index 63114d2a0af..0d1b626f1a9 100644 --- a/tests/_data/assets/config/config-with.in-file.name.ini +++ b/tests/_data/assets/config/config-with.in-file.name.ini @@ -9,7 +9,7 @@ options.prefix = app-data [config] adapter = ini -filePath = PATH_DATA"assets/config/config-with.in-file.name" +filePath = PATH_DATA"assets/config/config-with.in-file.name.ini" filePathExtension = PATH_DATA"assets/config/config-with.in-file.name.ini" [database]