Merge branch 'mkirk/url-detection'

Podfile.lock

@@ -269,7 +269,7 @@ CHECKOUT OPTIONS:
     :commit: 9599b1d9796280c97cb2f786f34984fc98a3b6ef
     :git: https://github.com/signalapp/Mantle
   SignalCoreKit:
-    :commit: 061f41321675ffe5af5e547d578bbd2266a46d33
+    :commit: 0326310d32744902539bd6a2f170ee7413805754
     :git: https://github.com/signalapp/SignalCoreKit.git
   SignalMetadataKit:
     :commit: 56f28fc3a6e35d548d034ef7d0009f233ca0aa62

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.h

@@ -25,8 +25,6 @@ typedef NS_ENUM(NSUInteger, OWSMessageGestureLocation) {
     OWSMessageGestureLocation_LinkPreview,
 };
 
-extern const UIDataDetectorTypes kOWSAllowedDataDetectorTypes;
-
 @protocol OWSMessageBubbleViewDelegate
 
 - (void)didTapImageViewItem:(id<ConversationViewItem>)viewItem

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m

@@ -21,9 +21,6 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-const UIDataDetectorTypes kOWSAllowedDataDetectorTypes
-    = UIDataDetectorTypeLink | UIDataDetectorTypeAddress | UIDataDetectorTypeCalendarEvent;
-
 @interface OWSMessageBubbleView () <OWSQuotedMessageViewDelegate, OWSContactShareButtonsViewDelegate>
 
 @property (nonatomic) OWSBubbleView *bubbleView;
@@ -107,8 +104,6 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes
     [self.senderNameLabel ows_autoPinToSuperviewMargins];
 
     self.bodyTextView = [self newTextView];
-    // Setting dataDetectorTypes is expensive.  Do it just once.
-    self.bodyTextView.dataDetectorTypes = kOWSAllowedDataDetectorTypes;
     self.bodyTextView.hidden = YES;
 
     self.linkPreviewView = [[LinkPreviewView alloc] initWithDraftDelegate:nil];
@@ -682,7 +677,7 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes
         shouldIgnoreEvents = outgoingMessage.messageState != TSOutgoingMessageStateSent;
     }
     [self.class loadForTextDisplay:self.bodyTextView
-                              text:self.displayableBodyText.displayText
+                   displayableText:self.displayableBodyText
                         searchText:self.delegate.lastSearchedText
                          textColor:self.bodyTextColor
                               font:self.textMessageFont
@@ -690,7 +685,7 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes
 }
 
 + (void)loadForTextDisplay:(OWSMessageTextView *)textView
-                      text:(NSString *)text
+           displayableText:(DisplayableText *)displayableText
                 searchText:(nullable NSString *)searchText
                  textColor:(UIColor *)textColor
                       font:(UIFont *)font
@@ -707,6 +702,8 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes
     };
     textView.shouldIgnoreEvents = shouldIgnoreEvents;
 
+    NSString *text = displayableText.displayText;
+
     NSMutableAttributedString *attributedText = [[NSMutableAttributedString alloc]
         initWithString:text
             attributes:@{ NSFontAttributeName : font, NSForegroundColorAttributeName : textColor }];
@@ -725,6 +722,8 @@ const UIDataDetectorTypes kOWSAllowedDataDetectorTypes
         }
     }
 
+    [textView ensureShouldLinkifyText:displayableText.shouldAllowLinkification];
+
     // For perf, set text last. Otherwise changing font/color is more expensive.
 
     // We use attributedText even when we're not highlighting searched text to esnure any lingering

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageTextView.h

@@ -1,5 +1,5 @@
 //
-//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2019 Open Whisper Systems. All rights reserved.
 //
 
 #import <SignalMessaging/OWSTextView.h>

Signal/src/ViewControllers/DebugUI/DebugUIMessages.m

@@ -287,6 +287,11 @@ NS_ASSUME_NONNULL_BEGIN
                         actionBlock:^{
                             [DebugUIMessages testDirectionalFilenamesInThread:thread];
                         }],
