Merge branch 'charlesmchen/linkPreviews3'

Pods

@@ -1 +1 @@
-Subproject commit ea60f60ea01bc51fc2434248890b494e37da98a5
+Subproject commit 527dca96c23f0ac15664e9762987ff017cabdf90

SignalServiceKit/src/Messages/DeviceSyncing/OWSIncomingSentMessageTranscript.m

@@ -45,8 +45,15 @@ NS_ASSUME_NONNULL_BEGIN
 
     _quotedMessage = [TSQuotedMessage quotedMessageForDataMessage:_dataMessage thread:_thread transaction:transaction];
     _contact = [OWSContacts contactForDataMessage:_dataMessage transaction:transaction];
-    _linkPreview =
-        [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:_dataMessage body:_body transaction:transaction];
+
+    NSError *linkPreviewError;
+    _linkPreview = [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:_dataMessage
+                                                                       body:_body
+                                                                transaction:transaction
+                                                                      error:&linkPreviewError];
+    if (linkPreviewError && ![OWSLinkPreview isNoPreviewError:linkPreviewError]) {
+        OWSLogError(@"linkPreviewError: %@", linkPreviewError);
+    }
 
     if (sentProto.unidentifiedStatus.count > 0) {
         NSMutableArray<NSString *> *nonUdRecipientIds = [NSMutableArray new];

SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift

@@ -4,6 +4,13 @@
 
 import Foundation
 
+@objc
+public enum LinkPreviewError: Int, Error {
+    case invalidInput
+    case assertionFailure
+    case noPreview
+}
+
 @objc(OWSLinkPreview)
 public class OWSLinkPreview: MTLModel {
     @objc
@@ -13,13 +20,13 @@ public class OWSLinkPreview: MTLModel {
     public var title: String?
 
     @objc
-    public var attachmentId: String?
+    public var imageAttachmentId: String?
 
     @objc
-    public init(urlString: String, title: String?, attachmentId: String?) {
+    public init(urlString: String, title: String?, imageAttachmentId: String?) {
         self.urlString = urlString
         self.title = title
-        self.attachmentId = attachmentId
+        self.imageAttachmentId = imageAttachmentId
 
         super.init()
     }
@@ -34,28 +41,36 @@ public class OWSLinkPreview: MTLModel {
         try super.init(dictionary: dictionaryValue)
     }
 
+    @objc
+    public class func isNoPreviewError(_ error: Error) -> Bool {
+        guard let error = error as? LinkPreviewError else {
+            return false
+        }
+        return error == .noPreview
+    }
+
     @objc
     public class func buildValidatedLinkPreview(dataMessage: SSKProtoDataMessage,
                                                 body: String?,
-                                                transaction: YapDatabaseReadWriteTransaction) -> OWSLinkPreview? {
+                                                transaction: YapDatabaseReadWriteTransaction) throws -> OWSLinkPreview {
         guard let previewProto = dataMessage.preview else {
-            return nil
+            throw LinkPreviewError.noPreview
         }
         let urlString = previewProto.url
 
         guard URL(string: urlString) != nil else {
-            owsFailDebug("Could not parse preview URL.")
-            return nil
+            Logger.error("Could not parse preview URL.")
+            throw LinkPreviewError.invalidInput
         }
 
         guard let body = body else {
-            owsFailDebug("Preview for message without body.")
-            return nil
+            Logger.error("Preview for message without body.")
+            throw LinkPreviewError.invalidInput
         }
         let bodyComponents = body.components(separatedBy: .whitespacesAndNewlines)
         guard bodyComponents.contains(urlString) else {
-            owsFailDebug("URL not present in body.")
-            return nil
+            Logger.error("URL not present in body.")
+            throw LinkPreviewError.invalidInput
         }
 
         // TODO: Verify that url host is in whitelist.
@@ -68,7 +83,8 @@ public class OWSLinkPreview: MTLModel {
                 imageAttachmentPointer.save(with: transaction)
                 imageAttachmentId = imageAttachmentPointer.uniqueId
             } else {
-                owsFailDebug("Could not parse image proto.")
+                Logger.error("Could not parse image proto.")
+                throw LinkPreviewError.invalidInput
             }
         }
 
@@ -78,23 +94,101 @@ public class OWSLinkPreview: MTLModel {
         }
         let hasImage = imageAttachmentId != nil
         if !hasTitle && !hasImage {
-            owsFailDebug("Preview has neither title nor image.")
-            return nil
+            Logger.error("Preview has neither title nor image.")
+            throw LinkPreviewError.invalidInput
         }
 
-        return OWSLinkPreview(urlString: urlString, title: title, attachmentId: imageAttachmentId)
+        return OWSLinkPreview(urlString: urlString, title: title, imageAttachmentId: imageAttachmentId)
     }
 
     @objc
     public func removeAttachment(transaction: YapDatabaseReadWriteTransaction) {
-        guard let attachmentId = attachmentId else {
+        guard let imageAttachmentId = imageAttachmentId else {
             owsFailDebug("No attachment id.")
             return
         }
-        guard let attachment = TSAttachment.fetch(uniqueId: attachmentId, transaction: transaction) else {
+        guard let attachment = TSAttachment.fetch(uniqueId: imageAttachmentId, transaction: transaction) else {
             owsFailDebug("Could not load attachment.")
             return
         }
         attachment.remove(with: transaction)
     }
+
+    // MARK: - Domain Whitelist
+
+    private static let linkDomainWhitelist = [
+    "youtube.com",
+    "reddit.com",
+    "imgur.com",
+    "instagram.com"
+    ]
+
+    private static let mediaDomainWhitelist = [
+        "ytimg.com",
+        "cdninstagram.com"
+        ]
+
+    private static let protocolWhitelist = [
+        "https"
+        ]
+
+    @objc
+    public class func isValidLinkUrl(_ urlString: String) -> Bool {
+        guard let url = URL(string: urlString) else {
+            return false
+        }
+        return isUrlInDomainWhitelist(url: url,
+                                      domainWhitelist: OWSLinkPreview.linkDomainWhitelist)
+    }
+
+    @objc
+    public class func isValidMediaUrl(_ urlString: String) -> Bool {
+        guard let url = URL(string: urlString) else {
+            return false
+        }
+        return isUrlInDomainWhitelist(url: url,
+            domainWhitelist: OWSLinkPreview.linkDomainWhitelist + OWSLinkPreview.mediaDomainWhitelist)
+    }
+
+    private class func isUrlInDomainWhitelist(url: URL, domainWhitelist: [String]) -> Bool {
+        guard let urlProtocol = url.scheme?.lowercased() else {
+            return false
+        }
+        guard protocolWhitelist.contains(urlProtocol) else {
+            return false
+        }
+        guard let domain = url.host?.lowercased() else {
+            return false
+        }
+        // TODO: We need to verify:
+        //
+        // * The final domain whitelist.
+        // * The relationship between the "link" whitelist and the "media" whitelist.
+        // * Exact match or suffix-based?
+        // * Case-insensitive?
+        // * Protocol?
+        for whitelistedDomain in domainWhitelist {
+            if domain == whitelistedDomain.lowercased() ||
+                domain.hasSuffix("." + whitelistedDomain.lowercased()) {
+                return true
+            }
+        }
+        return false
+    }
+
+    // MARK: - Text Parsing
+
+    @objc
+    public class func previewUrl(forMessageBodyText body: String?) -> String? {
+        guard let body = body else {
+            return nil
+        }
+        let components = body.components(separatedBy: .whitespacesAndNewlines)
+        for component in components {
+            if isValidLinkUrl(component) {
+                return component
+            }
+        }
+        return nil
+    }
 }

SignalServiceKit/src/Messages/Interactions/TSMessage.m

@@ -351,7 +351,7 @@ static const NSUInteger OWSMessageSchemaVersion = 4;
         [self.contactShare removeAvatarAttachmentWithTransaction:transaction];
     }
 
-    if (self.linkPreview.attachmentId) {
+    if (self.linkPreview.imageAttachmentId) {
         [self.linkPreview removeAttachmentWithTransaction:transaction];
     }
 

SignalServiceKit/src/Messages/OWSMessageManager.m

@@ -1287,10 +1287,15 @@ NS_ASSUME_NONNULL_BEGIN
                                                                                                  thread:oldGroupThread
                                                                                             transaction:transaction];
 
+                NSError *linkPreviewError;
                 OWSLinkPreview *_Nullable linkPreview =
                     [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:dataMessage
                                                                         body:body
-                                                                 transaction:transaction];
+                                                                 transaction:transaction
+                                                                       error:&linkPreviewError];
+                if (linkPreviewError && ![OWSLinkPreview isNoPreviewError:linkPreviewError]) {
+                    OWSLogError(@"linkPreviewError: %@", linkPreviewError);
+                }
 
                 OWSLogDebug(@"incoming message from: %@ for group: %@ with timestamp: %lu",
                     envelopeAddress(envelope),
@@ -1355,8 +1360,15 @@ NS_ASSUME_NONNULL_BEGIN
                                                                                          thread:thread
                                                                                     transaction:transaction];
 
+        NSError *linkPreviewError;
         OWSLinkPreview *_Nullable linkPreview =
-            [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:dataMessage body:body transaction:transaction];
+            [OWSLinkPreview buildValidatedLinkPreviewWithDataMessage:dataMessage
+                                                                body:body
+                                                         transaction:transaction
+                                                               error:&linkPreviewError];
+        if (linkPreviewError && ![OWSLinkPreview isNoPreviewError:linkPreviewError]) {
+            OWSLogError(@"linkPreviewError: %@", linkPreviewError);
+        }
 
         // Legit usage of senderTimestamp when creating incoming message from received envelope
         TSIncomingMessage *incomingMessage =

SignalServiceKit/tests/Messages/OWSLinkPreviewTest.swift

@@ -0,0 +1,157 @@
+//
+//  Copyright (c) 2019 Open Whisper Systems. All rights reserved.
+//
+
+import Foundation
+import SignalServiceKit
+import XCTest
+
+class OWSLinkPreviewTest: SSKBaseTestSwift {
+
+    override func setUp() {
+        super.setUp()
+        // Put setup code here. This method is called before the invocation of each test method in the class.
+    }
+
+    override func tearDown() {
+        // Put teardown code here. This method is called after the invocation of each test method in the class.
+        super.tearDown()
+    }
+
+    func testBuildValidatedLinkPreview_TitleAndImage() {
+        let url = "https://www.youtube.com/watch?v=tP-Ipsat90c"
+        let body = "\(url)"
+        let previewBuilder = SSKProtoDataMessagePreview.builder(url: url)
+        previewBuilder.setTitle("Some Youtube Video")
+        let imageAttachmentBuilder = SSKProtoAttachmentPointer.builder(id: 1)
+        imageAttachmentBuilder.setKey(Randomness.generateRandomBytes(32))
+        imageAttachmentBuilder.setContentType(OWSMimeTypeImageJpeg)
+        previewBuilder.setImage(try! imageAttachmentBuilder.build())
+        let dataBuilder = SSKProtoDataMessage.builder()
+        dataBuilder.setPreview(try! previewBuilder.build())
+
+        self.readWrite { (transaction) in
+            XCTAssertNotNil(try! OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(),
+                                                                     body: body,
+                                                                     transaction: transaction))
+        }
+    }
+
+    func testBuildValidatedLinkPreview_Title() {
+        let url = "https://www.youtube.com/watch?v=tP-Ipsat90c"
+        let body = "\(url)"
+        let previewBuilder = SSKProtoDataMessagePreview.builder(url: url)
+        previewBuilder.setTitle("Some Youtube Video")
+        let dataBuilder = SSKProtoDataMessage.builder()
+        dataBuilder.setPreview(try! previewBuilder.build())
+
+        self.readWrite { (transaction) in
+            XCTAssertNotNil(try! OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(),
+                                                                     body: body,
+                                                                     transaction: transaction))
+        }
+    }
+
+    func testBuildValidatedLinkPreview_Image() {
+        let url = "https://www.youtube.com/watch?v=tP-Ipsat90c"
+        let body = "\(url)"
+        let previewBuilder = SSKProtoDataMessagePreview.builder(url: url)
+        let imageAttachmentBuilder = SSKProtoAttachmentPointer.builder(id: 1)
+        imageAttachmentBuilder.setKey(Randomness.generateRandomBytes(32))
+        imageAttachmentBuilder.setContentType(OWSMimeTypeImageJpeg)
+        previewBuilder.setImage(try! imageAttachmentBuilder.build())
+        let dataBuilder = SSKProtoDataMessage.builder()
+        dataBuilder.setPreview(try! previewBuilder.build())
+
+        self.readWrite { (transaction) in
+            XCTAssertNotNil(try! OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(),
+                                                                     body: body,
+                                                                     transaction: transaction))
+        }
+    }
+
+    func testBuildValidatedLinkPreview_NoTitleOrImage() {
+        let url = "https://www.youtube.com/watch?v=tP-Ipsat90c"
+        let body = "\(url)"
+        let previewBuilder = SSKProtoDataMessagePreview.builder(url: url)
+        let dataBuilder = SSKProtoDataMessage.builder()
+        dataBuilder.setPreview(try! previewBuilder.build())
+
+        self.readWrite { (transaction) in
+            do {
+                _ = try OWSLinkPreview.buildValidatedLinkPreview(dataMessage: try! dataBuilder.build(),
+                                                         body: body,
+                                                         transaction: transaction)
+                XCTFail("Missing expected error.")
+            } catch {
+                // Do nothing.
+            }
+        }
+    }
+
+    func testIsValidLinkUrl() {
+        XCTAssertTrue(OWSLinkPreview.isValidLinkUrl("https://www.youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertTrue(OWSLinkPreview.isValidLinkUrl("https://youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertTrue(OWSLinkPreview.isValidLinkUrl("https://www.youtube.com"))
+
+        // Allow arbitrary subdomains.
+        XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://some.random.subdomain.youtube.com/watch?v=tP-Ipsat90c"))
+
+        // Don't allow HTTP, only HTTPS
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("http://youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("mailto://youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("ftp://youtube.com/watch?v=tP-Ipsat90c"))
+
+        // Don't allow similar domains.
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://xyoutube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://youtubex.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://youtube.comx/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://www.xyoutube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://www.youtubex.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://www.youtube.comx/watch?v=tP-Ipsat90c"))
+
+        // Don't allow media domains.
+        XCTAssertFalse(OWSLinkPreview.isValidLinkUrl("https://i.ytimg.com/vi/tP-Ipsat90c/maxresdefault.jpg"))
+    }
+
+    func testIsValidMediaUrl() {
+        XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://www.youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://www.youtube.com"))
+
+        // Allow arbitrary subdomains.
+        XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://some.random.subdomain.youtube.com/watch?v=tP-Ipsat90c"))
+
+        // Don't allow HTTP, only HTTPS
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("http://youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("mailto://youtube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("ftp://youtube.com/watch?v=tP-Ipsat90c"))
+
+        // Don't allow similar domains.
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://xyoutube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://youtubex.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://youtube.comx/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://www.xyoutube.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://www.youtubex.com/watch?v=tP-Ipsat90c"))
+        XCTAssertFalse(OWSLinkPreview.isValidMediaUrl("https://www.youtube.comx/watch?v=tP-Ipsat90c"))
+
+        // Allow media domains.
+        XCTAssertTrue(OWSLinkPreview.isValidMediaUrl("https://i.ytimg.com/vi/tP-Ipsat90c/maxresdefault.jpg"))
+    }
+
+    func testPreviewUrlForMessageBodyText() {
+        XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: ""))
+        XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim"))
+        XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim http://"))
+        XCTAssertNil(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob jim http://a.com"))
+
+        XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "https://www.youtube.com/watch?v=tP-Ipsat90c"),
+                       "https://www.youtube.com/watch?v=tP-Ipsat90c")
+        XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim"),
+                       "https://www.youtube.com/watch?v=tP-Ipsat90c")
+
+        // If there are more than one, take the first.
+        XCTAssertEqual(OWSLinkPreview.previewUrl(forMessageBodyText: "alice bob https://www.youtube.com/watch?v=tP-Ipsat90c jim https://www.youtube.com/watch?v=other-url carol"),
+                       "https://www.youtube.com/watch?v=tP-Ipsat90c")
+    }
+}