From 2950cea070e8370f45691c8cdea7e294951d7902 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 2 Jul 2026 08:55:39 +1000 Subject: [PATCH 1/3] Add failing test for merge_strings nested-dict crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `L10nHelper.merge_strings` bookkeeps keys via `plutil` (which parses a nested-dictionary value fine) but re-tokenizes the raw lines through `prefix_keys`, which raises `Invalid character` on the `{`. Such a file is a valid property list, still detected as `:text`, so it passes the format gate and reaches `prefix_keys` — a public `ios_merge_strings_files` run that previously copied the line through now aborts. This stacks a regression test on top of #741 and is intentionally left red so the reviewer can decide how to make the path fail-soft (e.g. rescue and copy the line verbatim, matching the scanner path). --- Generated with the help of Claude Code, https://claude.com/claude-code Co-Authored-By: Claude Opus 4.8 (1M context) --- spec/ios_l10n_helper_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/ios_l10n_helper_spec.rb b/spec/ios_l10n_helper_spec.rb index 5f01c93e0..c0dcddfd2 100644 --- a/spec/ios_l10n_helper_spec.rb +++ b/spec/ios_l10n_helper_spec.rb @@ -160,6 +160,22 @@ def file_encoding(path) end end + it 'does not crash merging a text `.strings` file whose value is a nested dictionary' do + # Regression: `merge_strings` bookkeeps keys via `plutil` (which parses a nested-dictionary value + # fine) but re-tokenizes the raw lines through `prefix_keys`, which raises `Invalid character` on + # the `{`. Such a file is still `:text` and passes the format gate, so a public `ios_merge_strings_files` + # run that previously copied the line through now aborts. It should degrade gracefully instead of raising. + content = %("k" = { a = b; };\n) + Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir| + input_file = File.join(tmp_dir, 'InfoPlist.strings') + File.write(input_file, content) + output_file = File.join(tmp_dir, 'output.strings') + expect do + described_class.merge_strings(paths: { input_file => 'pfx.' }, output_path: output_file) + end.not_to raise_error + end + end + it 'returns duplicate keys found' do paths = { fixture('Localizable-utf16.strings') => nil, fixture('non-latin-utf16.strings') => nil } Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir| From e155caf0b9b58e0a3e833d4ff5cbe3fe4010d2a9 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:09:15 -0600 Subject: [PATCH 2/3] Fix `merge_strings` crash on nested-dictionary `.strings` value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `merge_strings` bookkeeps keys via `plutil` but rewrites them by re-tokenizing the raw lines through `StringsFileValidationHelper.prefix_keys`, whose flat-`.strings` grammar has no `{`. A valid file like `"k" = { a = b; };` clears the `:text` format gate, reaches `prefix_keys`, and raised `Invalid character` — aborting a merge that previously copied the line through. Fail soft: rescue the tokenizer and copy the file's lines through unprefixed with a `UI.important` warning, mirroring the `scan_for_duplicate_keys` `:unscannable` path. Flip the regression test to assert the graceful degradation (no raise, warning emitted, line survives). --- CHANGELOG.md | 1 + .../helper/ios/ios_l10n_helper.rb | 15 ++++++++++++++- spec/ios_l10n_helper_spec.rb | 16 +++++++++++----- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0c2b1693..60759ef77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ _None_ - `StringsFileValidationHelper.find_duplicated_keys` now also accepts comments placed *between* the tokens of a statement (e.g. `"key" /* note */ = "value";`), which `plutil` allows, instead of raising `Invalid character` on the `/`. [#741] - `ios_lint_localizations`' `check_duplicate_keys` no longer crashes the lane on a file that parses as a property list but isn't a flat `.strings` (e.g. a nested-dictionary value); it now warns via `UI.important` and skips that file. [#741] - `L10nHelper.merge_strings` now applies the key prefix via a comment-aware tokenizer, so it correctly prefixes unquoted keys containing `. - $ : /`, lines with an *unquoted* value (e.g. `CFBundleName = WordPress;`), and keys sitting behind an inter-token `.strings` comment (e.g. `CFBundleName /* note */ = WordPress;`) — matching the grammar `plutil` accepts. Previously the line-based matcher left those keys written to the merged file without the prefix while still bookkeeping them *with* it, leaving the output inconsistent with the reported keys (which could resurface the very collisions the prefix avoids and break downstream key extraction). [#741] +- `L10nHelper.merge_strings` no longer crashes on a `:text` `.strings` file whose value is a nested dictionary (`"k" = { … };`) — valid input `plutil` accepts but the flat-`.strings` prefixing tokenizer can't rewrite. It now copies that file's lines through unprefixed with a `UI.important` warning instead of aborting the whole merge, mirroring the fail-soft behavior on the duplicate-key scanner path. [#750] ### Internal Changes diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb index 83168681f..9f082dc74 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb @@ -78,6 +78,9 @@ def self.read_utf8_lines(file) # @note The method is able to handle input files which are using different encodings, # guessing the encoding of each input file using the BOM (and defaulting to UTF8). # The generated file will always be in utf-8, by convention. + # @note If a file's keys can't be prefixed (e.g. it holds a nested-dictionary value, `"k" = { … };`, + # which `plutil` accepts but the flat-`.strings` tokenizer can't rewrite), its lines are copied + # through unprefixed with a warning rather than aborting the whole merge. # # @raise [RuntimeError] If one of the paths provided is not in text format (but XML or binary instead), or if any of the files are missing. # @@ -104,7 +107,17 @@ def self.merge_strings(paths:, output_path:) # sit (e.g. `CFBundleName /* note */ = WordPress;`) and `key = value`-looking text inside a comment is # left alone — keeping the written keys consistent with the (`plutil`-derived) keys bookkept above. lines = read_utf8_lines(input_file) - lines = Fastlane::Helper::Ios::StringsFileValidationHelper.prefix_keys(lines: lines, prefix: prefix) + begin + lines = Fastlane::Helper::Ios::StringsFileValidationHelper.prefix_keys(lines: lines, prefix: prefix) + rescue StandardError => e + # `plutil` accepts flat-`.strings` constructs the tokenizer doesn't — most notably a nested-dictionary + # value (`"k" = { … };`), which parses fine (so the file clears the `:text` gate above) yet exposes no + # flat `key = value` for `prefix_keys` to rewrite, so it raises on the `{`. Fail soft: copy this file's + # lines through unprefixed rather than aborting the whole merge — mirroring the scanner path, where + # `scan_for_duplicate_keys` returns `:unscannable` instead of crashing the lane. `lines` is untouched + # by the raise (the assignment above never completes), so it still holds the original file contents. + UI.important("Could not add prefix `#{prefix}` to the keys in `#{input_file}` (#{e.message}); copying its lines through unprefixed.") + end lines.each { |line| tmp_file.write(line) } tmp_file.write("\n") end diff --git a/spec/ios_l10n_helper_spec.rb b/spec/ios_l10n_helper_spec.rb index c0dcddfd2..e92b61d10 100644 --- a/spec/ios_l10n_helper_spec.rb +++ b/spec/ios_l10n_helper_spec.rb @@ -160,19 +160,25 @@ def file_encoding(path) end end - it 'does not crash merging a text `.strings` file whose value is a nested dictionary' do - # Regression: `merge_strings` bookkeeps keys via `plutil` (which parses a nested-dictionary value - # fine) but re-tokenizes the raw lines through `prefix_keys`, which raises `Invalid character` on - # the `{`. Such a file is still `:text` and passes the format gate, so a public `ios_merge_strings_files` - # run that previously copied the line through now aborts. It should degrade gracefully instead of raising. + it 'copies a nested-dictionary value through unprefixed (with a warning) instead of crashing the merge' do + # Regression: `merge_strings` bookkeeps keys via `plutil` (which parses a nested-dictionary value fine) + # but rewrites keys by re-tokenizing the raw lines through `prefix_keys`, whose flat-`.strings` grammar + # has no `{` — so it raised `Invalid character`. Such a file is still `:text` and clears the format gate, + # so a public `ios_merge_strings_files` run that previously copied the line through would abort. It now + # degrades gracefully — copying the un-prefixable line verbatim and warning — like the scanner path. content = %("k" = { a = b; };\n) Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir| input_file = File.join(tmp_dir, 'InfoPlist.strings') File.write(input_file, content) output_file = File.join(tmp_dir, 'output.strings') + + expect(FastlaneCore::UI).to receive(:important).with(a_string_including('Could not add prefix `pfx.`').and(a_string_including('unprefixed'))) expect do described_class.merge_strings(paths: { input_file => 'pfx.' }, output_path: output_file) end.not_to raise_error + + # The nested-dict line survives verbatim — unprefixed, since it has no flat `key = value` to rewrite. + expect(File.read(output_file)).to include('"k" = { a = b; };') end end From 7afb9a58dd0574a7e4d84b12038fc7d9645ce450 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:09:15 -0600 Subject: [PATCH 3/3] Fix typo in `ios_lint_localizations` unsupported-format warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 9eabbb06 reworded the warning to "can only occurr" (a typo) without updating the spec, which still expected the prior "only make sense" text — leaving `ios_lint_localizations_spec.rb`'s "warns if input files are not in ASCII-plist format" example failing on `foundation`. Correct the spelling to "can only occur" and align the spec. --- .../wpmreleasetoolkit/actions/ios/ios_lint_localizations.rb | 2 +- spec/ios_lint_localizations_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_lint_localizations.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_lint_localizations.rb index edfd1892c..c93481502 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_lint_localizations.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_lint_localizations.rb @@ -62,7 +62,7 @@ def self.find_duplicated_keys(params) duplicate_keys[language] = payload.map { |key, value| "`#{key}` was found at multiple lines: #{value.join(', ')}" } unless payload.empty? when :unsupported_format UI.important <<~WRONG_FORMAT - File `#{path}` is in #{payload} format, while finding duplicate keys can only occurr on files that are in ASCII-plist format. + File `#{path}` is in #{payload} format, while finding duplicate keys can only occur on files that are in ASCII-plist format. Since your files are in #{payload} format, you should probably disable the `check_duplicate_keys` option from this `#{action_name}` call. WRONG_FORMAT when :unscannable diff --git a/spec/ios_lint_localizations_spec.rb b/spec/ios_lint_localizations_spec.rb index 737eaa6a5..5223b293e 100644 --- a/spec/ios_lint_localizations_spec.rb +++ b/spec/ios_lint_localizations_spec.rb @@ -194,7 +194,7 @@ def run_l10n_linter_test(data_file:, check_duplicate_keys: nil, fail_on_strings_ File.write(File.join(fr_lproj, 'Localizable.strings'), File.read(xml_file)) expected_message = <<~EXPECTED_WARNING - File `#{fr_lproj}/Localizable.strings` is in xml format, while finding duplicate keys only make sense on files that are in ASCII-plist format. + File `#{fr_lproj}/Localizable.strings` is in xml format, while finding duplicate keys can only occur on files that are in ASCII-plist format. Since your files are in xml format, you should probably disable the `check_duplicate_keys` option from this `ios_lint_localizations` call. EXPECTED_WARNING expect(FastlaneCore::UI).to receive(:important).with(expected_message)