Noticed, while working on moving Drupal.org twitter embed blocks to Mastodon feed blocks. Mastodon feeds do not have item titles, so the description is taken as a fallback. However the description is not stripped of HTML and only the first 40 chars are taken, which is odd given the title could be much longer.
Decode entities and strip description of HTML for the title fallback and truncate at 255 chars similar to how it is done before storage in aggregator_aggregator_process(). Add ellipsis to indicate there was more content. Also use the build in Drupal core features for these instead of custom word boundary matching.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | Screenshot 2023-07-07 at 16.02.51.png | 359.45 KB | gábor hojtsy |
| #3 | Screenshot 2023-07-07 at 15.41.47.png | 255.45 KB | gábor hojtsy |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !4339
Comments
Comment #3
gábor hojtsyAdded screenshot of problems and expanded proposed resolution. Ended up also decoding entities, since those would/will also be escaped when displayed otherwise.
Comment #4
gábor hojtsyComment #5
fjgarlin commentedI reviewed the code in the MR and tested the fix here: https://fjgarlin-drupal.dev.devdrupal.org/security
The screenshots above from the before and after are from that URL, where I tested removing the items and then updating them again so they'd be resaved in the database.
The feeds we tested were https://drupal.community/@drupalsecurity.rss and https://drupal.community/@dropismoving.rss, where no "title" element is present in the items.
Marking RTBC.
Comment #6
poker10 commentedThanks for working on this!
Yes, this seems like a bug (because the resulting text is malformed). The fix looks good, but we are combining it with increasing the title length from 40 to 255 characters. I think we should add at least a short change record for people using this feature (e.g. feeds without titles), so they will not be surprised with longer text. Thanks!
I have tested this manually (comparing the old and new code for parsing the description) and it seems good. For example this text:
will change from this:
<p>Search Autocomplete -to this:
Search Autocomplete - Moderately critical - Cross Site Scripting - SA-CONTRIB-2023-026 3371114 at https://www.drupal.orgwhen using the patch.
Comment #7
poker10 commentedI have drafted a change record: https://www.drupal.org/node/3383554 . Feel free to update if needed.
Comment #9
mcdruid commentedAdded a test for this.
Assuming the tests all pass, I'm happy for this to be committed.
Comment #10
poker10 commentedThanks for adding the test @mcdruid! It is green, so I think we are good to go here.
Comment #12
poker10 commentedCommitted, thanks all!