+        [OWSTableItem itemWithTitle:@"Test Linkification"
+                        actionBlock:^{
+                            [DebugUIMessages testLinkificationInThread:thread];
+                        }],
+
     ]];
 
     if ([thread isKindOfClass:[TSContactThread class]]) {
@@ -4342,6 +4347,53 @@ typedef OWSContact * (^OWSContactBlock)(YapDatabaseReadWriteTransaction *transac
     [message save];
 }
 
++ (void)testLinkificationInThread:(TSThread *)thread
+{
+    NSArray<NSString *> *strings = @[@"google.com",
+                                     @"foo.google.com",
+                                     @"https://foo.google.com",
+                                     @"https://foo.google.com/some/path.html",
+                                     @"http://кц.com",
+                                     @"кц.com",
+                                     @"http://asĸ.com",
+                                     @"кц.рф",
+                                     @"кц.рф/some/path",
+                                     @"https://кц.рф/some/path",
+                                     @"http://foo.кц.рф"];
+
+    [OWSPrimaryStorage.sharedManager.dbReadWriteConnection
+     readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+         for (NSString *string in strings) {
+             // DO NOT log these strings with the debugger attached.
+             //        OWSLogInfo(@"%@", string);
+
+             {
+                 [self createFakeIncomingMessage:thread
+                                     messageBody:string
+                                 fakeAssetLoader:nil
+                          isAttachmentDownloaded:NO
+                                   quotedMessage:nil
+                                     transaction:transaction];
+             }
+             {
+                 NSString *recipientId = @"+1323555555";
+                 NSString *groupName = string;
+                 NSMutableArray<NSString *> *recipientIds = [@[
+                                                               recipientId,
+                                                               [TSAccountManager localNumber],
+                                                               ] mutableCopy];
+                 NSData *groupId = [Randomness generateRandomBytes:kGroupIdLength];
+                 TSGroupModel *groupModel =
+                 [[TSGroupModel alloc] initWithTitle:groupName memberIds:recipientIds image:nil groupId:groupId];
+
+                 TSGroupThread *groupThread =
+                 [TSGroupThread getOrCreateThreadWithGroupModel:groupModel transaction:transaction];
+                 OWSAssertDebug(groupThread);
+             }
+         }
+     }];
+}
+
 + (void)testIndicScriptsInThread:(TSThread *)thread
 {
     NSArray<NSString *> *strings = @[

Signal/src/ViewControllers/LongTextViewController.swift

@@ -28,10 +28,16 @@ public class LongTextViewController: OWSViewController {
 
     let viewItem: ConversationViewItem
 
-    let messageBody: String
-
     var messageTextView: UITextView!
 
+    var displayableText: DisplayableText? {
+        return viewItem.displayableBodyText
+    }
+
+    var fullText: String {
+        return displayableText?.fullText ?? ""
+    }
+
     // MARK: Initializers
 
     @available(*, unavailable, message:"use other constructor instead.")
@@ -42,23 +48,9 @@ public class LongTextViewController: OWSViewController {
     @objc
     public required init(viewItem: ConversationViewItem) {
         self.viewItem = viewItem
-
-        self.messageBody = LongTextViewController.displayableText(viewItem: viewItem)
-
         super.init(nibName: nil, bundle: nil)
     }
 
-    private class func displayableText(viewItem: ConversationViewItem) -> String {
-        guard viewItem.hasBodyText else {
-            return ""
-        }
-        guard let displayableText = viewItem.displayableBodyText else {
-            return ""
-        }
-        let messageBody = displayableText.fullText
-        return messageBody
-    }
-
     // MARK: View Lifecycle
 
     public override func viewDidLoad() {
@@ -137,8 +129,13 @@ public class LongTextViewController: OWSViewController {
         messageTextView.showsVerticalScrollIndicator = true
         messageTextView.isUserInteractionEnabled = true
         messageTextView.textColor = Theme.primaryColor
-        messageTextView.dataDetectorTypes = kOWSAllowedDataDetectorTypes
+        if let displayableText = displayableText {
-        messageTextView.text = messageBody
+            messageTextView.text = fullText
+            messageTextView.ensureShouldLinkifyText(displayableText.shouldAllowLinkification)
+        } else {
+            owsFailDebug("displayableText was unexpectedly nil")
+            messageTextView.text = ""
+        }
 
         // RADAR #18669
         // https://github.com/lionheart/openradar-mirror/issues/18669
@@ -153,8 +150,9 @@ public class LongTextViewController: OWSViewController {
 
         view.addSubview(messageTextView)
         messageTextView.autoPinEdge(toSuperviewEdge: .top)
-        messageTextView.autoPinEdge(toSuperviewMargin: .leading)
+        messageTextView.autoPinEdge(toSuperviewEdge: .leading)
-        messageTextView.autoPinEdge(toSuperviewMargin: .trailing)
+        messageTextView.autoPinEdge(toSuperviewEdge: .trailing)
+        messageTextView.textContainerInset = UIEdgeInsets(top: 0, leading: 16, bottom: 0, trailing: 16)
 
         let footer = UIToolbar()
         view.addSubview(footer)
@@ -172,6 +170,6 @@ public class LongTextViewController: OWSViewController {
     // MARK: - Actions
 
     @objc func shareButtonPressed() {
-        AttachmentSharing.showShareUI(forText: messageBody)
+        AttachmentSharing.showShareUI(forText: fullText)
     }
 }

Signal/test/util/DisplayableTextFilterTest.swift

@@ -1,5 +1,5 @@
 //
-//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2019 Open Whisper Systems. All rights reserved.
 //
 
 import XCTest
@@ -106,4 +106,60 @@ class DisplayableTextTest: SignalBaseTest {
         XCTAssertFalse("H҉̸̧͘͠A͢͞V̛̛I̴̸N͏̕͏G҉̵͜͏͢ ̧̧́T̶̛͘͡R̸̵̨̢̀O̷̡U͡҉B̶̛͢͞L̸̸͘͢͟É̸ ̸̛͘͏R͟È͠͞A̸͝Ḑ̕͘͜I̵͘҉͜͞N̷̡̢͠G̴͘͠ ͟͞T͏̢́͡È̀X̕҉̢̀T̢͠?̕͏̢͘͢".containsOnlyEmoji)
         XCTAssertFalse("L̷̳͔̲͝Ģ̵̮̯̤̩̙͍̬̟͉̹̘̹͍͈̮̦̰̣͟͝O̶̴̮̻̮̗͘͡!̴̷̟͓͓".containsOnlyEmoji)
     }
+
+    func test_shouldAllowLinkification() {
+        func assertLinkifies(_ text: String, file: StaticString = #file, line: UInt = #line) {
+            let displayableText = DisplayableText.displayableText(text)
+            XCTAssert(displayableText.shouldAllowLinkification, "was not linkifiable text: \(text)", file: file, line: line)
+        }
+
+        func assertNotLinkifies(_ text: String, file: StaticString = #file, line: UInt = #line) {
+            let displayableText = DisplayableText.displayableText(text)
+            XCTAssertFalse(displayableText.shouldAllowLinkification, "was linkifiable text: \(text)", file: file, line: line)
+        }
+
+        // some basic happy paths
+        assertLinkifies("foo google.com")
+        assertLinkifies("google.com/foo")
+        assertLinkifies("blah google.com/foo")
+        assertLinkifies("foo http://google.com")
+        assertLinkifies("foo https://google.com")
+
+        // cyrillic host with ascii tld
+        assertNotLinkifies("foo http://asĸ.com")
+        assertNotLinkifies("http://asĸ.com")
+        assertNotLinkifies("asĸ.com")
+
+        // Mixed latin and cyrillic text, but it's not a link
+        // (nothing to linkify, but there's nothing illegal here)
+        assertLinkifies("asĸ")
+
+        // Cyrillic host with cyrillic TLD
+        assertLinkifies("http://кц.рф")
+        assertLinkifies("https://кц.рф")
+        assertLinkifies("кц.рф")
+        assertLinkifies("https://кц.рф/foo")
+        assertLinkifies("https://кц.рф/кц")
+        assertLinkifies("https://кц.рф/кцfoo")
+
+        // ascii text outside of the link, with cyrillic host + cyrillic domain
+        assertLinkifies("some text: кц.рф")
+
+        // Mixed ascii/cyrillic text outside of the link, with cyrillic host + cyrillic domain
+        assertLinkifies("asĸ кц.рф")
+
+        assertLinkifies("google.com")
+        assertLinkifies("foo.google.com")
+        assertLinkifies("https://foo.google.com")
+        assertLinkifies("https://foo.google.com/some/path.html")
+
+        assertNotLinkifies("asĸ.com")
+        assertNotLinkifies("https://кц.cфm")
+        assertNotLinkifies("https://google.cфm")
+
+        assertLinkifies("кц.рф")
+        assertLinkifies("кц.рф/some/path")
+        assertLinkifies("https://кц.рф/some/path")
+        assertNotLinkifies("http://foo.кц.рф")
+    }
 }

SignalMessaging/Views/OWSTextView.h

@@ -1,11 +1,15 @@
 //
-//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2019 Open Whisper Systems. All rights reserved.
 //
 
 NS_ASSUME_NONNULL_BEGIN
 
+extern const UIDataDetectorTypes kOWSAllowedDataDetectorTypes;
+
 @interface OWSTextView : UITextView
 
+- (void)ensureShouldLinkifyText:(BOOL)shouldLinkifyText;
+
 @end
 
 NS_ASSUME_NONNULL_END

SignalMessaging/Views/OWSTextView.m

@@ -1,5 +1,5 @@
 //
-//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2019 Open Whisper Systems. All rights reserved.
 //
 
 #import "OWSTextView.h"
@@ -7,6 +7,12 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+const UIDataDetectorTypes kOWSAllowedDataDetectorTypes
+    = UIDataDetectorTypeLink | UIDataDetectorTypeAddress | UIDataDetectorTypeCalendarEvent;
+
+const UIDataDetectorTypes kOWSAllowedDataDetectorTypesExceptLinks
+    = UIDataDetectorTypeAddress | UIDataDetectorTypeCalendarEvent;
+
 @implementation OWSTextView
 
 - (instancetype)initWithFrame:(CGRect)frame textContainer:(nullable NSTextContainer *)textContainer
@@ -15,6 +21,9 @@ NS_ASSUME_NONNULL_BEGIN
         [self ows_applyTheme];
     }
 
+    // Setting dataDetectorTypes is expensive.  Do it just once.
+    self.dataDetectorTypes = kOWSAllowedDataDetectorTypes;
+
     return self;
 }
 
@@ -24,6 +33,8 @@ NS_ASSUME_NONNULL_BEGIN
         [self ows_applyTheme];
     }
 
+    self.dataDetectorTypes = kOWSAllowedDataDetectorTypes;
+
     return self;
 }
 
@@ -32,6 +43,23 @@ NS_ASSUME_NONNULL_BEGIN
     self.keyboardAppearance = Theme.keyboardAppearance;
 }
 
+// MARK: -
+
+- (void)ensureShouldLinkifyText:(BOOL)shouldLinkifyText
+{
+    if (shouldLinkifyText) {
+        // Setting dataDetectorTypes can be expensive, so we only update it when it's changed.
+        if (self.dataDetectorTypes != kOWSAllowedDataDetectorTypes) {
+            self.dataDetectorTypes = kOWSAllowedDataDetectorTypes;
+        }
+    } else {
+        // Setting dataDetectorTypes can be expensive, so we only update it when it's changed.
+        if (self.dataDetectorTypes != kOWSAllowedDataDetectorTypesExceptLinks) {
+            self.dataDetectorTypes = kOWSAllowedDataDetectorTypesExceptLinks;
+        }
+    }
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

SignalMessaging/utils/DisplayableText.swift

@@ -1,5 +1,5 @@
 //
-//  Copyright (c) 2018 Open Whisper Systems. All rights reserved.
+//  Copyright (c) 2019 Open Whisper Systems. All rights reserved.
 //
 
 import Foundation
@@ -165,7 +165,8 @@ extension String {
 
     // MARK: Initializers
 
-    @objc public init(fullText: String, displayText: String, isTextTruncated: Bool) {
+    @objc
+    public init(fullText: String, displayText: String, isTextTruncated: Bool) {
         self.fullText = fullText
         self.displayText = displayText
         self.isTextTruncated = isTextTruncated
@@ -198,6 +199,67 @@ extension String {
         return UInt(emojiCount)
     }
 
+    // For perf we use a static linkDetector. It doesn't change and building DataDetectors is
+    // surprisingly expensive. This should be fine, since NSDataDetector is an NSRegularExpression
+    // and NSRegularExpressions are thread safe.
+    private static let linkDetector: NSDataDetector? = {
+        return try? NSDataDetector(types: NSTextCheckingResult.CheckingType.link.rawValue)
+    }()
+
+    private static let hostRegex: NSRegularExpression? = {
+        let pattern = "^(?:https?:\\/\\/)?([^:\\/\\s]+)(.*)?$"
+        return try? NSRegularExpression(pattern: pattern)
+    }()
+
+    @objc
+    public lazy var shouldAllowLinkification: Bool = {
+        guard let linkDetector: NSDataDetector = DisplayableText.linkDetector else {
+            owsFailDebug("linkDetector was unexpectedly nil")
+            return false
+        }
+
+        func isValidLink(linkText: String) -> Bool {
+            guard let hostRegex = DisplayableText.hostRegex else {
+                owsFailDebug("hostRegex was unexpectedly nil")
+                return false
+            }
+
+            guard let hostText = hostRegex.parseFirstMatch(inText: linkText) else {
+                owsFailDebug("hostText was unexpectedly nil")
+                return false
+            }
+
+            let strippedHost = hostText.replacingOccurrences(of: ".", with: "") as NSString
+
+            if strippedHost.isOnlyASCII {
+                return true
+            } else if strippedHost.hasAnyASCII {
+                // mix of ascii and non-ascii is invalid
+                return false
+            } else {
+                // IDN
+                return true
+            }
+        }
+
+        for match in linkDetector.matches(in: fullText, options: [], range: NSRange(location: 0, length: fullText.utf16.count)) {
+            guard let matchURL: URL = match.url else {
+                continue
+            }
+
+            // We extract the exact text from the `fullText` rather than use match.url.host
+            // because match.url.host actually escapes non-ascii domains into puny-code.
+            //
+            // But what we really want is to check the text which will ultimately be presented to
+            // the user.
+            let rawTextOfMatch = (fullText as NSString).substring(with: match.range)
+            guard isValidLink(linkText: rawTextOfMatch) else {
+                return false
+            }
+        }
+        return true
+    }()
+
     // MARK: Filter Methods
 
     @objc

SignalServiceKit/src/Util/NSRegularExpression+SSK.swift

@@ -35,4 +35,21 @@ public extension NSRegularExpression {
             return nil
         }
     }
+
+    @objc
+    public func parseFirstMatch(inText text: String,
+                                options: NSRegularExpression.Options = []) -> String? {
+        guard let match = self.firstMatch(in: text,
+                                          options: [],
+                                          range: NSRange(location: 0, length: text.utf16.count)) else {
+                                            return nil
+        }
+        let matchRange = match.range(at: 1)
+        guard let textRange = Range(matchRange, in: text) else {
+            owsFailDebug("Invalid match.")
+            return nil
+        }
+        let substring = String(text[textRange])
+        return substring
+    }
 }