[PR #4918] [CLOSED] fix(apktool): select three-digit regions through the b+ locale table #6674

Closed
opened 2026-11-01 21:42:13 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4918
Author: @mertkayacs
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/apktool-numeric-region-locale


📝 Commits (6)

  • 7e39288 test(apktool): select three-digit regions through the b+ locale table
  • 1299b4b fix(apktool): select three-digit regions through the b+ locale table
  • 631ae5b test(apktool): cover more three-digit regions and the rejected r-form spelling
  • 5924e23 docs(apktool): name the Apktool versions behind the b+ region rule
  • 1450f8e test(apktool): keep the string table describe blocks under the lint limit
  • facf1d0 docs(apktool): split the locale spelling sentence into short sentences

📊 Changes

5 files changed (+95 additions, -8 deletions)

View changed files

📝 docs/apktool-resource-analysis.md (+3 -2)
📝 src/apktool/ApktoolStringTables.test.ts (+37 -1)
📝 src/domain/apktool/apktoolResourceAnalysis.test.ts (+20 -2)
📝 src/domain/apktool/apktoolResourceAnalysis.ts (+10 -3)
📝 tests/boundary/cli/apktoolLocaleOption.test.ts (+25 -0)

📄 Description

Summary

decode_android_resources normalizes a regional locale alias at the selection owner, so pt-BR reads res/values-pt-rBR/strings.xml. The same alias rule turned a three-digit region into -r form: es-419 selected res/values-es-r419/strings.xml. Apktool never writes that directory. For a region of three digits it writes only the BCP 47 form, values-b+es+419, so the request came back missing while the table existed under its real name. es-419 now selects values-b+es+419, and the hybrid spelling es-r419, which names no table, is no longer admitted. Fixes #4916.

Problem and expected behavior

Trigger: rea decode-android-resources <apk> --locale es-419 --json on an APK whose resources carry Latin American Spanish, which Apktool decodes to res/values-b+es+419/strings.xml. Before this change the result reports the table missing and names values-es-r419 in the limitation.

Violated invariant: the alias must select the directory the producer writes. Every Apktool REA supports writes a three-digit region in the b+ form. At the documented floor, v2.7.0, ResConfigFlags.getLocaleString (brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/ResConfigFlags.java, lines 440-470) takes the legacy -<lang>[-r<REGION>] branch only when region.length != 3, and ARSCDecoder.unpackLanguageOrRegion (lines 512-524) expands a packed region to three characters; v2.11.1 keeps the same rule (lines 594-630) and v2.12.1 states it as mRegion.length() == 2 (generateQualifiers, lines 275-285). On 3.x the rule lives in ResConfig.computeQualifiers (brut/androlib/res/table/ResConfig.java, v3.0.3 lines 275-295): legacy form only when the script is empty, the variant is empty and the region is empty or two characters; any other locale, a three-digit region included, is written as -b+<lang>[+<script>][+<region>][+<variant>]. aapt2's directory parser agrees from the other side: LocaleValue::InitFromParts (libs/androidfw/Locale.cpp) takes a legacy region part only when it is r followed by two characters, so values-es-r419 is not a resource directory aapt2 accepts either. The -r419 spelling has one source, ResTable_config::appendDirLocale in ResourceTypes.cpp, which prints a config without a script that way; that is a runtime printer, not the decoder that lays out the workspace REA reads.

Expected: es-419 and b+es+419 are two aliases of one physical table, read once, both reported present; es-r419 is refused by the locale grammar like any other spelling that cannot name a table.

Change and scope

  • androidLocaleDirectorySuffix maps <lang>-<DDD> to b+<lang>+<DDD>; <lang>-<RR> still maps to <lang>-r<RR>.
  • The locale grammar keeps <lang>-<DDD> as an admitted alias and drops <lang>-r<DDD>, which no Android resource directory carries. The grammar's message now lists es-419 among its examples.
  • docs/apktool-resource-analysis.md states the three-digit mapping next to the existing pt-BR sentence.
  • Not included: inferring Android's runtime locale matching between tables, and the combined-qualifier disclosure that #4738 adds in the same module; this change touches only the alias mapping and the grammar, so the two compose.

Contribution scope: ordinary fix within the existing read-only Apktool decode boundary; not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: androidLocaleDirectorySuffix and androidLocaleQualifierSchema in src/domain/apktool/apktoolResourceAnalysis.ts; request parsing and table selection.
  • Contribution scope fit and linked maintainer decision: not applicable.
  • CLI and MCP/tool-catalog contract: locale and locales keep their grammar for every spelling except <lang>-r<DDD>, which is now refused with the owner's message. The locales listing is unchanged: b+<lang>+<DDD> directories were already advertised as b+…, and <lang>-r<DDD> directories do not occur.
  • Provider, bridge, target-format, or platform compatibility: Apktool output only; no change to the launcher or the decode command.
  • Evidence, artifact, provenance, or reconstruction contract: a table that exists is now reported present for the alias instead of missing; nothing else changes shape.
  • Target selection, authority, upstream behavior, and cleanup details: unchanged.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: src/domain/apktool/apktoolResourceAnalysis.test.ts (aliases en-419, en-001, ar-015 and fil-419 normalize to the b+ form; b+es+419, b+en+001 and b+ar+015 are selectable tables; es-r419, en-r001 and ar-r015 are refused with the message that names es-419; pt-BR still maps to pt-rBR), src/apktool/ApktoolStringTables.test.ts (es-419, en-001 and ar-015 each read their values-b+<lang>+<DDD>/strings.xml once alongside the b+ alias and both are present; the existing pt-BR and pt-rBR case still reads values-pt-rBR once), tests/boundary/cli/apktoolLocaleOption.test.ts (the public CLI refuses --locale en-r419 and --locale es-r419 with code: VALIDATION_ERROR and a locale field error carrying the grammar message).
  • Base reproduction or other evidence: executed. With only the test commit (7e39288e4) on a25eb5fbb, npm run test:local on the two unit files reports 5 failed / 39 passed, and npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts reports 1 failed / 5 passed. The producer behavior is source-derived from Apktool ResConfig.computeQualifiers and androidfw LocaleValue::InitFromParts, cited above.
  • User-visible CLI/MCP output: for --locale es-419, string_tables[0] (and the legacy strings field) now carry the entries of values-b+es+419/strings.xml with status: "present" instead of status: "missing" and the limitation No strings.xml was found for locale es-419. --locale en-r419 now exits with the grammar message Locale must look like de, pt-BR, pt-rBR, es-419, or b+zh+Hans+CN.
  • Remaining proof gaps: no Apktool run on a real APK here; the directory name comes from Apktool's source rather than from decoding a Latin American Spanish APK on this machine.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:local -- src/domain/apktool/apktoolResourceAnalysis.test.ts src/apktool/ApktoolStringTables.test.ts at 7e39288e4 (tests only) — 5 failed, 39 passed.
  • npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts at 7e39288e4 — 1 failed, 5 passed.
  • npm run test:local -- src/domain/apktool/apktoolResourceAnalysis.test.ts src/apktool/ApktoolStringTables.test.ts src/apktool/ApktoolProvider.test.ts src/apktool/ApktoolDecodeOutput.test.ts src/application/apktool/ApktoolResourceAnalysisService.test.ts at 1299b4bd4 — 152 passed.
  • npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts tests/boundary/cli/apktoolCli.test.ts tests/boundary/mcp/apktoolMcp.test.ts at 1299b4bd4 — 11 passed.
  • npm run check:fast — 2 tasks successful.
  • npm run docs:check — generated-doc checks passed; tests/conformance/readmeConsistency.test.ts passed.
  • npx oxfmt --check on the four touched TypeScript files — all formatted.
  • Follow-up after review (commits 631ae5b37 and 5924e2372), then rebased onto a25eb5fbb with the string table describe block split in two (1450f8e51) so the merged file stays under the lint limit, and the docs sentence split (facf1d0a2): npm run test:local on all nine src/apktool/*.test.ts and src/domain/apktool/*.test.ts files — 253 passed; npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts tests/boundary/cli/apktoolCli.test.ts tests/boundary/mcp/apktoolMcp.test.ts tests/boundary/cli/cliInputSchemaRequired.test.ts — 17 passed; npm run check:fast — 2 tasks successful; npm run docs:check and tests/conformance/readmeConsistency.test.ts — passed; npx oxfmt --check on the touched files — all formatted. Node 24.18.0, Linux x64.

Compatibility, safety, and release

  • Breaking changes or migration steps: none for any locale that names a table. A caller that passed <lang>-r<DDD> is now refused instead of receiving missing; <lang>-<DDD> or b+<lang>+<DDD> selects the table.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider executed.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: path construction only changes the directory name under the owned workspace; the grammar still bounds the suffix to the same character set and length.

Author readiness

Human owner: @mertkayacs

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4918 **Author:** [@mertkayacs](https://github.com/mertkayacs) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/apktool-numeric-region-locale` --- ### 📝 Commits (6) - [`7e39288`](https://github.com/morluto/rea/commit/7e39288e4ca4f1decb42d2760aeeca746b610ac9) test(apktool): select three-digit regions through the b+ locale table - [`1299b4b`](https://github.com/morluto/rea/commit/1299b4bd4529d5479b3fa5cd09c8af284d4192c4) fix(apktool): select three-digit regions through the b+ locale table - [`631ae5b`](https://github.com/morluto/rea/commit/631ae5b379889ac1d8a48b6b924da722efde3c9c) test(apktool): cover more three-digit regions and the rejected r-form spelling - [`5924e23`](https://github.com/morluto/rea/commit/5924e237256415eb0e3a707f5dda40f430308638) docs(apktool): name the Apktool versions behind the b+ region rule - [`1450f8e`](https://github.com/morluto/rea/commit/1450f8e51a30604d88d3e2ca58c255193954e88a) test(apktool): keep the string table describe blocks under the lint limit - [`facf1d0`](https://github.com/morluto/rea/commit/facf1d0a2cfb546b68f733282928c6f0b58bb7e5) docs(apktool): split the locale spelling sentence into short sentences ### 📊 Changes **5 files changed** (+95 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `docs/apktool-resource-analysis.md` (+3 -2) 📝 `src/apktool/ApktoolStringTables.test.ts` (+37 -1) 📝 `src/domain/apktool/apktoolResourceAnalysis.test.ts` (+20 -2) 📝 `src/domain/apktool/apktoolResourceAnalysis.ts` (+10 -3) 📝 `tests/boundary/cli/apktoolLocaleOption.test.ts` (+25 -0) </details> ### 📄 Description ## Summary `decode_android_resources` normalizes a regional locale alias at the selection owner, so `pt-BR` reads `res/values-pt-rBR/strings.xml`. The same alias rule turned a three-digit region into `-r` form: `es-419` selected `res/values-es-r419/strings.xml`. Apktool never writes that directory. For a region of three digits it writes only the BCP 47 form, `values-b+es+419`, so the request came back `missing` while the table existed under its real name. `es-419` now selects `values-b+es+419`, and the hybrid spelling `es-r419`, which names no table, is no longer admitted. Fixes #4916. ## Problem and expected behavior Trigger: `rea decode-android-resources <apk> --locale es-419 --json` on an APK whose resources carry Latin American Spanish, which Apktool decodes to `res/values-b+es+419/strings.xml`. Before this change the result reports the table `missing` and names `values-es-r419` in the limitation. Violated invariant: the alias must select the directory the producer writes. Every Apktool REA supports writes a three-digit region in the `b+` form. At the documented floor, v2.7.0, `ResConfigFlags.getLocaleString` (`brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/ResConfigFlags.java`, lines 440-470) takes the legacy `-<lang>[-r<REGION>]` branch only when `region.length != 3`, and `ARSCDecoder.unpackLanguageOrRegion` (lines 512-524) expands a packed region to three characters; v2.11.1 keeps the same rule (lines 594-630) and v2.12.1 states it as `mRegion.length() == 2` (`generateQualifiers`, lines 275-285). On 3.x the rule lives in `ResConfig.computeQualifiers` (`brut/androlib/res/table/ResConfig.java`, v3.0.3 lines 275-295): legacy form only when the script is empty, the variant is empty and the region is empty or two characters; any other locale, a three-digit region included, is written as `-b+<lang>[+<script>][+<region>][+<variant>]`. aapt2's directory parser agrees from the other side: `LocaleValue::InitFromParts` (`libs/androidfw/Locale.cpp`) takes a legacy region part only when it is `r` followed by two characters, so `values-es-r419` is not a resource directory aapt2 accepts either. The `-r419` spelling has one source, `ResTable_config::appendDirLocale` in `ResourceTypes.cpp`, which prints a config without a script that way; that is a runtime printer, not the decoder that lays out the workspace REA reads. Expected: `es-419` and `b+es+419` are two aliases of one physical table, read once, both reported `present`; `es-r419` is refused by the locale grammar like any other spelling that cannot name a table. ## Change and scope - `androidLocaleDirectorySuffix` maps `<lang>-<DDD>` to `b+<lang>+<DDD>`; `<lang>-<RR>` still maps to `<lang>-r<RR>`. - The locale grammar keeps `<lang>-<DDD>` as an admitted alias and drops `<lang>-r<DDD>`, which no Android resource directory carries. The grammar's message now lists `es-419` among its examples. - `docs/apktool-resource-analysis.md` states the three-digit mapping next to the existing `pt-BR` sentence. - Not included: inferring Android's runtime locale matching between tables, and the combined-qualifier disclosure that #4738 adds in the same module; this change touches only the alias mapping and the grammar, so the two compose. Contribution scope: ordinary fix within the existing read-only Apktool decode boundary; not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `androidLocaleDirectorySuffix` and `androidLocaleQualifierSchema` in `src/domain/apktool/apktoolResourceAnalysis.ts`; request parsing and table selection. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: `locale` and `locales` keep their grammar for every spelling except `<lang>-r<DDD>`, which is now refused with the owner's message. The `locales` listing is unchanged: `b+<lang>+<DDD>` directories were already advertised as `b+…`, and `<lang>-r<DDD>` directories do not occur. - Provider, bridge, target-format, or platform compatibility: Apktool output only; no change to the launcher or the decode command. - Evidence, artifact, provenance, or reconstruction contract: a table that exists is now reported `present` for the alias instead of `missing`; nothing else changes shape. - Target selection, authority, upstream behavior, and cleanup details: unchanged. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/domain/apktool/apktoolResourceAnalysis.test.ts` (aliases `en-419`, `en-001`, `ar-015` and `fil-419` normalize to the `b+` form; `b+es+419`, `b+en+001` and `b+ar+015` are selectable tables; `es-r419`, `en-r001` and `ar-r015` are refused with the message that names `es-419`; `pt-BR` still maps to `pt-rBR`), `src/apktool/ApktoolStringTables.test.ts` (`es-419`, `en-001` and `ar-015` each read their `values-b+<lang>+<DDD>/strings.xml` once alongside the `b+` alias and both are `present`; the existing `pt-BR` and `pt-rBR` case still reads `values-pt-rBR` once), `tests/boundary/cli/apktoolLocaleOption.test.ts` (the public CLI refuses `--locale en-r419` and `--locale es-r419` with `code: VALIDATION_ERROR` and a `locale` field error carrying the grammar message). - Base reproduction or other evidence: executed. With only the test commit (`7e39288e4`) on `a25eb5fbb`, `npm run test:local` on the two unit files reports 5 failed / 39 passed, and `npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts` reports 1 failed / 5 passed. The producer behavior is source-derived from Apktool `ResConfig.computeQualifiers` and androidfw `LocaleValue::InitFromParts`, cited above. - User-visible CLI/MCP output: for `--locale es-419`, `string_tables[0]` (and the legacy `strings` field) now carry the entries of `values-b+es+419/strings.xml` with `status: "present"` instead of `status: "missing"` and the limitation `No strings.xml was found for locale es-419`. `--locale en-r419` now exits with the grammar message `Locale must look like de, pt-BR, pt-rBR, es-419, or b+zh+Hans+CN`. - Remaining proof gaps: no Apktool run on a real APK here; the directory name comes from Apktool's source rather than from decoding a Latin American Spanish APK on this machine. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/domain/apktool/apktoolResourceAnalysis.test.ts src/apktool/ApktoolStringTables.test.ts` at `7e39288e4` (tests only) — 5 failed, 39 passed. - `npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts` at `7e39288e4` — 1 failed, 5 passed. - `npm run test:local -- src/domain/apktool/apktoolResourceAnalysis.test.ts src/apktool/ApktoolStringTables.test.ts src/apktool/ApktoolProvider.test.ts src/apktool/ApktoolDecodeOutput.test.ts src/application/apktool/ApktoolResourceAnalysisService.test.ts` at `1299b4bd4` — 152 passed. - `npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts tests/boundary/cli/apktoolCli.test.ts tests/boundary/mcp/apktoolMcp.test.ts` at `1299b4bd4` — 11 passed. - `npm run check:fast` — 2 tasks successful. - `npm run docs:check` — generated-doc checks passed; `tests/conformance/readmeConsistency.test.ts` passed. - `npx oxfmt --check` on the four touched TypeScript files — all formatted. - Follow-up after review (commits `631ae5b37` and `5924e2372`), then rebased onto `a25eb5fbb` with the string table describe block split in two (`1450f8e51`) so the merged file stays under the lint limit, and the docs sentence split (`facf1d0a2`): `npm run test:local` on all nine `src/apktool/*.test.ts` and `src/domain/apktool/*.test.ts` files — 253 passed; `npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts tests/boundary/cli/apktoolCli.test.ts tests/boundary/mcp/apktoolMcp.test.ts tests/boundary/cli/cliInputSchemaRequired.test.ts` — 17 passed; `npm run check:fast` — 2 tasks successful; `npm run docs:check` and `tests/conformance/readmeConsistency.test.ts` — passed; `npx oxfmt --check` on the touched files — all formatted. Node 24.18.0, Linux x64. ## Compatibility, safety, and release - Breaking changes or migration steps: none for any locale that names a table. A caller that passed `<lang>-r<DDD>` is now refused instead of receiving `missing`; `<lang>-<DDD>` or `b+<lang>+<DDD>` selects the table. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; no provider executed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: path construction only changes the directory name under the owned workspace; the grammar still bounds the suffix to the same character set and length. ## Author readiness Human owner: @mertkayacs - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:42:13 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6674
No description provided.