Skip to content

fix: read base64 encoded values as binary, whatever the property - #798

Open
odolbeau wants to merge 1 commit into
sabre-io:masterfrom
odolbeau:fix/base64-encoding-is-binary
Open

odolbeau wants to merge 1 commit into
sabre-io:masterfrom
odolbeau:fix/base64-encoding-is-binary

Conversation

@odolbeau

@odolbeau odolbeau commented Oct 2, 2026

Copy link
Copy Markdown

The class of a property comes from its VALUE parameter, or else from its name. PHOTO and LOGO map to Binary, but KEY and SOUND map to FlatText. vCard 2.1 and 3.0 embed keys and sounds in base64 too:

KEY;ENCODING=b;TYPE=PGP:LS0tLS1CRUdJTg==
SOUND;ENCODING=BASE64;TYPE=WAVE:UklGRg==

Such properties are read as text. As a result:

  • getValue() returns the base64 string, not the data;
  • converting the vCard to 4.0 leaves KEY:LS0tLS1CRUdJTg==, where VCardConverter would otherwise give a data: URI, as it does for PHOTO and LOGO.

Change

In Document::createProperty(), when no VALUE parameter picks the class, an ENCODING parameter of b or BASE64 selects Property\Binary. An explicit VALUE still comes first.

Serialization is unchanged: Binary writes the value back in base64 with its ENCODING parameter.

Not covered: the nameless BASE64 parameter of vCard 2.1 (KEY;BASE64:…). The MIME-dir parser only adds nameless parameters after creating the property, so its class is already chosen.

Tests

tests/VObject/Base64EncodingTest.php covers a vCard 3.0 KEY (value and round trip), a vCard 2.1 SOUND, the conversion to vCard 4.0, and VALUE taking precedence. Three of its tests fail without the fix.

🤖 Generated with Claude Code

KEY and SOUND map to FlatText, so a base64 encoded key or sound in
vCard 2.1 or 3.0 was read as text: getValue() returned the base64
string, and the conversion to vCard 4.0 did not give a data: URI.
An ENCODING parameter of b or BASE64 now selects Property\Binary when
no VALUE parameter picks the class.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant