fix(message): fix in-message embedded link validator (WPB-3554) #2088
Conversation
|
Build 1025 failed. |
| return if (containsSchema(normalizedUrl)) { | ||
| normalizedUrl | ||
| } else { | ||
| "https://$normalizedUrl" |
There was a problem hiding this comment.
question: if the link already have something like, "/google.com" then it will turn into "https:///google.com" and it will fail to open, do we need to care about those edge cases ?
There was a problem hiding this comment.
but what if we send [google](/google.com) then it will go https:///google.com as @MohamadJaara mentioned, no? 🤔
There was a problem hiding this comment.
I'd blame whoever pasted the invalid URI there to be honest 😄.
Would be a nice to have for the rare cases where people mistype that hard, but not sure if worth the effort
tmspzz
requested review from
a team,
yamilmedina,
alexandreferris,
borichellow,
MohamadJaara,
saleniuk and
vitorhugods
and removed request for
a team
app/src/main/kotlin/com/wire/android/ui/home/conversations/MessageComposerViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/com/wire/android/ui/home/conversations/MessageComposerViewModel.kt
Outdated
Show resolved
Hide resolved
| return if (containsSchema(normalizedUrl)) { | ||
| normalizedUrl | ||
| } else { | ||
| "https://$normalizedUrl" |
There was a problem hiding this comment.
but what if we send [google](/google.com) then it will go https:///google.com as @MohamadJaara mentioned, no? 🤔
| val regexPattern = Regex(".+://.+") | ||
|
|
||
| return regexPattern.matches(url) |
There was a problem hiding this comment.
Suggestion: leverage already existing functions in the JVM.
| val regexPattern = Regex(".+://.+") | |
| return regexPattern.matches(url) | |
| return try { | |
| URI.create(string).schema != null | |
| } catch (iae: IllegalArgumentException) { | |
| false // invalid URI | |
| } |
There was a problem hiding this comment.
this only validates against valid and known schemes, it's not passing against unknown schemes.
|
APKs built during tests are available here. Scroll down to Artifacts! |
|
Build 1028 succeeded. The build produced the following APK's: |
echoes-hq
bot
added
the
echoes: product-roadmap
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Update the logic to check the embedded link in a message:
1- Show a visit link confirmation dialog when the user click on an embedded link
2- if the link starts with schema [anyString]://[anystring] we don't modify the link
ELSE -> add https:// to the link/text
Testing
Unit tests added
Test Coverage (Optional)
How to Test
Send a message with any combination of links you can imagine:
1- the visit link dialog, always must be appear
2- if the link misses the schema the shown link in the above dialog must start with https
3- if the link starts with schema, the shown link in the dialog must be the same as you posted
4- the app must not crash in any cases
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.