Load avatar data over HTTP if available in vCard EXTVAL #3782
Conversation
| fullname: iq.querySelector(':scope > vCard FN')?.textContent, | ||
| image: iq.querySelector(':scope > vCard PHOTO BINVAL')?.textContent, | ||
| image_type: iq.querySelector(':scope > vCard PHOTO TYPE')?.textContent, | ||
| image_url: iq.querySelector(':scope > vCard PHOTO EXTVAL')?.textContent, |
There was a problem hiding this comment.
XEP-0054 and XEP-0292 both support PHOTO EXTVAL but funnily enough XEP-0153 VCard-based Avatars discourages it. I'm not sure why.
The <PHOTO/> element SHOULD NOT contain an <EXTVAL/> that points to a URI for the image file.
In any case, the changes look OK to me, but I think we should put result.image first in the if statement below, and then check for result.image_url.
There was a problem hiding this comment.
XEP-0054 and XEP-0292 both support
PHOTO EXTVALbut funnily enough XEP-0153 VCard-based Avatars discourages it. I'm not sure why.
I guess it is to prevent IP leaking. An attacker could use this field to point to their server, and get other users IP. Which could be an issue when you have high privacy expectation.
There was a problem hiding this comment.
Perhaps, but this is the same with HTTP file upload.
My guess is it's because that XEP deals with creating hashes of BINVAL values and advertising those in order to let clients know when they need to update the VCard.
When you're using a URL, presumably you'd change the URL when there's a new avatar so the usage of a hash is not necessary (but then you'd ideally advertise the new image URL similarly to how the hash is advertised).
There was a problem hiding this comment.
Perhaps, but this is the same with HTTP file upload.
yes, and that's why some clients don't display images automatically.
My guess is it's because that XEP deals with creating hashes of BINVAL values and advertising those in order to let clients know when they need to update the VCard.
When you're using a URL, presumably you'd change the URL when there's a new avatar so the usage of a hash is not necessary (but then you'd ideally advertise the new image URL similarly to how the hash is advertised)
Could be because of this too, indeed.
There was a problem hiding this comment.
yes, and that's why some clients don't display images automatically.
This is actually a good feature which I'd like to have in Converse as well.
There was a problem hiding this comment.
I didn't see an issue with IP leaks since many clients already block untrusted domains and i'd assume deployments of Converse would set up content security policy rules. I'm not sure if the same applies with Converse Desktop so that would have to be checked first.
If CSP isn't enough then creating some configuration to allow trusted domains for avatars can be created.
|
Added new changes. |
|
Thanks @edshot99. Can you please rebase your branch onto the latest Also, can you please add a test here: It can be similar to the To run that test, you can run |
edshot99
force-pushed
the
http-avatars
branch
from
aa529bf to
917446a
Compare
|
I'm still having issues with the tests. Specifically the vCard removal test so i'm just going to put this on hold until I get that sorted out when I have time. |
|
@edshot99 If you commit the tests to this PR, then I can take a look at what you've done so far and perhaps be able to help unblock you. |
3 participants
Its possible to point the user avatar to a URL address and store it in PEP. It then becomes accessible from vcard-temp in the EXTVAL tag.
The same can be done with MUC avatars by setting the EXTVAL tag with the url, but this is not oficially supported by XEP-0486.
Edit: I think handleVCardUpdatePresence() in src/headless/plugins/vcard/utils.js should be modified, but i'm not sure if its only needed for the binary avatars or not since i'm not too familiar with Javascript or Sizzle. Seems to function fine without any changes to it however.