-
-
Notifications
You must be signed in to change notification settings - Fork 806
feat: Add XEP-0231 Bits of Binary support #3922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: Add XEP-0231 Bits of Binary support #3922
Conversation
src/headless/plugins/bob/plugin.js
Outdated
| import _converse from '../../shared/_converse.js'; | ||
| import log from '@converse/log'; | ||
|
|
||
| const { Strophe, $iq } = converse.env; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please pay attention to these security bot messages.
jcbrand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your pull request @sreeshanth-soma
You noted that the headless tests failed, but there are also non-headless tests which now fail due to the new urn:xmpp:bob feature supported.
Please fix those as well.
- Add converse-bob plugin for receiving BOB images (custom smileys) - Implements cache, API, IQ handling, and message parsing - Register urn:xmpp:bob disco feature - Add comprehensive test suite (7 tests) - Update capability hashes for new feature
Update XEP-0115 entity capability verification hashes in test files to account for the added urn:xmpp:bob feature. Headless tests now use: 9zuZdgDXVE0fkcAjeaokq9JxHJw= Non-headless tests now use: KjyaPNsNXajTrXVlYimRsIvje2g= RAI tests now use: 3Tday6wfQnY1R8zGPMO3CBYFrik=
06c6529 to
6deb51b
Compare
|
Hello @jcbrand . I've addressed your feedback. The non-headless tests that were failing due to the urn:xmpp:bob feature have been fixed:
|
jcbrand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sreeshanth-soma, please see my review comments.
src/headless/plugins/bob/plugin.js
Outdated
| const MAX_BOB_SIZE = 8192; | ||
|
|
||
| // In-memory cache for BOB data | ||
| const bob_cache = new Map(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced that an in-memory cache is the right way to go here.
Every time you reload the tab the cache will be wiped and then you lose all the images.
If you then try to refetch the images, you might not get all of them due to the sender being offline.
So instead, we should probably create a persistent collection of BOB images, similar to what we do with VCards: https://github.com/conversejs/converse.js/blob/master/src/headless/plugins/vcard/vcards.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I've replaced the in-memory Map with a persistent Skeletor Collection, following the VCards pattern:
- Created bob.js model with isExpired() and getBlobURL() methods.
- Created bobs.js collection using initStorage() for IndexedDB persistence
- Collection is initialized on connected event and cleaned up on clearSession
BOB images now persist across page reloads and remain available even if the sender goes offline.
src/shared/texture/texture.js
Outdated
| * @param {number} offset - The index of the passed in text relative to | ||
| * the start of the message body text. | ||
| */ | ||
| async addBOBImages(text, offset) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function should go into a bob-views plugin and then in that plugin you should listen for the afterMessageBodyTransformed event in order to call this function.
See here:
| api.listen.on('afterMessageBodyTransformed', handleEncryptedFiles); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Created a new bob-views plugin that:
- Listens to afterMessageBodyTransformed event (following the omemo-views pattern)
- Moved the BOB image handling logic from texture.js to the new plugin
- Registered it in src/index.js and added to VIEW_PLUGINS whitelist
This properly separates the headless BOB storage from the view-layer rendering.
src/shared/texture/texture.js
Outdated
| import { until } from "lit/directives/until.js"; | ||
| import { Directive, directive } from "lit/directive.js"; | ||
| import { api, u } from "@converse/headless"; | ||
| import log from "@converse/log"; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
src/headless/plugins/bob/plugin.js
Outdated
| import _converse from '../../shared/_converse.js'; | ||
| import log from '@converse/log'; | ||
|
|
||
| const { Strophe, $iq } = converse.env; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please pay attention to these security bot messages.
…eanup Use configurable max_bob_size setting, add setInterval for expired blob cleanup, and update test fixtures for new caps hashes
Summary
Implements XEP-0231: Bits of Binary, enabling Converse.js to receive and display inline binary data such as custom smileys from clients like Pidgin.
Closes #3611
Implementation
New
converse-bobPluginapi.bob.get(),api.bob.store(),api.bob.has()<iq type="get"><data xmlns="urn:xmpp:bob">elementsurn:xmpp:bobTest Results
TOTAL: 94 SUCCESS
New Tests (7)
Capability Hash
Adding
urn:xmpp:bobupdates XEP-0115 verification string from1T0pIfIxYO645OaT9gpXVXOvb9s=toZXDrJD54GWZYlZif9IuMxum/u+g=