Skip to content
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

Special symbols in matrix names #10994

Closed
itay-grudev opened this issue Sep 29, 2019 · 11 comments · Fixed by matrix-org/matrix-react-sdk#11166
Closed

Special symbols in matrix names #10994

itay-grudev opened this issue Sep 29, 2019 · 11 comments · Fixed by matrix-org/matrix-react-sdk#11166
Labels
A-Pills O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@itay-grudev
Copy link

Description

When a user uses special characters in their name it breaks the citing feature. Here is an example of a user that uses two line feed characters in their name which produces the following result in the message input field:

Screenshot from 2019-09-29 12-47-51

And after posting produces the following message:

Screenshot from 2019-09-29 12-48-39

The exact JSON with the user details is the following:

{ 
    "displayname": "\n\nJonas"
}

Interesting note: The API returns the line feeds in the name, but the user claims what they originally inputted was an em-space character. So this is likely the results of an ASCII transformation of the non-ASCII characters (hence his message, intentionally included in the screenshot).

Steps to reproduce

  • Find a user with a hacked name that includes new lines
  • Try and cite them
  • Be miserable for a second hating Riot and Matrix while they are both really great projects.
  • Post the message, then edit it and remove the new lines to produce a nice looking pill.

Version information

riot: 1.4.0
matrix-react-sdk version: 1.6.0
riot-web version: master
olm version: 3.1.0

@weeman1337

This comment was marked as outdated.

@weeman1337 weeman1337 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 16, 2023
@weeman1337 weeman1337 reopened this Mar 16, 2023
@weeman1337
Copy link
Contributor

Still valid

@t3chguy t3chguy added S-Major Severely degrades major functionality or product features, with no satisfactory workaround O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Mar 16, 2023
@Johennes Johennes added S-Minor Impairs non-critical functionality or suitable workarounds exist and removed S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Jun 28, 2023
@Johennes
Copy link
Contributor

I'm downgrading this to minor because not using newlines in your display name feels like an acceptable workaround to me.

@Johennes
Copy link
Contributor

Otherwise I can reproduce the issue. After changing my display name to this

Screenshot 2023-06-28 at 20 10 24

others can still mention me in the composer

Screenshot 2023-06-28 at 20 11 05 Screenshot 2023-06-28 at 20 11 11

but the timeline rendering doesn't display the user pill

Screenshot 2023-06-28 at 20 11 21

@Johennes
Copy link
Contributor

I think part of the issue here is that Markdown doesn't support linebreaks in links without HTML. E.g. you can't do

[a
b](foo.bar)

but you can do

[a<br/>b](foo.bar)

I tried replacing \n with <br/> when serializing the user pill to Markdown in https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/editor/serialize.ts#L51. That actually unbreaks the timeline rendering of pills but the HTML that is serialized from the Markdown+HTML ends up being half-escaped only:

<a href=\"https://matrix.to/#/@johannesm:element.io\">Johannes&lt;br/&gt;<br>Marbach</a>

@Johennes
Copy link
Contributor

Johennes commented Jun 28, 2023

[...] but the HTML that is serialized from the Markdown+HTML ends up being half-escaped only

Actually scratch that. I was missing the g flag on the regex replace. 🤦‍♂️

@Johennes
Copy link
Contributor

I think we would actually want the <br/>s to be unescaped in the HTML though. For this to happen, we'd have to allow them in https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/Markdown.ts#L30. It looks like that might get quite fiddly though because we probably only want to allow them in this edge case.

@Johennes
Copy link
Contributor

In reconsidering, something else we could do is simply strip the newlines from both body and formatted_body. We already ignore them when rendering pills due to our use of textContent.

@t3chguy
Copy link
Member

t3chguy commented Jun 29, 2023

It looks like that might get quite fiddly though because we probably only want to allow them in this edge case.

I don't think that necessarily has to be true. Technically commonmark is a superset of HTML even though we don't treat it as such. We allow certain tags like del u sup sub so adding br wouldn't be a big deal.

