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

fix(chat): fix not escaping for inline codeblocks and url parsing issues #1782

Merged
merged 18 commits into from
Feb 6, 2024

Conversation

Flemmli97
Copy link
Collaborator

@Flemmli97 Flemmli97 commented Jan 30, 2024

What this PR does 📖

  • Fix escaping issue with inline codeblocks
  • Fix url parsing
  • URL and Image markdown are now ignored as we do links separate from markdown

Which issue(s) this PR fixes 🔨

@github-actions github-actions bot added Don't merge yet DO NOT MERGE Missing dev review Still needs to be reviewed by a dev linter failing Cargo Workflow (linter) failed on this PR labels Jan 30, 2024
Copy link
Member

@stavares843 stavares843 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first thing in the ticket is fixed

Captura de ecrã 2024-01-30, às 19 09 06

but the second isn't

sending

<img src=x onerror=alert(1)" onmouseover="alert(2)">

still shows this
Captura de ecrã 2024-01-30, às 19 09 00

macOS, m1

@stavares843 stavares843 added the Changes requested When other dev or QA request a change label Jan 30, 2024
Copy link
Contributor

github-actions bot commented Jan 30, 2024

UI Automated Test Results Summary for MacOS/Windows

500 tests   448 ✅  2h 15m 3s ⏱️
 42 suites   52 💤
  3 files      0 ❌

Results for commit a121413.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 30, 2024

UI Automated Tests execution is complete! You can find the test results report here

@github-actions github-actions bot added the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Jan 31, 2024
@github-actions github-actions bot removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Jan 31, 2024
@Flemmli97 Flemmli97 removed Changes requested When other dev or QA request a change linter failing Cargo Workflow (linter) failed on this PR labels Feb 1, 2024
@github-actions github-actions bot added the linter failing Cargo Workflow (linter) failed on this PR label Feb 1, 2024
@github-actions github-actions bot added missing fixing conflict A conflict exists in current PR and needs resolution and removed linter failing Cargo Workflow (linter) failed on this PR labels Feb 1, 2024
@stavares843 stavares843 removed the missing fixing conflict A conflict exists in current PR and needs resolution label Feb 1, 2024
@Flemmli97 Flemmli97 changed the title fix(chat): fix not escaping for inline codeblocks fix(chat): fix not escaping for inline codeblocks and url parsing issues Feb 1, 2024
@stavares843
Copy link
Member

Captura de ecrã 2024-02-01, às 18 18 01

when sending this

![Image](data:text/html;base64,PHNjcmlwdD5hbGVydCgiSGVsbG8sIFdvcmxkISIpPC9zY3JpcHQ+Cg==)

it appears clickable on the preview but then when sending it isnt

Captura de ecrã 2024-02-01, às 18 21 18

this is on dev

so im not saying it should be on dev in this case, but if is not gonna be clickable when sending should not be clickable in the preview

@stavares843
Copy link
Member

this branch

Captura de ecrã 2024-02-01, às 18 18 20

vs dev

Captura de ecrã 2024-02-01, às 18 22 19

content

[Click me](javascript:window.location='http://malicious-site.com')

should not be displaying this

Captura de ecrã 2024-02-01, às 18 24 19

@Flemmli97
Copy link
Collaborator Author

sending this

# Extended Markdown Test

## Table of Contents

- [Headers](#headers)
- [Emphasis](#emphasis)
- [Lists](#lists)
- [Links](#links)
- [Images](#images)
- [Blockquotes](#blockquotes)
- [Code](#code)
- [Horizontal Rule](#horizontal-rule)
- [Tables](#tables)
- [Task Lists](#task-lists)
- [Definition Lists](#definition-lists)
- [Code Blocks](#code-blocks)
- [Strikethrough](#strikethrough)
- [Fenced Code Blocks with Language Specification](#fenced-code-blocks-with-language-specification)
- [Footnotes](#footnotes)
- [Superscript and Subscript](#superscript-and-subscript)
- [Emoji](#emoji)
- [LaTeX Math](#latex-math)

this branch

Captura de ecrã 2024-02-01, às 18 14 13 vs dev Captura de ecrã 2024-02-01, às 18 14 57

idk if we want it though? isnt that basically a hidden link?

@stavares843
Copy link
Member

stavares843 commented Feb 1, 2024

i get your point, fine with it

i mean that is useful for things like github and what not but on uplink the link is not gonna work so we can ignore that

@luisecm
Copy link
Contributor

luisecm commented Feb 1, 2024

sending www.google.pt

this branch

Captura de ecrã 2024-02-01, às 17 02 14 vs dev Captura de ecrã 2024-02-01, às 17 02 58

This one specifically is affecting the CI automation tests:

User A sends a message with www.apple.com
User B receives a message with www.apple.com text and not an attached link

Chat User - Chat Messages containing links contents on local side - Failed

@stavares843
Copy link
Member

thanks for checking @luisecm 🔨

@Flemmli97 Flemmli97 removed the Changes requested When other dev or QA request a change label Feb 2, 2024
@github-actions github-actions bot removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Feb 2, 2024
@stavares843
Copy link
Member

Captura de ecrã 2024-02-01, às 18 18 01

when sending this

![Image](data:text/html;base64,PHNjcmlwdD5hbGVydCgiSGVsbG8sIFdvcmxkISIpPC9zY3JpcHQ+Cg==)

it appears clickable on the preview but then when sending it isnt

Captura de ecrã 2024-02-01, às 18 21 18

this is on dev

so im not saying it should be on dev in this case, but if is not gonna be clickable when sending should not be clickable in the preview

@stavares843 stavares843 added the Changes requested When other dev or QA request a change label Feb 2, 2024
@Flemmli97
Copy link
Collaborator Author

Captura de ecrã 2024-02-01, às 18 18 01 when sending this
![Image](data:text/html;base64,PHNjcmlwdD5hbGVydCgiSGVsbG8sIFdvcmxkISIpPC9zY3JpcHQ+Cg==)

it appears clickable on the preview but then when sending it isnt

Captura de ecrã 2024-02-01, às 18 21 18 this is on dev

so im not saying it should be on dev in this case, but if is not gonna be clickable when sending should not be clickable in the preview

would create a new issue for that. as that problem is caused by the markdown preview js library

@stavares843
Copy link
Member

sounds good, thanks! 🔨

@stavares843 stavares843 removed the Changes requested When other dev or QA request a change label Feb 2, 2024
@phillsatellite phillsatellite added the QA Tested QA has tested and approved label Feb 2, 2024
@dariusc93 dariusc93 added Ready to Merge This PR is ready to merge and removed Missing dev review Still needs to be reviewed by a dev Don't merge yet DO NOT MERGE labels Feb 6, 2024
@stavares843 stavares843 merged commit def3a6d into dev Feb 6, 2024
11 checks passed
@stavares843 stavares843 deleted the inline_code_escape_fix branch February 6, 2024 15:15
@github-actions github-actions bot removed QA Tested QA has tested and approved Ready to Merge This PR is ready to merge labels Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants