fix(contacts): show photos that have no TYPE parameter#5565
Open
MiMoHo wants to merge 2 commits into
Open
Conversation
The TYPE parameter of PHOTO is optional, but getPhotoUrl() called toLowerCase() on it unconditionally. Contacts stored e.g. as 'PHOTO;ENCODING=b:…' (without TYPE) threw a TypeError and showed initials instead of the photo, while other CardDAV clients displayed them fine. Detect the image type from the magic bytes of the base64 data when the TYPE parameter is missing. Unknown signatures fall back to jpeg, which browsers happily content-sniff in an img element anyway. Also log the contact itself in getPhotoUrl() error messages instead of the undefined this.contact. Resolves nextcloud#5401 Assisted-by: Claude:claude-fable-5 Signed-off-by: MiMoHo <37556964+MiMoHo@users.noreply.github.com>
MiMoHo
requested review from
GVodyanov,
SebastianKrupinski and
hamza221
as code owners
July 12, 2026 17:27
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| const signature = Object.keys(signatures).find((signature) => photoB64Data.startsWith(signature)) | ||
| // browsers detect raster images in an img element from the content, | ||
| // so a wrong subtype still renders fine | ||
| return signature ? signatures[signature] : 'jpeg' |
Contributor
There was a problem hiding this comment.
@SebastianKrupinski: I don't know if we should do such a fallback here. I think adding a warning or something like this would make more sense?
Contributor
|
Hey @MiMoHo, thank you very much for your first PR here! Looks pretty good and useful to me. But additional colleagues will have a look into it as well. |
Address review feedback on nextcloud#5565: instead of silently assuming JPEG for a PHOTO whose magic bytes match no known signature, log a warning so the guess is visible in the console. The fallback itself stays, as browsers content-sniff raster images in an img element regardless of the declared subtype. Add a test for the unknown-signature path, closing the coverage gap codecov reported on the fallback branch. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: MiMoHo <37556964+MiMoHo@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Contact photos stored without an explicit
TYPEparameter, e.g.were not displayed:
getPhotoUrl()calledphotoType.toLowerCase()on the missing parameter and threwso the app fell back to initials, while other CardDAV clients (eM Client, iOS Contacts) display these photos fine. The
TYPEparameter is optional per RFC 2426/6350.Changes
TYPEparameter is missing, detect the image type from the magic bytes of the base64 data (png, jpeg, gif, webp, bmp, svg). SVG detected this way still goes through the existingsanitizeSVGpath.jpeg: browsers content-sniff raster images in animgelement, so a wrong subtype still renders — before this change the app just threw.getPhotoUrl()referencedthis.contact, which does not exist on theContactclass and always loggedundefined; they now logthis.Checklist
TYPE, binary photo withoutTYPE(png + jpeg magic bytes), vCard 4.0 data-URI photonpm run lintclean for the touched files, production build succeedsResolve #5401
🤖 Generated with Claude Code