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

Content safety #5486

Merged
merged 2 commits into from
Jun 25, 2021
Merged

Content safety #5486

merged 2 commits into from
Jun 25, 2021

Conversation

enahum
Copy link
Contributor

@enahum enahum commented Jun 25, 2021

Summary

This PR adds some safety checks around post message attachments and app bindings to prevent the app from crashing when null is present in the post metadata for some reason.

Also includes a fix for YouTube video playback (for example this one https://www.youtube.com/watch?v=eWCXZIRm7iU&t=707). Fix taken from 0xced/XCDYouTubeKit#534

Release Note

* Prevented crash when message attachments or app bindings are `null`
* Fixed iOS YouTube video playback

@enahum enahum added 2: Dev Review Requires review by a core commiter CherryPick/Approved Meant for the quality or patch release tracked in the milestone 3: QA Review Requires review by a QA tester labels Jun 25, 2021
@enahum enahum added this to the v1.44.0 milestone Jun 25, 2021
@migbot migbot removed the 2: Dev Review Requires review by a core commiter label Jun 25, 2021
@lindalumitchell lindalumitchell added the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 25, 2021
@mattermod
Copy link
Contributor

Building app in separate branch.

@mattermod mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Jun 25, 2021
@lindalumitchell lindalumitchell requested review from lindalumitchell and removed request for josephbaylon June 25, 2021 18:20
Copy link
Contributor

@lindalumitchell lindalumitchell left a comment

Choose a reason for hiding this comment

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

Verified on test builds on Android and iOS (13.7 on iPad and 14.3 on iPhone 7) that the YouTube link opens the video as expected. LGTM.

Note: Created https://mattermost.atlassian.net/browse/MM-36688 for the issue.

@enahum enahum removed the 3: QA Review Requires review by a QA tester label Jun 25, 2021
@enahum enahum merged commit 47d081b into master Jun 25, 2021
@mattermod
Copy link
Contributor

Cherry pick is scheduled.

@enahum enahum deleted the content-safety branch June 25, 2021 18:23
@mattermod
Copy link
Contributor

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
Failed to add the RSA host key for IP address '140.82.113.3' to the list of known hosts (/app/.ssh/known_hosts).
From github.com:mattermost/mattermost-mobile
 * [new branch]          MM-17135              -> upstream/MM-17135
 * [new branch]          MM-28880              -> upstream/MM-28880
 * [new branch]          MM-34619              -> upstream/MM-34619
 * [new branch]          build-pr-5486-54874f9 -> upstream/build-pr-5486-54874f9
 * [new branch]          build-pr-5489-65053bd -> upstream/build-pr-5489-65053bd
   cb7f97e88..47d081b21  master                -> upstream/master
 * [new branch]          onViewableItemsChange -> upstream/onViewableItemsChange
   2119cb4f8..b028b2e14  release-1.44          -> upstream/release-1.44
 + a8e37ed42...7142ec639 version               -> upstream/version  (forced update)
Fetching origin
Failed to add the RSA host key for IP address '140.82.113.3' to the list of known hosts (/app/.ssh/known_hosts).
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-mattermost-mobile-#5486-upstream-release-1.44-1624645406
Switched to a new branch 'automated-cherry-pick-of-mattermost-mobile-#5486-upstream-release-1.44-1624645406'
Branch 'automated-cherry-pick-of-mattermost-mobile-#5486-upstream-release-1.44-1624645406' set up to track remote branch 'release-1.44' from 'upstream'.

+++ About to attempt cherry pick of PR #5486 with merge commit 47d081b216aab55bb81b0a92de6a233c99bdf26f.

error: could not apply 47d081b21... Content safety (#5486)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

+++ Conflicts detected:

UU ios/Podfile.lock
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the master branch and cleaning up.

@enahum
Copy link
Contributor Author

enahum commented Jun 25, 2021

manual cherry-pick

enahum added a commit that referenced this pull request Jun 25, 2021
* add content safety check to avoid potential crashes

* Fixes iOS YouTube video playback
@enahum enahum added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants