From 9e98ab181a927736b3f74084b71f36363df5b761 Mon Sep 17 00:00:00 2001
From: Daniel <kingdread@gmx.de>
Date: Sat, 8 Aug 2015 00:41:17 +0200
Subject: [PATCH] Add URL validity check + tests to incdec_number

---
 qutebrowser/utils/urlutils.py | 11 +++++------
 tests/utils/test_urlutils.py  | 26 +++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/qutebrowser/utils/urlutils.py b/qutebrowser/utils/urlutils.py
index a1348f41b..5d8987138 100644
--- a/qutebrowser/utils/urlutils.py
+++ b/qutebrowser/utils/urlutils.py
@@ -475,18 +475,17 @@ def incdec_number(url, incdec):
 
     Raises IncDecError if the url contains no number.
     """
+    if not url.isValid():
+        raise ValueError(get_errstring(url))
+
     path = url.path()
     # Get the last number in a string
     match = re.match(r'(.*\D|^)(\d+)(.*)', path)
     if not match:
         raise IncDecError("No number found in URL!", url)
     pre, number, post = match.groups()
-    if not number:
-        raise IncDecError("No number found in URL!", url)
-    try:
-        val = int(number)
-    except ValueError:
-        raise IncDecError("Could not parse number '{}'.".format(number), url)
+    # This should always succeed because we match \d+
+    val = int(number)
     if incdec == 'decrement':
         if val <= 0:
             raise IncDecError("Can't decrement {}!".format(val), url)
diff --git a/tests/utils/test_urlutils.py b/tests/utils/test_urlutils.py
index b6b41eef5..1db7e7a59 100644
--- a/tests/utils/test_urlutils.py
+++ b/tests/utils/test_urlutils.py
@@ -549,7 +549,7 @@ def test_incdec_number(url, incdec, output):
     "http://example.com/%C3%B6/urlencoded/data",
     "http://www2.ex4mple.com:42/all/of/the/%C3%A4bove",
 ])
-def test_incdec_number_invalid(url):
+def test_incdec_number_no_number(url):
     """Test incdec_number with URLs that don't contain a number."""
     with pytest.raises(urlutils.IncDecError):
         urlutils.incdec_number(QUrl(url), "increment")
@@ -560,3 +560,27 @@ def test_incdec_number_below_0():
     with pytest.raises(urlutils.IncDecError):
         urlutils.incdec_number(QUrl('http://example.com/page_0.html'),
                 'decrement')
+
+def test_incdec_number_invalid_url():
+    """Test if incdec_number rejects an invalid URL."""
+    with pytest.raises(ValueError):
+        urlutils.incdec_number(QUrl(""), "increment")
+
+def test_incdec_number_wrong_mode():
+    """Test if incdec_number rejects a wrong parameter for the incdec
+    argument."""
+    valid_url = QUrl("http://example.com/0")
+    with pytest.raises(ValueError):
+        urlutils.incdec_number(valid_url, "foobar")
+
+@pytest.mark.parametrize("url, msg, expected_str", [
+    ("http://example.com", "Invalid", "Invalid: http://example.com"),
+])
+def test_incdec_error(url, msg, expected_str):
+    """Test IncDecError."""
+    url = QUrl(url)
+    with pytest.raises(urlutils.IncDecError) as excinfo:
+        raise urlutils.IncDecError(msg, url)
+
+    assert excinfo.value.url == url
+    assert str(excinfo.value) == expected_str