Merge pull request #6851 from litetex/make-parsing-of-timestamp-links-more-robust

Catch errors while processing timestamp-links

app/src/main/java/org/schabi/newpipe/info_list/holder/CommentsMiniInfoItemHolder.java

@@ -5,6 +5,7 @@ import android.text.TextUtils;
 import android.text.method.LinkMovementMethod;
 import android.text.style.URLSpan;
 import android.text.util.Linkify;
+import android.util.Log;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.RelativeLayout;
@@ -24,17 +25,19 @@ import org.schabi.newpipe.util.DeviceUtils;
 import org.schabi.newpipe.util.ImageDisplayConstants;
 import org.schabi.newpipe.util.Localization;
 import org.schabi.newpipe.util.NavigationHelper;
+import org.schabi.newpipe.util.external_communication.TimestampExtractor;
 import org.schabi.newpipe.util.external_communication.ShareUtils;
 
 import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import de.hdodenhof.circleimageview.CircleImageView;
 
 public class CommentsMiniInfoItemHolder extends InfoItemHolder {
+    private static final String TAG = "CommentsMiniIIHolder";
+
     private static final int COMMENT_DEFAULT_LINES = 2;
     private static final int COMMENT_EXPANDED_LINES = 1000;
-    private static final Pattern PATTERN = Pattern.compile("(\\d+:)?(\\d+)?:(\\d+)");
+
     private final String downloadThumbnailKey;
     private final int commentHorizontalPadding;
     private final int commentVerticalPadding;
@@ -44,7 +47,6 @@ public class CommentsMiniInfoItemHolder extends InfoItemHolder {
     public final CircleImageView itemThumbnailView;
     private final TextView itemContentView;
     private final TextView itemLikesCountView;
-    private final TextView itemDislikesCountView;
     private final TextView itemPublishedTime;
 
     private String commentText;
@@ -53,20 +55,21 @@ public class CommentsMiniInfoItemHolder extends InfoItemHolder {
     private final Linkify.TransformFilter timestampLink = new Linkify.TransformFilter() {
         @Override
         public String transformUrl(final Matcher match, final String url) {
-            int timestamp = 0;
+            try {
-            final String hours = match.group(1);
+                final TimestampExtractor.TimestampMatchDTO timestampMatchDTO =
-            final String minutes = match.group(2);
+                        TimestampExtractor.getTimestampFromMatcher(match, commentText);
-            final String seconds = match.group(3);
+
-            if (hours != null) {
+                if (timestampMatchDTO == null) {
-                timestamp += (Integer.parseInt(hours.replace(":", "")) * 3600);
+                    return url;
+                }
+
+                return streamUrl + url.replace(
+                        match.group(0),
+                        "#timestamp=" + timestampMatchDTO.seconds());
+            } catch (final Exception ex) {
+                Log.e(TAG, "Unable to process url='" + url + "' as timestampLink", ex);
+                return url;
             }
-            if (minutes != null) {
-                timestamp += (Integer.parseInt(minutes.replace(":", "")) * 60);
-            }
-            if (seconds != null) {
-                timestamp += (Integer.parseInt(seconds));
-            }
-            return streamUrl + url.replace(match.group(0), "#timestamp=" + timestamp);
         }
     };
 
@@ -77,7 +80,6 @@ public class CommentsMiniInfoItemHolder extends InfoItemHolder {
         itemRoot = itemView.findViewById(R.id.itemRoot);
         itemThumbnailView = itemView.findViewById(R.id.itemThumbnailView);
         itemLikesCountView = itemView.findViewById(R.id.detail_thumbs_up_count_view);
-        itemDislikesCountView = itemView.findViewById(R.id.detail_thumbs_down_count_view);
         itemPublishedTime = itemView.findViewById(R.id.itemPublishedTime);
         itemContentView = itemView.findViewById(R.id.itemCommentContentView);
 
@@ -254,7 +256,14 @@ public class CommentsMiniInfoItemHolder extends InfoItemHolder {
     }
 
     private void linkify() {
-        Linkify.addLinks(itemContentView, Linkify.WEB_URLS);
+        Linkify.addLinks(
-        Linkify.addLinks(itemContentView, PATTERN, null, null, timestampLink);
+                itemContentView,
+                Linkify.WEB_URLS);
+        Linkify.addLinks(
+                itemContentView,
+                TimestampExtractor.TIMESTAMPS_PATTERN,
+                null,
+                null,
+                timestampLink);
     }
 }

app/src/main/java/org/schabi/newpipe/util/external_communication/TextLinkifier.java

@@ -32,9 +32,8 @@ import static org.schabi.newpipe.util.external_communication.InternalUrlsHandler
 
 public final class TextLinkifier {
     public static final String TAG = TextLinkifier.class.getSimpleName();
+
     private static final Pattern HASHTAGS_PATTERN = Pattern.compile("(#[A-Za-z0-9_]+)");
-    private static final Pattern TIMESTAMPS_PATTERN = Pattern.compile(
-            "(?:^|(?!:)\\W)(?:([0-5]?[0-9]):)?([0-5]?[0-9]):([0-5][0-9])(?=$|(?!:)\\W)");
 
     private TextLinkifier() {
     }
@@ -174,33 +173,34 @@ public final class TextLinkifier {
                                                       final Info relatedInfo,
                                                       final CompositeDisposable disposables) {
         final String descriptionText = spannableDescription.toString();
-        final Matcher timestampsMatches = TIMESTAMPS_PATTERN.matcher(descriptionText);
+        final Matcher timestampsMatches =
+                TimestampExtractor.TIMESTAMPS_PATTERN.matcher(descriptionText);
 
         while (timestampsMatches.find()) {
-            final int timestampStart = timestampsMatches.start(2);
+            final TimestampExtractor.TimestampMatchDTO timestampMatchDTO =
-            final int timestampEnd = timestampsMatches.end(3);
+                    TimestampExtractor.getTimestampFromMatcher(
-            final String parsedTimestamp = descriptionText.substring(timestampStart, timestampEnd);
+                            timestampsMatches,
-            final String[] timestampParts = parsedTimestamp.split(":");
+                            descriptionText);
 
-            final int seconds;
+            if (timestampMatchDTO == null) {
-            if (timestampParts.length == 3) { // timestamp format: XX:XX:XX
-                seconds = Integer.parseInt(timestampParts[0]) * 3600 // hours
-                        + Integer.parseInt(timestampParts[1]) * 60 // minutes
-                        + Integer.parseInt(timestampParts[2]); // seconds
-            } else if (timestampParts.length == 2) { // timestamp format: XX:XX
-                seconds = Integer.parseInt(timestampParts[0]) * 60 // minutes
-                        + Integer.parseInt(timestampParts[1]); // seconds
-            } else {
                 continue;
             }
 
-            spannableDescription.setSpan(new ClickableSpan() {
+            spannableDescription.setSpan(
-                @Override
+                    new ClickableSpan() {
-                public void onClick(@NonNull final View view) {
+                        @Override
-                    playOnPopup(context, relatedInfo.getUrl(), relatedInfo.getService(), seconds,
+                        public void onClick(@NonNull final View view) {
-                            disposables);
+                            playOnPopup(
-                }
+                                    context,
-            }, timestampStart, timestampEnd, 0);
+                                    relatedInfo.getUrl(),
+                                    relatedInfo.getService(),
+                                    timestampMatchDTO.seconds(),
+                                    disposables);
+                        }
+                    },
+                    timestampMatchDTO.timestampStart(),
+                    timestampMatchDTO.timestampEnd(),
+                    0);
         }
     }
 

app/src/main/java/org/schabi/newpipe/util/external_communication/TimestampExtractor.java

@@ -0,0 +1,79 @@
+package org.schabi.newpipe.util.external_communication;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Extracts timestamps.
+ */
+public final class TimestampExtractor {
+    public static final Pattern TIMESTAMPS_PATTERN = Pattern.compile(
+            "(?:^|(?!:)\\W)(?:([0-5]?[0-9]):)?([0-5]?[0-9]):([0-5][0-9])(?=$|(?!:)\\W)");
+
+    private TimestampExtractor() {
+        // No impl pls
+    }
+
+    /**
+     * Get's a single timestamp from a matcher.
+     *
+     * @param timestampMatches The matcher which was created using {@link #TIMESTAMPS_PATTERN}
+     * @param baseText         The text where the pattern was applied to /
+     *                         where the matcher is based upon
+     * @return If a match occurred: a {@link TimestampMatchDTO} filled with information.<br/>
+     * If not <code>null</code>.
+     */
+    public static TimestampMatchDTO getTimestampFromMatcher(
+            final Matcher timestampMatches,
+            final String baseText) {
+        int timestampStart = timestampMatches.start(1);
+        if (timestampStart == -1) {
+            timestampStart = timestampMatches.start(2);
+        }
+        final int timestampEnd = timestampMatches.end(3);
+
+        final String parsedTimestamp = baseText.substring(timestampStart, timestampEnd);
+        final String[] timestampParts = parsedTimestamp.split(":");
+
+        final int seconds;
+        if (timestampParts.length == 3) { // timestamp format: XX:XX:XX
+            seconds = Integer.parseInt(timestampParts[0]) * 3600 // hours
+                    + Integer.parseInt(timestampParts[1]) * 60 // minutes
+                    + Integer.parseInt(timestampParts[2]); // seconds
+        } else if (timestampParts.length == 2) { // timestamp format: XX:XX
+            seconds = Integer.parseInt(timestampParts[0]) * 60 // minutes
+                    + Integer.parseInt(timestampParts[1]); // seconds
+        } else {
+            return null;
+        }
+
+        return new TimestampMatchDTO(timestampStart, timestampEnd, seconds);
+    }
+
+    public static class TimestampMatchDTO {
+        private final int timestampStart;
+        private final int timestampEnd;
+        private final int seconds;
+
+        public TimestampMatchDTO(
+                final int timestampStart,
+                final int timestampEnd,
+                final int seconds) {
+            this.timestampStart = timestampStart;
+            this.timestampEnd = timestampEnd;
+            this.seconds = seconds;
+        }
+
+        public int timestampStart() {
+            return timestampStart;
+        }
+
+        public int timestampEnd() {
+            return timestampEnd;
+        }
+
+        public int seconds() {
+            return seconds;
+        }
+    }
+}

app/src/test/java/org/schabi/newpipe/util/external_communication/TimestampExtractorTest.java

@@ -0,0 +1,101 @@
+package org.schabi.newpipe.util.external_communication;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
+import java.util.regex.Matcher;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
+@RunWith(Parameterized.class)
+public class TimestampExtractorTest {
+
+    @Parameterized.Parameter(0)
+    public Duration expected;
+
+    @Parameterized.Parameter(1)
+    public String stringToProcess;
+
+    @Parameterized.Parameters(name = "Expecting {0} for \"{1}\"")
+    public static List<Object[]> dataForTests() {
+        return Arrays.asList(new Object[][]{
+                // Simple valid values
+                {Duration.ofSeconds(1), "0:01"},
+                {Duration.ofSeconds(1), "00:01"},
+                {Duration.ofSeconds(1), "0:00:01"},
+                {Duration.ofSeconds(1), "00:00:01"},
+                {Duration.ofMinutes(1).plusSeconds(23), "1:23"},
+                {Duration.ofMinutes(1).plusSeconds(23), "01:23"},
+                {Duration.ofMinutes(1).plusSeconds(23), "0:01:23"},
+                {Duration.ofMinutes(1).plusSeconds(23), "00:01:23"},
+                {Duration.ofHours(1).plusMinutes(23).plusSeconds(45), "1:23:45"},
+                {Duration.ofHours(1).plusMinutes(23).plusSeconds(45), "01:23:45"},
+                // Check with additional text
+                {Duration.ofSeconds(1), "Wow 0:01 words"},
+                {Duration.ofMinutes(1).plusSeconds(23), "Wow 1:23 words"},
+                {Duration.ofSeconds(1), "Wow 0:01 words! 33:"},
+                {null, "Wow0:01 abc"},
+                {null, "Wow 0:01abc"},
+                {null, "Wow0:01abc"},
+                {null, "Wow0:01"},
+                {null, "0:01abc"},
+                // Boundary checks
+                {Duration.ofSeconds(0), "0:00"},
+                {Duration.ofHours(59).plusMinutes(59).plusSeconds(59), "59:59:59"},
+                {null, "60:59:59"},
+                {null, "60:59"},
+                {null, "0:60"},
+                // Format checks
+                {null, "000:0"},
+                {null, "123:01"},
+                {null, "123:123"},
+                {null, "2:123"},
+                {null, "2:3"},
+                {null, "1:2:3"},
+                {null, ":3"},
+                {null, "01:"},
+                {null, ":01"},
+                {null, "a:b:c"},
+                {null, "abc:def:ghj"},
+                {null, "::"},
+                {null, ":"},
+                {null, ""}
+        });
+    }
+
+    @Test
+    public void testExtract() {
+        final Matcher m = TimestampExtractor.TIMESTAMPS_PATTERN.matcher(this.stringToProcess);
+
+        if (!m.find()) {
+            if (expected == null) {
+                return;
+            }
+            fail("No match found but expected one");
+        }
+
+        final TimestampExtractor.TimestampMatchDTO timestampMatchDTO =
+                TimestampExtractor
+                        .getTimestampFromMatcher(m, this.stringToProcess);
+
+        if (timestampMatchDTO == null) {
+            if (expected == null) {
+                return;
+            }
+            fail("Result shouldn't be null");
+        } else if (expected == null) {
+            assertNull("Expected that the dto is null, but it isn't", timestampMatchDTO);
+            return;
+        }
+
+        final int actualSeconds = timestampMatchDTO.seconds();
+
+        assertEquals(expected.getSeconds(), actualSeconds);
+    }
+}