From 64312f9c7f972c7d565d19865161373f7322b36d Mon Sep 17 00:00:00 2001 From: Alan Evans Date: Tue, 5 Jan 2021 17:07:18 -0400 Subject: [PATCH] Fix non-rendered previews when differ by trailing slash. --- .../securesms/jobs/PushProcessMessageJob.java | 8 +- .../linkpreview/LinkPreviewUtil.java | 48 ++++++++-- .../linkpreview/LinkPreviewViewModel.java | 4 +- ...kPreviewUtilTest_findValidPreviewUrls.java | 89 +++++++++++++++++++ 4 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtilTest_findValidPreviewUrls.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java index be8ee32529..00f791b9d0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushProcessMessageJob.java @@ -57,7 +57,6 @@ import org.thoughtcrime.securesms.jobmanager.Job; import org.thoughtcrime.securesms.jobmanager.JobManager; import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint; import org.thoughtcrime.securesms.keyvalue.SignalStore; -import org.thoughtcrime.securesms.linkpreview.Link; import org.thoughtcrime.securesms.linkpreview.LinkPreview; import org.thoughtcrime.securesms.linkpreview.LinkPreviewUtil; import org.thoughtcrime.securesms.mms.IncomingMediaMessage; @@ -1797,9 +1796,10 @@ public final class PushProcessMessageJob extends BaseJob { } private Optional> getLinkPreviews(Optional> previews, @NonNull String message) { - if (!previews.isPresent()) return Optional.absent(); + if (!previews.isPresent() || previews.get().isEmpty()) return Optional.absent(); - List linkPreviews = new ArrayList<>(previews.get().size()); + List linkPreviews = new ArrayList<>(previews.get().size()); + LinkPreviewUtil.Links urlsInMessage = LinkPreviewUtil.findValidPreviewUrls(message); for (Preview preview : previews.get()) { Optional thumbnail = PointerAttachment.forPointer(preview.getImage()); @@ -1807,7 +1807,7 @@ public final class PushProcessMessageJob extends BaseJob { Optional title = Optional.fromNullable(preview.getTitle()); Optional description = Optional.fromNullable(preview.getDescription()); boolean hasTitle = !TextUtils.isEmpty(title.or("")); - boolean presentInBody = url.isPresent() && Stream.of(LinkPreviewUtil.findValidPreviewUrls(message)).map(Link::getUrl).collect(Collectors.toSet()).contains(url.get()); + boolean presentInBody = url.isPresent() && urlsInMessage.containsUrl(url.get()); boolean validDomain = url.isPresent() && LinkPreviewUtil.isValidPreviewUrl(url.get()); if (hasTitle && presentInBody && validDomain) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtil.java b/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtil.java index 5ce8cd8203..a9627b2fd6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtil.java @@ -11,6 +11,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import com.annimon.stream.Collectors; import com.annimon.stream.Stream; import org.signal.core.util.logging.Log; @@ -50,18 +51,18 @@ public final class LinkPreviewUtil { /** * @return All whitelisted URLs in the source text. */ - public static @NonNull List findValidPreviewUrls(@NonNull String text) { + public static @NonNull Links findValidPreviewUrls(@NonNull String text) { SpannableString spannable = new SpannableString(text); boolean found = Linkify.addLinks(spannable, Linkify.WEB_URLS); if (!found) { - return Collections.emptyList(); + return Links.EMPTY; } - return Stream.of(spannable.getSpans(0, spannable.length(), URLSpan.class)) - .map(span -> new Link(span.getURL(), spannable.getSpanStart(span))) - .filter(link -> isValidPreviewUrl(link.getUrl())) - .toList(); + return new Links(Stream.of(spannable.getSpans(0, spannable.length(), URLSpan.class)) + .map(span -> new Link(span.getURL(), spannable.getSpanStart(span))) + .filter(link -> isValidPreviewUrl(link.getUrl())) + .toList()); } /** @@ -217,4 +218,39 @@ public final class LinkPreviewUtil { public interface HtmlDecoder { @NonNull String fromEncoded(@NonNull String html); } + + public static class Links { + static final Links EMPTY = new Links(Collections.emptyList()); + + private final List links; + private final Set urlSet; + + private Links(@NonNull List links) { + this.links = links; + this.urlSet = Stream.of(links) + .map(link -> trimTrailingSlash(link.getUrl())) + .collect(Collectors.toSet()); + } + + public Optional findFirst() { + return links.isEmpty() ? Optional.absent() + : Optional.of(links.get(0)); + } + + /** + * Slightly forgiving comparison where it will ignore trailing '/' on the supplied url. + */ + public boolean containsUrl(@NonNull String url) { + return urlSet.contains(trimTrailingSlash(url)); + } + + private @NonNull String trimTrailingSlash(@NonNull String url) { + return url.endsWith("/") ? url.substring(0, url.length() - 1) + : url; + } + + public int size() { + return links.size(); + } + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewViewModel.java index 0237811713..d81fea67ae 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewViewModel.java @@ -75,8 +75,8 @@ public class LinkPreviewViewModel extends ViewModel { return; } - List links = LinkPreviewUtil.findValidPreviewUrls(text); - Optional link = links.isEmpty() ? Optional.absent() : Optional.of(links.get(0)); + Optional link = LinkPreviewUtil.findValidPreviewUrls(text) + .findFirst(); if (link.isPresent() && link.get().getUrl().equals(activeUrl)) { return; diff --git a/app/src/test/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtilTest_findValidPreviewUrls.java b/app/src/test/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtilTest_findValidPreviewUrls.java new file mode 100644 index 0000000000..11010196d8 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/linkpreview/LinkPreviewUtilTest_findValidPreviewUrls.java @@ -0,0 +1,89 @@ +package org.thoughtcrime.securesms.linkpreview; + +import android.app.Application; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE, application = Application.class) +public final class LinkPreviewUtilTest_findValidPreviewUrls { + + @Test + public void no_links() { + LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("No links"); + + assertEquals(0, links.size()); + assertSame(LinkPreviewUtil.Links.EMPTY, links); + } + + @Test + public void contains_a_link() { + LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("https://signal.org"); + + assertEquals(1, links.size()); + + assertTrue(links.containsUrl("https://signal.org")); + } + + @Test + public void does_not_contain_link() { + LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("https://signal.org"); + + assertEquals(1, links.size()); + + assertFalse(links.containsUrl("https://signal.org/page")); + } + + @Test + public void contains_two_links() { + LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Links https://signal.org https://android.com"); + + assertEquals(2, links.size()); + + assertTrue(links.containsUrl("https://signal.org")); + assertTrue(links.containsUrl("https://android.com")); + } + + @Test + public void link_trailing_slash_insensitivity() { + LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Links https://signal.org/ https://android.com"); + + assertEquals(2, links.size()); + + assertTrue(links.containsUrl("https://signal.org")); + assertTrue(links.containsUrl("https://android.com")); + assertTrue(links.containsUrl("https://signal.org/")); + assertTrue(links.containsUrl("https://android.com/")); + } + + @Test + public void link_trailing_slash_insensitivity_where_last_url_has_trailing_slash() { + LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Links https://signal.org https://android.com/"); + + assertEquals(2, links.size()); + + assertTrue(links.containsUrl("https://signal.org")); + assertTrue(links.containsUrl("https://android.com")); + assertTrue(links.containsUrl("https://signal.org/")); + assertTrue(links.containsUrl("https://android.com/")); + } + + @Test + public void multiple_trailing_slashes_are_not_stripped() { + LinkPreviewUtil.Links links = LinkPreviewUtil.findValidPreviewUrls("Link https://android.com/"); + + assertEquals(1, links.size()); + + assertTrue(links.containsUrl("https://android.com")); + assertTrue(links.containsUrl("https://android.com/")); + assertFalse(links.containsUrl("https://android.com//")); + } +}