image

In reconsidering, something else we could do is simply strip the newlines from both body and formatted_body. We already ignore them when rendering pills due to our use of textContent.

Stripping newlines from body would break paragraphs in plaintext messages, no?

@Johennes
Copy link
Contributor

I don't think that necessarily has to be true. Technically commonmark is a superset of HTML even though we don't treat it as such. We allow certain tags like del u sup sub so adding br wouldn't be a big deal.

Hm, good point. It would just mean that you couldn't send a literal "<br/>" anymore without putting it into backticks. But that's probably not problematic.

Stripping newlines from body would break paragraphs in plaintext messages, no?

Yes sorry, I meant stripping them only within user display name parts.

That being said, since we can easily and generally exclude brs from escaping in our Markdown to HTML conversion, retaining the newlines is probably the more correct fix.

@t3chguy
Copy link
Member

t3chguy commented Jun 29, 2023

without putting it into backticks

or escape it using \

Johennes added a commit to matrix-org/matrix-react-sdk that referenced this issue Jun 29, 2023
Johennes added a commit to matrix-org/matrix-react-sdk that referenced this issue Jun 29, 2023
github-merge-queue bot pushed a commit to matrix-org/matrix-react-sdk that referenced this issue Jul 5, 2023
* Handle newlines in user pills

Fixes: element-hq/element-web#10994

* Fix typo in comment

Co-authored-by: Richard van der Hoff <[email protected]>

* Refactor link generation for better readability

* Use `<br>` instead of `<br/>`

* Fix copy/paste error

---------

