morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4964] [Bug] XML plist decoder treats processing instructions as container values #3074

New issue

Closed

opened 2026-10-31 00:35:59 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-31 00:35:59 +00:00

Owner

Originally created by @Mangolycheematcha on GitHub (Oct 18, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4964

Before submitting

Searched open and closed issues and pull requests for the same behavior.
Removed credentials, private paths, and sensitive target data. All samples are authored fixtures.

Area

Evidence, artifacts, or reconstruction

Observed behavior

The shared XML plist decoder treats processing instructions between container members as values. For example:

<plist version="1.0"><?rea ignored?><string>demo</string></plist>

The decoder reports Invalid PLIST tag rea instead of returning "demo". Instructions between array entries or between a dictionary key and its value cause the same class of failure. An otherwise readable app Info.plist can therefore fail target resolution.

Reproduction and environment

Baseline: 6b180783ed04ebc87548c8385c80e05725804c47 (source version 6.6.0). Windows x64, Node 24.18.0, npm 11.16.0.

Added regression cases and executed:

npm run test:local -- src/domain/propertyListXml.test.ts src/application/AppBundleExecutable.test.ts

On the unchanged implementation, five new cases failed and 35 controls passed. The cases cover plist/array/dict member positions and app resolution through actual temporary macOS-style and flat iOS-style bundle directories on Windows. The fixture program files contain inert text and are never executed.

Expected behavior

Ignore processing-instruction nodes between plist/container members. Preserve existing scalar-content handling and malformed-XML rejection.

Apple's getContentObject implementation explicitly skips XML processing instructions before reading the next object. Python 3.12.14 plistlib.loads also accepts the matching root, array, and dictionary fixtures. macOS plutil was not run.

Impact and scope

Observed: deterministic failures on authored fixtures in the shared decoder and app executable resolver. Production-file prevalence is NOT_ENOUGH_INFO; no production-frequency or performance claim is made.

Analysis

src/domain/propertyListXml.ts filters text, CDATA, and comments in members(), but passes processing-instruction nodes to decode(). Fixing that shared boundary avoids consumer-specific workarounds. This is separate from #2967 (preflight scanning), #1843 (dictionary-key text), and #4714 (malformed XML admission).

Originally created by @Mangolycheematcha on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4964 ### Before submitting - [x] Searched open and closed issues and pull requests for the same behavior. - [x] Removed credentials, private paths, and sensitive target data. All samples are authored fixtures. ### Area Evidence, artifacts, or reconstruction ### Observed behavior The shared XML plist decoder treats processing instructions between container members as values. For example: ```xml <plist version="1.0"><?rea ignored?><string>demo</string></plist> ``` The decoder reports `Invalid PLIST tag rea` instead of returning `"demo"`. Instructions between array entries or between a dictionary key and its value cause the same class of failure. An otherwise readable app `Info.plist` can therefore fail target resolution. ### Reproduction and environment Baseline: `6b180783ed04ebc87548c8385c80e05725804c47` (source version 6.6.0). Windows x64, Node 24.18.0, npm 11.16.0. Added regression cases and executed: ```sh npm run test:local -- src/domain/propertyListXml.test.ts src/application/AppBundleExecutable.test.ts ``` On the unchanged implementation, five new cases failed and 35 controls passed. The cases cover plist/array/dict member positions and app resolution through actual temporary macOS-style and flat iOS-style bundle directories on Windows. The fixture program files contain inert text and are never executed. ### Expected behavior Ignore processing-instruction nodes between plist/container members. Preserve existing scalar-content handling and malformed-XML rejection. Apple's [getContentObject implementation](https://github.com/apple-oss-distributions/CF/blob/5b29c619aaf0cf1042bcfefc4bb10ef15b89dc11/CFPropertyList.c#L911) explicitly skips XML processing instructions before reading the next object. Python 3.12.14 `plistlib.loads` also accepts the matching root, array, and dictionary fixtures. macOS `plutil` was not run. ### Impact and scope Observed: deterministic failures on authored fixtures in the shared decoder and app executable resolver. Production-file prevalence is NOT_ENOUGH_INFO; no production-frequency or performance claim is made. ### Analysis `src/domain/propertyListXml.ts` filters text, CDATA, and comments in `members()`, but passes processing-instruction nodes to `decode()`. Fixing that shared boundary avoids consumer-specific workarounds. This is separate from #2967 (preflight scanning), #1843 (dictionary-key text), and #4714 (malformed XML admission).

bsakaguchi closed this issue

2026-10-31 00:35:59 +00:00

bsakaguchi commented

2026-10-31 00:36:00 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 18, 2026):

Root-cause analysis is sound — filtering at the members() boundary in propertyListXml.ts is the right layer, and it keeps the fix consistent with how text/CDATA/comments are already handled rather than adding consumer-specific workarounds. Cross-checking against Apple's getContentObject and Python plistlib strengthens the expected behavior. Two small suggestions: (1) scope the skip to processing instructions that appear between members, so a document-level PI (e.g. the XML declaration) or one legitimately carried in content isn't silently swallowed — the fixtures already cover root/array/dict positions, worth adding one asserting a PI before the plist root doesn't break the decoder; (2) keep the "ignore, don't error" policy uniform with comment handling so malformed-XML rejection (#4714) stays untouched. The five failing cases plus 35 passing controls on the unchanged baseline look like a clean regression set.

@XinwangTao commented on GitHub (Oct 18, 2026): Root-cause analysis is sound — filtering at the members() boundary in propertyListXml.ts is the right layer, and it keeps the fix consistent with how text/CDATA/comments are already handled rather than adding consumer-specific workarounds. Cross-checking against Apple's getContentObject and Python plistlib strengthens the expected behavior. Two small suggestions: (1) scope the skip to processing instructions that appear between members, so a document-level PI (e.g. the XML declaration) or one legitimately carried in content isn't silently swallowed — the fixtures already cover root/array/dict positions, worth adding one asserting a PI before the plist root doesn't break the decoder; (2) keep the "ignore, don't error" policy uniform with comment handling so malformed-XML rejection (#4714) stays untouched. The five failing cases plus 35 passing controls on the unchanged baseline look like a clean regression set.

bsakaguchi commented

2026-10-31 00:36:01 +00:00

Author

Owner

@Mangolycheematcha commented on GitHub (Oct 18, 2026):

@XinwangTao Thanks for the review. I added the boundary coverage to #4965 in d34d704.

Four additional test cases cover an XML declaration alongside ordinary processing instructions before and after the plist root, literal PI-shaped text in CDATA and escaped content, and existing numeric-scalar handling with PI/comment markup. All 107 selected tests passed, along with typecheck, lint, formatting, and the commit/push hooks.

The production skip was already confined to container members, so no further decoder change was needed. Scalar-content interpretation remains unchanged, and malformed XML still goes through strict validation before value decoding. The numeric-scalar case protects REA's existing partial/incomplete outcome; it makes no claim about Apple accepting that scalar syntax.

The full Windows-check and actual Apple-runtime coverage limits remain disclosed in the PR.

@Mangolycheematcha commented on GitHub (Oct 18, 2026): @XinwangTao Thanks for the review. I added the boundary coverage to #4965 in [d34d704](https://github.com/Mangolycheematcha/rea/commit/d34d704e11b29870e1412b68c9eb8584fc54c56f). Four additional test cases cover an XML declaration alongside ordinary processing instructions before and after the plist root, literal PI-shaped text in CDATA and escaped content, and existing numeric-scalar handling with PI/comment markup. All 107 selected tests passed, along with typecheck, lint, formatting, and the commit/push hooks. The production skip was already confined to container members, so no further decoder change was needed. Scalar-content interpretation remains unchanged, and malformed XML still goes through strict validation before value decoding. The numeric-scalar case protects REA's existing partial/incomplete outcome; it makes no claim about Apple accepting that scalar syntax. The full Windows-check and actual Apple-runtime coverage limits remain disclosed in the PR.

bsakaguchi referenced this issue

2026-10-31 00:40:20 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425

bsakaguchi referenced this issue

2026-10-31 01:07:50 +00:00

[PR #3074] docs: clarify automated agent development setup #5593

bsakaguchi referenced this issue

2026-11-01 21:10:54 +00:00

[GH-ISSUE #3795] [Tracker] Open documentation issues and pull requests (October 2026) #1468