[PR #4738] [CLOSED] fix(apktool): disclose locale tables that carry other qualifiers #6382

Closed
opened 2026-10-31 01:14:03 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: fix/apktool-qualified-locale-tables


📝 Commits (2)

  • 906676e fix(apktool): disclose locale tables that carry other qualifiers
  • b6c1ea0 docs(apktool): describe qualified locale table disclosure

📊 Changes

5 files changed (+108 additions, -5 deletions)

View changed files

📝 docs/apktool-resource-analysis.md (+3 -0)
📝 src/apktool/ApktoolProvider.test.ts (+28 -0)
📝 src/apktool/ApktoolProvider.ts (+25 -5)
➕ src/domain/apktool/apktoolResourceAnalysis.test.ts (+29 -0)
📝 src/domain/apktool/apktoolResourceAnalysis.ts (+23 -0)

📄 Description

Summary

Fixes #4612. decode_android_resources no longer drops locale strings tables silently when their directory also carries another Android qualifier, such as values-en-rUS-v21, values-b+zh+Hans+CN-night, values-pt-rBR-sw600dp or values-mcc310-fr. locales keeps its documented meaning, and a limitation now names every such table.

Problem and expected behavior

countDecodedFiles in src/apktool/ApktoolProvider.ts validated the whole values-<suffix> against the locale-only grammar (androidLocaleQualifierSchema). Any suffix that combined a locale with other qualifiers failed, so the table was neither listed nor mentioned, and the result still reported coverage: "complete". An analyst could not learn from the result that those locale tables existed.

Change and scope

  • src/domain/apktool/apktoolResourceAnalysis.ts: qualifiedResourceLocale(suffix) returns the locale of a suffix that combines a locale with other qualifiers. It follows Android's qualifier order, where only MCC and MNC precede the locale. It returns undefined for locale-only suffixes (already handled) and for suffixes without a locale (night, v21, car-night, mcc310).
  • src/apktool/ApktoolProvider.ts: discovery collects such tables when they contain strings.xml, and the result adds the limitation locales lists values-<locale> tables only; these locale strings tables also carry other configuration qualifiers and are neither listed nor selectable by locale: values-....
  • docs/apktool-resource-analysis.md: describes the disclosure.

Design choice: locales still lists only values-<locale> tables, because locale selection reads res/values-<locale>/strings.xml. Adding en-rUS to locales when only values-en-rUS-v21 exists would advertise a locale that selection cannot return. coverage stays complete: every table that locales is documented to list is listed, and the qualified ones are disclosed. Making qualified tables selectable (or adding a structured field) is a possible follow-up that would change the output contract, so I left it out.

Contract and boundary impact

  • Semantic owner and earliest changed stage: Apktool decoded-workspace locale discovery.

  • Contribution scope fit and linked maintainer decision: not applicable (ordinary fix within existing boundary).

  • CLI and MCP/tool-catalog contract: output schema unchanged; one new limitation string when qualified locale tables exist.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: previously undisclosed tables are now named in limitations.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added:

    • src/domain/apktool/apktoolResourceAnalysis.test.ts: qualifiedResourceLocale finds en-rUS, b+zh+Hans+CN, pt-rBR, fr (fr-land, mcc310-fr) and en-rUS (mcc310-mnc004-en-rUS-v21); returns nothing for de, pt-rBR, b+sr+Latn, night, car-night, v21, sw600dp-land, mcc310.
    • src/apktool/ApktoolProvider.test.ts "names locale tables that also carry other qualifiers (#4612)": the issue's directories plus mcc310-fr, night and v21 through the existing Apktool stub; asserts locales: ["de"] and the exact limitation.
  • Remaining proof gaps: the provider tests use POSIX shell stubs and are skipped on Windows, where I developed this, so the provider-level test was not executed locally; it runs in the CI test shards. No real Apktool decode or APK was used (the issue's reproduction is also a stub).

  • 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

  • npx vitest run src/domain/apktool src/apktool on Windows 11: 70 passed, 27 skipped (the POSIX-stub provider tests, including the new one).
  • npx tsc --noEmit: exit 0. npm run verify:test-discovery: 1500 files, each once. npm run check:fast (typecheck, lint, pre-push hook): passed.
  • npm run docs:check could not run here: it requires the packaged Windows native addon, which this checkout lacks.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: none; Apktool stub only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: reads the provider-owned decoded workspace only.

Author readiness

Human owner:

  • 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/4738 **Author:** [@Iyanuoluwa007](https://github.com/Iyanuoluwa007) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/apktool-qualified-locale-tables` --- ### 📝 Commits (2) - [`906676e`](https://github.com/morluto/rea/commit/906676ef9eb934d663b24f81af6f170a008c2b65) fix(apktool): disclose locale tables that carry other qualifiers - [`b6c1ea0`](https://github.com/morluto/rea/commit/b6c1ea0aeb4ec4d268fad6358f5bba1c23de84ec) docs(apktool): describe qualified locale table disclosure ### 📊 Changes **5 files changed** (+108 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `docs/apktool-resource-analysis.md` (+3 -0) 📝 `src/apktool/ApktoolProvider.test.ts` (+28 -0) 📝 `src/apktool/ApktoolProvider.ts` (+25 -5) ➕ `src/domain/apktool/apktoolResourceAnalysis.test.ts` (+29 -0) 📝 `src/domain/apktool/apktoolResourceAnalysis.ts` (+23 -0) </details> ### 📄 Description ## Summary Fixes #4612. `decode_android_resources` no longer drops locale strings tables silently when their directory also carries another Android qualifier, such as `values-en-rUS-v21`, `values-b+zh+Hans+CN-night`, `values-pt-rBR-sw600dp` or `values-mcc310-fr`. `locales` keeps its documented meaning, and a limitation now names every such table. ## Problem and expected behavior `countDecodedFiles` in `src/apktool/ApktoolProvider.ts` validated the whole `values-<suffix>` against the locale-only grammar (`androidLocaleQualifierSchema`). Any suffix that combined a locale with other qualifiers failed, so the table was neither listed nor mentioned, and the result still reported `coverage: "complete"`. An analyst could not learn from the result that those locale tables existed. ## Change and scope - `src/domain/apktool/apktoolResourceAnalysis.ts`: `qualifiedResourceLocale(suffix)` returns the locale of a suffix that combines a locale with other qualifiers. It follows Android's qualifier order, where only MCC and MNC precede the locale. It returns `undefined` for locale-only suffixes (already handled) and for suffixes without a locale (`night`, `v21`, `car-night`, `mcc310`). - `src/apktool/ApktoolProvider.ts`: discovery collects such tables when they contain `strings.xml`, and the result adds the limitation `locales lists values-<locale> tables only; these locale strings tables also carry other configuration qualifiers and are neither listed nor selectable by locale: values-...`. - `docs/apktool-resource-analysis.md`: describes the disclosure. Design choice: `locales` still lists only `values-<locale>` tables, because `locale` selection reads `res/values-<locale>/strings.xml`. Adding `en-rUS` to `locales` when only `values-en-rUS-v21` exists would advertise a locale that selection cannot return. `coverage` stays `complete`: every table that `locales` is documented to list is listed, and the qualified ones are disclosed. Making qualified tables selectable (or adding a structured field) is a possible follow-up that would change the output contract, so I left it out. ## Contract and boundary impact - Semantic owner and earliest changed stage: Apktool decoded-workspace locale discovery. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix within existing boundary). - CLI and MCP/tool-catalog contract: output schema unchanged; one new limitation string when qualified locale tables exist. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: previously undisclosed tables are now named in limitations. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added: - `src/domain/apktool/apktoolResourceAnalysis.test.ts`: `qualifiedResourceLocale` finds `en-rUS`, `b+zh+Hans+CN`, `pt-rBR`, `fr` (`fr-land`, `mcc310-fr`) and `en-rUS` (`mcc310-mnc004-en-rUS-v21`); returns nothing for `de`, `pt-rBR`, `b+sr+Latn`, `night`, `car-night`, `v21`, `sw600dp-land`, `mcc310`. - `src/apktool/ApktoolProvider.test.ts` "names locale tables that also carry other qualifiers (#4612)": the issue's directories plus `mcc310-fr`, `night` and `v21` through the existing Apktool stub; asserts `locales: ["de"]` and the exact limitation. - Remaining proof gaps: the provider tests use POSIX shell stubs and are skipped on Windows, where I developed this, so the provider-level test was not executed locally; it runs in the CI test shards. No real Apktool decode or APK was used (the issue's reproduction is also a stub). - [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 - `npx vitest run src/domain/apktool src/apktool` on Windows 11: 70 passed, 27 skipped (the POSIX-stub provider tests, including the new one). - `npx tsc --noEmit`: exit 0. `npm run verify:test-discovery`: 1500 files, each once. `npm run check:fast` (typecheck, lint, pre-push hook): passed. - `npm run docs:check` could not run here: it requires the packaged Windows native addon, which this checkout lacks. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none; Apktool stub only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: reads the provider-owned decoded workspace only. ## Author readiness Human owner: <!-- @GitHub-handle --> - [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-10-31 01:14:03 +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#6382
No description provided.