Co-authored-by: Richard van der Hoff <[email protected]>
su-ex added a commit to SchildiChat/element-desktop that referenced this issue Feb 24, 2024
* Fixes for [CVE-2023-37259](https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=CVE-2023-37259) / [GHSA-c9vx-2g7w-rp65](GHSA-c9vx-2g7w-rp65)
* Deprecate customisations in favour of Module API ([\#25736](element-hq/element-web#25736)). Fixes element-hq/element-web#25733.
* OIDC: store initial screen in session storage  ([\#25688](element-hq/element-web#25688)). Fixes element-hq/element-web#25656. Contributed by @kerryarchibald.
* Allow default_server_config as a fallback config ([\#25682](element-hq/element-web#25682)). Contributed by @ShadowRZ.
* OIDC: remove auth params from url after login attempt ([\#25664](element-hq/element-web#25664)). Contributed by @kerryarchibald.
* feat(faq): remove keyboard shortcuts button ([\#9342](matrix-org/matrix-react-sdk#9342)). Fixes element-hq/element-web#22625. Contributed by @gefgu.
* GYU: Update banner ([\#11211](matrix-org/matrix-react-sdk#11211)). Fixes element-hq/element-web#25530. Contributed by @justjanne.
* Linkify mxc:// URLs as links to your media repo ([\#11213](matrix-org/matrix-react-sdk#11213)). Fixes element-hq/element-web#6942.
* OIDC: Log in ([\#11199](matrix-org/matrix-react-sdk#11199)). Fixes element-hq/element-web#25657. Contributed by @kerryarchibald.
* Handle all permitted url schemes in linkify ([\#11215](matrix-org/matrix-react-sdk#11215)). Fixes element-hq/element-web#4457 and element-hq/element-web#8720.
* Autoapprove Element Call oidc requests ([\#11209](matrix-org/matrix-react-sdk#11209)). Contributed by @toger5.
* Allow creating knock rooms ([\#11182](matrix-org/matrix-react-sdk#11182)). Contributed by @charlynguyen.
* Expose and pre-populate thread ID in devtools dialog ([\#10953](matrix-org/matrix-react-sdk#10953)).
* Hide URL preview if it will be empty ([\#9029](matrix-org/matrix-react-sdk#9029)).
* Change wording from avatar to profile picture ([\#7015](matrix-org/matrix-react-sdk#7015)). Fixes element-hq/element-meta#1331. Contributed by @aaronraimist.
* Quick and dirty devtool to explore state history ([\#11197](matrix-org/matrix-react-sdk#11197)).
* Consider more user inputs when calculating zxcvbn score ([\#11180](matrix-org/matrix-react-sdk#11180)).
* GYU: Account Notification Settings ([\#11008](matrix-org/matrix-react-sdk#11008)). Fixes element-hq/element-web#24567. Contributed by @justjanne.
* Compound Typography pass ([\#11103](matrix-org/matrix-react-sdk#11103)). Fixes element-hq/element-web#25548.
* OIDC: navigate to authorization endpoint ([\#11096](matrix-org/matrix-react-sdk#11096)). Fixes element-hq/element-web#25574. Contributed by @kerryarchibald.
* Fix read receipt sending behaviour around thread roots ([\#3600](matrix-org/matrix-js-sdk#3600)).
* Fix missing metaspace notification badges ([\#11269](matrix-org/matrix-react-sdk#11269)). Fixes element-hq/element-web#25679.
* Make checkboxes less rounded ([\#11224](matrix-org/matrix-react-sdk#11224)). Contributed by @andybalaam.
* GYU: Fix issues with audible keywords without activated mentions ([\#11218](matrix-org/matrix-react-sdk#11218)). Contributed by @justjanne.
* PosthogAnalytics unwatch settings on logout ([\#11207](matrix-org/matrix-react-sdk#11207)). Fixes element-hq/element-web#25703.
* Avoid trying to set room account data for pinned events as guest ([\#11216](matrix-org/matrix-react-sdk#11216)). Fixes element-hq/element-web#6300.
* GYU: Disable sound for DMs checkbox when DM notifications are disabled ([\#11210](matrix-org/matrix-react-sdk#11210)). Contributed by @justjanne.
* force to allow calls without video and audio in embedded mode ([\#11131](matrix-org/matrix-react-sdk#11131)). Contributed by @EnricoSchw.
* Fix room tile text clipping ([\#11196](matrix-org/matrix-react-sdk#11196)). Fixes element-hq/element-web#25718.
* Handle newlines in user pills ([\#11166](matrix-org/matrix-react-sdk#11166)). Fixes element-hq/element-web#10994.
* Limit width of user menu in space panel ([\#11192](matrix-org/matrix-react-sdk#11192)). Fixes element-hq/element-web#22627.
* Add isLocation to ComposerEvent analytics events ([\#11187](matrix-org/matrix-react-sdk#11187)). Contributed by @andybalaam.
* Fix: hide unsupported login elements ([\#11185](matrix-org/matrix-react-sdk#11185)). Fixes element-hq/element-web#25711. Contributed by @kerryarchibald.
* Scope smaller font size to user info panel ([\#11178](matrix-org/matrix-react-sdk#11178)). Fixes element-hq/element-web#25683.
* Apply i18n to strings in the html export ([\#11176](matrix-org/matrix-react-sdk#11176)).
* Inhibit url previews on MXIDs containing slashes same as those without ([\#11160](matrix-org/matrix-react-sdk#11160)).
* Make event info size consistent with state events ([\#11181](matrix-org/matrix-react-sdk#11181)).
* Fix markdown content spacing ([\#11177](matrix-org/matrix-react-sdk#11177)). Fixes element-hq/element-web#25685.
* Fix font-family definition for emojis ([\#11170](matrix-org/matrix-react-sdk#11170)). Fixes element-hq/element-web#25686.
* Fix spurious error sending receipt in thread errors ([\#11157](matrix-org/matrix-react-sdk#11157)).
* Consider the empty push rule actions array equiv to deprecated dont_notify ([\#11155](matrix-org/matrix-react-sdk#11155)). Fixes element-hq/element-web#25674.
* Only trap escape key for cancel reply if there is a reply ([\#11140](matrix-org/matrix-react-sdk#11140)). Fixes element-hq/element-web#25640.
* Update linkify to 4.1.1 ([\#11132](matrix-org/matrix-react-sdk#11132)). Fixes element-hq/element-web#23806.
su-ex added a commit to SchildiChat/element-web that referenced this issue Feb 24, 2024
* Fixes for [CVE-2023-37259](https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=CVE-2023-37259) / [GHSA-c9vx-2g7w-rp65](GHSA-c9vx-2g7w-rp65)
* Deprecate customisations in favour of Module API ([\element-hq#25736](element-hq#25736)). Fixes element-hq#25733.
* OIDC: store initial screen in session storage  ([\element-hq#25688](element-hq#25688)). Fixes element-hq#25656. Contributed by @kerryarchibald.
* Allow default_server_config as a fallback config ([\element-hq#25682](element-hq#25682)). Contributed by @ShadowRZ.
* OIDC: remove auth params from url after login attempt ([\element-hq#25664](element-hq#25664)). Contributed by @kerryarchibald.
* feat(faq): remove keyboard shortcuts button ([\element-hq#9342](matrix-org/matrix-react-sdk#9342)). Fixes element-hq#22625. Contributed by @gefgu.
* GYU: Update banner ([\element-hq#11211](matrix-org/matrix-react-sdk#11211)). Fixes element-hq#25530. Contributed by @justjanne.
* Linkify mxc:// URLs as links to your media repo ([\element-hq#11213](matrix-org/matrix-react-sdk#11213)). Fixes element-hq#6942.
* OIDC: Log in ([\element-hq#11199](matrix-org/matrix-react-sdk#11199)). Fixes element-hq#25657. Contributed by @kerryarchibald.
* Handle all permitted url schemes in linkify ([\element-hq#11215](matrix-org/matrix-react-sdk#11215)). Fixes element-hq#4457 and element-hq#8720.
* Autoapprove Element Call oidc requests ([\element-hq#11209](matrix-org/matrix-react-sdk#11209)). Contributed by @toger5.
* Allow creating knock rooms ([\#11182](matrix-org/matrix-react-sdk#11182)). Contributed by @charlynguyen.
* Expose and pre-populate thread ID in devtools dialog ([\element-hq#10953](matrix-org/matrix-react-sdk#10953)).
* Hide URL preview if it will be empty ([\element-hq#9029](matrix-org/matrix-react-sdk#9029)).
* Change wording from avatar to profile picture ([\element-hq#7015](matrix-org/matrix-react-sdk#7015)). Fixes element-hq/element-meta#1331. Contributed by @aaronraimist.
* Quick and dirty devtool to explore state history ([\element-hq#11197](matrix-org/matrix-react-sdk#11197)).
* Consider more user inputs when calculating zxcvbn score ([\element-hq#11180](matrix-org/matrix-react-sdk#11180)).
* GYU: Account Notification Settings ([\element-hq#11008](matrix-org/matrix-react-sdk#11008)). Fixes element-hq#24567. Contributed by @justjanne.
* Compound Typography pass ([\element-hq#11103](matrix-org/matrix-react-sdk#11103)). Fixes element-hq#25548.
* OIDC: navigate to authorization endpoint ([\#11096](matrix-org/matrix-react-sdk#11096)). Fixes element-hq#25574. Contributed by @kerryarchibald.
* Fix read receipt sending behaviour around thread roots ([\element-hq#3600](matrix-org/matrix-js-sdk#3600)).
* Fix missing metaspace notification badges ([\element-hq#11269](matrix-org/matrix-react-sdk#11269)). Fixes element-hq#25679.
* Make checkboxes less rounded ([\element-hq#11224](matrix-org/matrix-react-sdk#11224)). Contributed by @andybalaam.
* GYU: Fix issues with audible keywords without activated mentions ([\element-hq#11218](matrix-org/matrix-react-sdk#11218)). Contributed by @justjanne.
* PosthogAnalytics unwatch settings on logout ([\element-hq#11207](matrix-org/matrix-react-sdk#11207)). Fixes element-hq#25703.
* Avoid trying to set room account data for pinned events as guest ([\element-hq#11216](matrix-org/matrix-react-sdk#11216)). Fixes element-hq#6300.
* GYU: Disable sound for DMs checkbox when DM notifications are disabled ([\element-hq#11210](matrix-org/matrix-react-sdk#11210)). Contributed by @justjanne.
* force to allow calls without video and audio in embedded mode ([\element-hq#11131](matrix-org/matrix-react-sdk#11131)). Contributed by @EnricoSchw.
* Fix room tile text clipping ([\element-hq#11196](matrix-org/matrix-react-sdk#11196)). Fixes element-hq#25718.
* Handle newlines in user pills ([\element-hq#11166](matrix-org/matrix-react-sdk#11166)). Fixes element-hq#10994.
* Limit width of user menu in space panel ([\element-hq#11192](matrix-org/matrix-react-sdk#11192)). Fixes element-hq#22627.
* Add isLocation to ComposerEvent analytics events ([\element-hq#11187](matrix-org/matrix-react-sdk#11187)). Contributed by @andybalaam.
* Fix: hide unsupported login elements ([\element-hq#11185](matrix-org/matrix-react-sdk#11185)). Fixes element-hq#25711. Contributed by @kerryarchibald.
* Scope smaller font size to user info panel ([\element-hq#11178](matrix-org/matrix-react-sdk#11178)). Fixes element-hq#25683.
* Apply i18n to strings in the html export ([\element-hq#11176](matrix-org/matrix-react-sdk#11176)).
* Inhibit url previews on MXIDs containing slashes same as those without ([\element-hq#11160](matrix-org/matrix-react-sdk#11160)).
* Make event info size consistent with state events ([\element-hq#11181](matrix-org/matrix-react-sdk#11181)).
* Fix markdown content spacing ([\element-hq#11177](matrix-org/matrix-react-sdk#11177)). Fixes element-hq#25685.
* Fix font-family definition for emojis ([\element-hq#11170](matrix-org/matrix-react-sdk#11170)). Fixes element-hq#25686.
* Fix spurious error sending receipt in thread errors ([\element-hq#11157](matrix-org/matrix-react-sdk#11157)).
* Consider the empty push rule actions array equiv to deprecated dont_notify ([\element-hq#11155](matrix-org/matrix-react-sdk#11155)). Fixes element-hq#25674.
* Only trap escape key for cancel reply if there is a reply ([\element-hq#11140](matrix-org/matrix-react-sdk#11140)). Fixes element-hq#25640.
* Update linkify to 4.1.1 ([\element-hq#11132](matrix-org/matrix-react-sdk#11132)). Fixes element-hq#23806.
su-ex added a commit to SchildiChat/matrix-react-sdk that referenced this issue Feb 24, 2024
* Fixes for [CVE-2023-37259](https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=CVE-2023-37259) / [GHSA-c9vx-2g7w-rp65](GHSA-c9vx-2g7w-rp65)
* GYU: Update banner ([\matrix-org#11211](matrix-org#11211)). Fixes element-hq/element-web#25530. Contributed by @justjanne.
* Linkify mxc:// URLs as links to your media repo ([\matrix-org#11213](matrix-org#11213)). Fixes element-hq/element-web#6942.
* OIDC: Log in ([\matrix-org#11199](matrix-org#11199)). Fixes element-hq/element-web#25657. Contributed by @kerryarchibald.
* Handle all permitted url schemes in linkify ([\matrix-org#11215](matrix-org#11215)). Fixes element-hq/element-web#4457 and element-hq/element-web#8720.
* Autoapprove Element Call oidc requests ([\matrix-org#11209](matrix-org#11209)). Contributed by @toger5.
* Allow creating knock rooms ([\matrix-org#11182](matrix-org#11182)). Contributed by @charlynguyen.
* feat(faq): remove keyboard shortcuts button ([\matrix-org#9342](matrix-org#9342)). Fixes element-hq/element-web#22625. Contributed by @gefgu.
* Expose and pre-populate thread ID in devtools dialog ([\matrix-org#10953](matrix-org#10953)).
* Hide URL preview if it will be empty ([\matrix-org#9029](matrix-org#9029)).
* Change wording from avatar to profile picture ([\matrix-org#7015](matrix-org#7015)). Fixes element-hq/element-meta#1331. Contributed by @aaronraimist.
* Quick and dirty devtool to explore state history ([\matrix-org#11197](matrix-org#11197)).
* Consider more user inputs when calculating zxcvbn score ([\matrix-org#11180](matrix-org#11180)).
* GYU: Account Notification Settings ([\matrix-org#11008](matrix-org#11008)). Fixes element-hq/element-web#24567. Contributed by @justjanne.
* Compound Typography pass ([\matrix-org#11103](matrix-org#11103)). Fixes element-hq/element-web#25548.
* OIDC: navigate to authorization endpoint ([\matrix-org#11096](matrix-org#11096)). Fixes element-hq/element-web#25574. Contributed by @kerryarchibald.
* Fix missing metaspace notification badges ([\matrix-org#11269](matrix-org#11269)). Fixes element-hq/element-web#25679.
* Make checkboxes less rounded ([\matrix-org#11224](matrix-org#11224)). Contributed by @andybalaam.
* GYU: Fix issues with audible keywords without activated mentions ([\matrix-org#11218](matrix-org#11218)). Contributed by @justjanne.
* PosthogAnalytics unwatch settings on logout ([\matrix-org#11207](matrix-org#11207)). Fixes element-hq/element-web#25703.
* Avoid trying to set room account data for pinned events as guest ([\matrix-org#11216](matrix-org#11216)). Fixes element-hq/element-web#6300.
* GYU: Disable sound for DMs checkbox when DM notifications are disabled ([\matrix-org#11210](matrix-org#11210)). Contributed by @justjanne.
* force to allow calls without video and audio in embedded mode ([\matrix-org#11131](matrix-org#11131)). Contributed by @EnricoSchw.
* Fix room tile text clipping ([\matrix-org#11196](matrix-org#11196)). Fixes element-hq/element-web#25718.
* Handle newlines in user pills ([\matrix-org#11166](matrix-org#11166)). Fixes element-hq/element-web#10994.
* Limit width of user menu in space panel ([\matrix-org#11192](matrix-org#11192)). Fixes element-hq/element-web#22627.
* Add isLocation to ComposerEvent analytics events ([\matrix-org#11187](matrix-org#11187)). Contributed by @andybalaam.
* Fix: hide unsupported login elements ([\matrix-org#11185](matrix-org#11185)). Fixes element-hq/element-web#25711. Contributed by @kerryarchibald.
* Scope smaller font size to user info panel ([\matrix-org#11178](matrix-org#11178)). Fixes element-hq/element-web#25683.
* Apply i18n to strings in the html export ([\matrix-org#11176](matrix-org#11176)).
* Inhibit url previews on MXIDs containing slashes same as those without ([\matrix-org#11160](matrix-org#11160)).
* Make event info size consistent with state events ([\matrix-org#11181](matrix-org#11181)).
* Fix markdown content spacing ([\matrix-org#11177](matrix-org#11177)). Fixes element-hq/element-web#25685.
* Fix font-family definition for emojis ([\matrix-org#11170](matrix-org#11170)). Fixes element-hq/element-web#25686.
* Fix spurious error sending receipt in thread errors ([\matrix-org#11157](matrix-org#11157)).
* Consider the empty push rule actions array equiv to deprecated dont_notify ([\matrix-org#11155](matrix-org#11155)). Fixes element-hq/element-web#25674.
* Only trap escape key for cancel reply if there is a reply ([\matrix-org#11140](matrix-org#11140)). Fixes element-hq/element-web#25640.
* Update linkify to 4.1.1 ([\matrix-org#11132](matrix-org#11132)). Fixes element-hq/element-web#23806.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Pills O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants