Skip to content

Re-parse anonymous BPD parameter segments when loading a persisted instance - #595

Open
Cosnavel wants to merge 1 commit into
nemiah:masterfrom
Cosnavel:fix/reparse-anonymous-bpd-segments
Open

Cosnavel wants to merge 1 commit into
nemiah:masterfrom
Cosnavel:fix/reparse-anonymous-bpd-segments

Conversation

@Cosnavel

Copy link
Copy Markdown

Problem

persist() stores the BPD with the segments in the shape the then current library version could parse: business transactions it did not implement are kept as AnonymousSegment. A bank only resends the BPD when it bumps its own BPD version (HKVVB announces ours), so after a library upgrade those segments stay anonymous indefinitely.

Concrete case with #574: every connection that already had a persisted instance keeps reporting

The server does not support any DIKKUS versions implemented in this library

from GetCreditCardStatement, although the bank's BPD contains DIKKUS:45:2:3+1+1+0+90:N:J' — until the bank happens to change something in its BPD. The same applies to any segment added in the future (e.g. HIWPDS v6 from #480).

Change

  • BPD::reparseAnonymousSegments() re-parses anonymous parameter segments through Parser::detectAndParseSegment(). AnonymousSegment serializes to the original wire format, so nothing is lost; segments the library still does not implement stay anonymous.
  • FinTs::loadPersistedInstance() calls it after restoring the BPD.

Tests

Tests/Unit/Protocol/BPDReparseTest.php: an anonymous DIKKUS becomes DIKKUSv2 (speicherzeitraum 90) while an unknown segment stays anonymous, and the same through FinTs::new() with a persisted instance.

Note on authorship

Developed with AI assistance (Cursor) and reviewed by the submitting human; the vouching statement per #586 will be added by the author once the review is complete.

…stance

The BPD stored by persist() keep segments in the shape the library version that
received them could parse: business transactions unknown at that time are
AnonymousSegments. Because a bank only resends the BPD when it bumps the BPD version,
those segments stay anonymous across library upgrades, so e.g. DIKKUS (credit card
statements, added in nemiah#574) looks unsupported to getLatestSupportedParameters() on
every connection that already had a persisted instance. Parse such segments again on
load; AnonymousSegment serializes to the original wire format, so nothing is lost.
@Cosnavel

Copy link
Copy Markdown
Author

@nemiah PR ist ready für's Review

@Cosnavel
Cosnavel marked this pull request as ready for review September 15, 2026 14:26
@nemiah

nemiah commented Sep 15, 2026

Copy link
Copy Markdown
Owner

It probably won't break something and doesn't look too verbose for me. I'll merge it if your human takes responsibility

@Philipp91 Philipp91 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

Just for consideration: An alternative solution would be to somehow figure out the library version (maybe it can know its own semver from the deployment somehow?) and persist it as part of the BPD or FinTs instance or so. Then upon re-init, if the persisted version doesn't match the current library version (in either direction), just discard the BPD, so we'll end up requesting it from the server again. This would additionally cover cases where the server sends different BPD when it receives a different request. That said, we haven't changed the request we send for BPD retrieval in many years, so probably this wouldn't matter.

@Cosnavel

Copy link
Copy Markdown
Author

Thanks. I considered version-stamping, but the library can't reliably know its own version today (composer.json still says 4.1.0 on master, and many installs are dev-master#sha), and discarding the BPD costs an extra anonymous dialog per persisted instance after every deploy. The actual gap is narrower: segments are stored as wire text and re-parsed with the current classes on load already; only AnonymousSegment stays anonymous because __unserialize doesn't re-dispatch it — which is exactly what the re-parse closes, offline. If the BPD request itself ever changes incompatibly, the persist format version (2 in loadPersistedInstance()) seems like the right knob: bump it and drop the BPD for older blobs. Happy to do that as a follow-up if the request ever moves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants