From 7d676952c8f4f3cb4e16337d7138e0d5d31098e0 Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Tue, 6 Oct 2026 20:44:47 +0200 Subject: [PATCH 1/2] Incorporate pocketarc 3.4.4: guard get_dir_file_info(), add Edge UA test get_dir_file_info() wrote 'relative_path' onto the return of get_file_info() without checking it; for a broken symlink that is FALSE, triggering the PHP 8.1 "automatic conversion of false to array" deprecation (fatal in PHP 9). Such entries are now skipped. Array keys are unchanged, so there is no BC break (pocketarc PR #48, issue #47). Port test_edge() from pocketarc PR #50 to pin Edge vs Chrome detection order, adjusted for the local 'Edge Legacy (Spartan)' label. Co-Authored-By: Claude Opus 5.5 --- system/helpers/file_helper.php | 10 ++++++-- .../codeigniter/libraries/Useragent_test.php | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/system/helpers/file_helper.php b/system/helpers/file_helper.php index a2adaf296ed..45f89d8e09f 100644 --- a/system/helpers/file_helper.php +++ b/system/helpers/file_helper.php @@ -248,8 +248,14 @@ function get_dir_file_info($source_dir, $top_level_only = TRUE, $_recursion = FA } elseif ($file[0] !== '.') { - $_filedata[$file] = get_file_info($source_dir.$file); - $_filedata[$file]['relative_path'] = $relative_path; + $filedata = get_file_info($source_dir.$file); + + // get_file_info() returns FALSE for e.g. broken symlinks + if (is_array($filedata)) + { + $filedata['relative_path'] = $relative_path; + $_filedata[$file] = $filedata; + } } } diff --git a/tests/codeigniter/libraries/Useragent_test.php b/tests/codeigniter/libraries/Useragent_test.php index 087544a1a50..07cf26e60c9 100644 --- a/tests/codeigniter/libraries/Useragent_test.php +++ b/tests/codeigniter/libraries/Useragent_test.php @@ -122,4 +122,27 @@ public function test_parse() $this->assertTrue($this->agent->is_mobile('android')); } + // -------------------------------------------------------------------- + + public function test_edge() + { + // Chromium-based Edge (desktop) - must not be reported as Chrome + $this->agent->parse('Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36 Edg/91.0.864.59'); + $this->assertEquals('Edge', $this->agent->browser()); + $this->assertEquals('91.0.864.59', $this->agent->version()); + // is_browser() takes the config key, not the label + $this->assertTrue($this->agent->is_browser('Edg')); + $this->assertFalse($this->agent->is_browser('Chrome')); + + // Legacy EdgeHTML (Spartan) + $this->agent->parse('Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36 Edge/18.18363'); + $this->assertEquals('Edge Legacy (Spartan)', $this->agent->browser()); + $this->assertEquals('18.18363', $this->agent->version()); + + // Chromium-based Edge (Android) + $this->agent->parse('Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36 EdgA/120.0.0.0'); + $this->assertEquals('Edge', $this->agent->browser()); + $this->assertEquals('120.0.0.0', $this->agent->version()); + } + } \ No newline at end of file From a11d5d08c7a4a4e9e2394093573528dc608d8b02 Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Tue, 6 Oct 2026 20:45:06 +0200 Subject: [PATCH 2/2] Archive pocketarc 3.4.4 incorporation notes Status updated to applied in 7d676952c. Add docs/todo/archive/ for review notes whose status is final. Co-Authored-By: Claude Opus 5.5 --- docs/todo/CLAUDE.md | 2 ++ .../pocketarc-3.4.4-incorporation.md | 36 ++++++++++++++++--- 2 files changed, 34 insertions(+), 4 deletions(-) rename docs/todo/{ => archive}/pocketarc-3.4.4-incorporation.md (80%) diff --git a/docs/todo/CLAUDE.md b/docs/todo/CLAUDE.md index c0d711092df..bc9a0f32e83 100644 --- a/docs/todo/CLAUDE.md +++ b/docs/todo/CLAUDE.md @@ -26,6 +26,8 @@ without re-doing the investigation. Convert relative dates to absolute. Update `Status` when the work lands. +Once `Status` is final (applied or rejected), move the file to `archive/`. + ## Voice Write for any reader, not for the person who requested the review. diff --git a/docs/todo/pocketarc-3.4.4-incorporation.md b/docs/todo/archive/pocketarc-3.4.4-incorporation.md similarity index 80% rename from docs/todo/pocketarc-3.4.4-incorporation.md rename to docs/todo/archive/pocketarc-3.4.4-incorporation.md index cef1ec68593..4065aec632c 100644 --- a/docs/todo/pocketarc-3.4.4-incorporation.md +++ b/docs/todo/archive/pocketarc-3.4.4-incorporation.md @@ -3,7 +3,10 @@ **Release:** https://github.com/pocketarc/codeigniter/releases/tag/3.4.4 (commit `fb57d35`, 18 Jul) **Target:** this repository (CodeIgniter 3.1-stable derivative) **Reviewed:** 2026-08-21 -**Status:** Not actioned. One functional gap (#48), one test gap (#50). +**Status:** Applied 2026-10-06 in `7d676952c` — `is_array()` guard +(section 2) and `test_edge()` (section 5). Key change deferred; see section 6. +**Verdict:** Two items already present; the deprecation guard and the Edge test were +the only gaps, and both are now in place. --- @@ -76,6 +79,16 @@ elseif ($file[0] !== '.') This guards the assignment while preserving the existing basename keys. +**Decided: applied as above** in +[file_helper.php:251-258](system/helpers/file_helper.php#L251-L258). Entries for +which `get_file_info()` fails are now omitted from the result rather than stored +as `FALSE`. + +No unit test was added. `get_dir_file_info()` calls `realpath()` on its first +call, which does not resolve `vfs://` paths, and a dangling symlink on the real +filesystem needs elevated privileges on Windows. The existing `file_helper` tests +pass. + ### PR #48 is not directly applicable — two obstacles 1. **It references `$_root_dir`**, a variable that does not exist in this @@ -157,17 +170,32 @@ Porting it is worthwhile: it pins ordering behaviour that is easy to break by editing `user_agents.php`. The expected legacy label needs adjusting to `Edge Legacy (Spartan)` to match the local config. +**Decided: ported** as `test_edge()` in +[Useragent_test.php](tests/codeigniter/libraries/Useragent_test.php), covering all +three agent strings in the table above (browser label and version). + +### `is_browser()` takes the key, not the label + +Found while writing the test. +[User_agent.php:431](system/libraries/User_agent.php#L431) compares +`$this->browsers[$key]` against the detected label, so the argument is the +**config key**. For Chromium Edge, `is_browser('Edg')` is `TRUE` and +`is_browser('Edge')` is `FALSE`, because the `Edge` key maps to the legacy label. +Callers that test for modern Edge with `is_browser('Edge')` get the wrong answer. +This behaviour is inherited from upstream and is not changed here; the test +asserts `is_browser('Edg')` to document it. + --- ## 6. Recommended order of work 1. **Add the `is_array()` guard in `get_dir_file_info()`** — the only functional gap. Small, no BC break, and it clears a PHP 8.1 deprecation that becomes - fatal under PHP 9. + fatal under PHP 9. **Done.** 2. **Port `test_edge()`** — inexpensive regression protection for behaviour that - is already correct. + is already correct. **Done.** 3. **Defer** the basename-to-relative-path key change. It fixes a real collision - defect but breaks the array contract; decide separately. + defect but breaks the array contract; decide separately. **Deferred.** Nothing in 3.4.4 requires the Edge configuration or `write_log` changes; both are already in place.