Ensure validity of Qt objects

doc/HACKING.asciidoc

@@ -288,7 +288,8 @@ displaying it to the user.
 * Name a string URL something like `urlstr`, and a `QUrl` something like `url`.
 * Mention in the docstring whether your function needs a URL string or a
 `QUrl`.
-* Call QUrl.isValid() and take appropriate action if not.
+* Call `qt_ensure_valid` from `utils.misc` whenever getting or creating a
+`QUrl` and take appropriate action if not.
 
 
 Style conventions

qutebrowser/browser/commands.py

@@ -38,7 +38,7 @@ import qutebrowser.browser.quickmarks as quickmarks
 import qutebrowser.utils.log as log
 import qutebrowser.utils.url as urlutils
 from qutebrowser.utils.misc import (check_overflow, shell_escape,
-                                    check_print_compat)
+                                    check_print_compat, qt_ensure_valid)
 from qutebrowser.utils.editor import ExternalEditor
 from qutebrowser.commands.exceptions import CommandError
 from qutebrowser.commands.userscripts import UserscriptRunner
@@ -70,6 +70,12 @@ class CommandDispatcher:
         self._tabs = parent
         self._editor = None
 
+    def _current_url(self):
+        """Get the URL of the current tab."""
+        url = self._tabs.currentWidget().url()
+        qt_ensure_valid(url)
+        return url
+
     def _scroll_percent(self, perc=None, count=None, orientation=None):
         """Inner logic for scroll_percent_(x|y).
 
@@ -97,7 +103,8 @@ class CommandDispatcher:
         frame = widget.page().currentFrame()
         if frame is None:
             raise CommandError("No frame focused!")
-        widget.hintmanager.follow_prevnext(frame, widget.url(), prev, newtab)
+        widget.hintmanager.follow_prevnext(frame, self._current_url(), prev,
+                                           newtab)
 
     def _tab_move_absolute(self, idx):
         """Get an index for moving a tab absolutely.
@@ -270,7 +277,7 @@ class CommandDispatcher:
             target = getattr(hints.Target, targetstr.replace('-', '_'))
         except AttributeError:
             raise CommandError("Unknown hinting target {}!".format(targetstr))
-        widget.hintmanager.start(frame, widget.url(), group, target)
+        widget.hintmanager.start(frame, self._current_url(), group, target)
 
     @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True)
     def follow_hint(self):
@@ -356,8 +363,8 @@ class CommandDispatcher:
         Args:
             sel: True to use primary selection, False to use clipboard
         """
-        urlstr = self._tabs.currentWidget().url().toString(
+        urlstr = self._current_url().toString(QUrl.FullyEncoded |
-            QUrl.FullyEncoded | QUrl.RemovePassword)
+                                              QUrl.RemovePassword)
         if sel:
             mode = QClipboard.Selection
             target = "primary selection"
@@ -450,19 +457,19 @@ class CommandDispatcher:
     @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True)
     def open_tab_cur(self):
         """Set the statusbar to :tabopen and the current URL."""
-        urlstr = self._tabs.currentWidget().url().toDisplayString()
+        urlstr = self._current_url().toDisplayString()
         message.set_cmd_text(':open-tab ' + urlstr)
 
     @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True)
     def open_cur(self):
         """Set the statusbar to :open and the current URL."""
-        urlstr = self._tabs.currentWidget().url().toDisplayString()
+        urlstr = self._current_url().toDisplayString()
         message.set_cmd_text(':open ' + urlstr)
 
     @cmdutils.register(instance='mainwindow.tabs.cmd', hide=True)
     def open_tab_bg_cur(self):
         """Set the statusbar to :tabopen-bg and the current URL."""
-        urlstr = self._tabs.currentWidget().url().toDisplayString()
+        urlstr = self._current_url().toDisplayString()
         message.set_cmd_text(':open-tab-bg ' + urlstr)
 
     @cmdutils.register(instance='mainwindow.tabs.cmd')
@@ -609,8 +616,8 @@ class CommandDispatcher:
         Args:
             cmd: The command to execute.
         """
-        urlstr = self._tabs.currentWidget().url().toString(
+        urlstr = self._current_url().toString(QUrl.FullyEncoded |
-            QUrl.FullyEncoded | QUrl.RemovePassword)
+                                              QUrl.RemovePassword)
         cmd = cmd.replace('{}', shell_escape(urlstr))
         log.procs.debug("Executing: {}".format(cmd))
         subprocess.Popen(cmd, shell=True)
@@ -625,7 +632,7 @@ class CommandDispatcher:
         """Run an userscript given as argument."""
         # We don't remove the password in the URL here, as it's probably safe
         # to pass via env variable.
-        urlstr = self._tabs.currentWidget().url().toString(QUrl.FullyEncoded)
+        urlstr = self._current_url().toString(QUrl.FullyEncoded)
         runner = UserscriptRunner(self._tabs)
         runner.got_cmd.connect(self._tabs.got_cmd)
         runner.run(cmd, *args, env={'QUTE_URL': urlstr})
@@ -634,7 +641,7 @@ class CommandDispatcher:
     @cmdutils.register(instance='mainwindow.tabs.cmd')
     def quickmark_save(self):
         """Save the current page as a quickmark."""
-        quickmarks.prompt_save(self._tabs.currentWidget().url())
+        quickmarks.prompt_save(self._current_url())
 
     @cmdutils.register(instance='mainwindow.tabs.cmd')
     def quickmark_load(self, name):
@@ -642,7 +649,8 @@ class CommandDispatcher:
         urlstr = quickmarks.get(name)
         url = QUrl(urlstr)
         if not url.isValid():
-            raise CommandError("Invalid URL {}".format(urlstr))
+            raise CommandError("Invalid URL {} ({})".format(
+                urlstr, url.errorString()))
         self._tabs.currentWidget().openurl(url)
 
     @cmdutils.register(instance='mainwindow.tabs.cmd')
@@ -680,8 +688,7 @@ class CommandDispatcher:
     @cmdutils.register(instance='mainwindow.tabs.cmd')
     def download_page(self):
         """Download the current page."""
-        url = self._tabs.currentWidget().url()
+        QApplication.instance().downloadmanager.get(self._current_url())
-        QApplication.instance().downloadmanager.get(url)
 
     @cmdutils.register(instance='mainwindow.tabs.cmd', modes=['insert'],
                        hide=True)

qutebrowser/browser/downloads.py

@@ -36,7 +36,7 @@ from qutebrowser.utils.log import downloads as logger
 from qutebrowser.utils.log import fix_rfc2622
 from qutebrowser.utils.usertypes import PromptMode, Question, Timer
 from qutebrowser.utils.misc import (interpolate_color, format_seconds,
-                                    format_size)
+                                    format_size, qt_ensure_valid)
 from qutebrowser.commands.exceptions import CommandError
 
 
@@ -366,6 +366,7 @@ class DownloadManager(QObject):
         Args:
             url: The URL to get, as QUrl
         """
+        qt_ensure_valid(url)
         req = QNetworkRequest(url)
         reply = QCoreApplication.instance().networkmanager.get(req)
         self.fetch(reply)

qutebrowser/browser/hints.py

@@ -33,6 +33,7 @@ import qutebrowser.utils.webelem as webelem
 from qutebrowser.commands.exceptions import CommandError
 from qutebrowser.utils.usertypes import enum
 from qutebrowser.utils.log import hints as logger
+from qutebrowser.utils.misc import qt_ensure_valid
 
 
 ElemTuple = namedtuple('ElemTuple', 'elem, label')
@@ -299,6 +300,7 @@ class HintManager(QObject):
         Args:
             url: The URL to open as a QURL.
         """
+        qt_ensure_valid(url)
         sel = self._context.target == Target.yank_primary
         mode = QClipboard.Selection if sel else QClipboard.Clipboard
         urlstr = url.toString(QUrl.FullyEncoded | QUrl.RemovePassword)
@@ -312,6 +314,7 @@ class HintManager(QObject):
         Args:
             url: The URL to open as a QUrl.
         """
+        qt_ensure_valid(url)
         commands = {
             Target.cmd: 'open',
             Target.cmd_tab: 'open-tab',
@@ -326,6 +329,7 @@ class HintManager(QObject):
         Args:
             url: The URL to download, as a QUrl.
         """
+        qt_ensure_valid(url)
         QApplication.instance().downloadmanager.get(url)
 
     def _resolve_url(self, elem, baseurl=None):
@@ -347,6 +351,7 @@ class HintManager(QObject):
         url = QUrl(text)
         if url.isRelative():
             url = baseurl.resolved(url)
+        qt_ensure_valid(url)
         return url
 
     def _find_prevnext(self, frame, prev=False):

qutebrowser/browser/quickmarks.py

@@ -33,7 +33,7 @@ import qutebrowser.utils.message as message
 import qutebrowser.commands.utils as cmdutils
 from qutebrowser.utils.usertypes import PromptMode
 from qutebrowser.config.lineparser import LineConfigParser
-from qutebrowser.utils.misc import get_standard_dir
+from qutebrowser.utils.misc import get_standard_dir, qt_ensure_valid
 from qutebrowser.commands.exceptions import CommandError
 
 
@@ -63,6 +63,7 @@ def prompt_save(url):
     Args:
         url: The quickmark url as a QUrl.
     """
+    qt_ensure_valid(url)
     urlstr = url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
     message.question("Add quickmark:", PromptMode.text,
                      partial(quickmark_add, urlstr))
@@ -81,6 +82,8 @@ def quickmark_add(url, name):
     if not url:
         raise CommandError("Can't set mark with empty URL!")
 
+    qt_ensure_valid(url)
+
     def set_mark():
         """Really set the quickmark."""
         marks[name] = url
@@ -99,6 +102,6 @@ def get(name):
     urlstr = marks[name]
     url = QUrl(urlstr)
     if not url.isValid():
-        raise CommandError("Invalid URL for quickmark {}: {}".format(
+        raise CommandError("Invalid URL for quickmark {}: {} ({})".format(
-            name, urlstr))
+            name, urlstr, url.errorString()))
     return url

qutebrowser/config/configdata.py

@@ -767,7 +767,7 @@ DATA = OrderedDict([
 
     ('colors', sect.KeyValue(
         ('completion.fg',
-         SettingValue(types.Color(), 'white'),
+         SettingValue(types.QtColor(), 'white'),
          "Text color of the completion widget."),
 
         ('completion.bg',
@@ -779,7 +779,7 @@ DATA = OrderedDict([
          "Background color of completion widget items."),
 
         ('completion.category.fg',
-         SettingValue(types.Color(), 'white'),
+         SettingValue(types.QtColor(), 'white'),
          "Foreground color of completion widget category headers."),
 
         ('completion.category.bg',
@@ -796,7 +796,7 @@ DATA = OrderedDict([
          "Bottom border color of the completion widget category headers."),
 
         ('completion.item.selected.fg',
-         SettingValue(types.Color(), 'black'),
+         SettingValue(types.QtColor(), 'black'),
          "Foreground color of the selected completion item."),
 
         ('completion.item.selected.bg',

qutebrowser/config/conftypes.py

@@ -751,7 +751,10 @@ class Proxy(BaseType):
         if value in self.valid_values:
             return
         url = QUrl(value)
-        if (url.isValid() and not url.isEmpty() and
+        if not url.isValid():
+            raise ValidationError(value, "invalid url, {}".format(
+                url.errorString()))
+        elif (url.isValid() and not url.isEmpty() and
                 url.scheme() in self.PROXY_TYPES):
             if url.userName() and url.password():
                 pass

qutebrowser/models/basecompletion.py

@@ -27,6 +27,7 @@ from PyQt5.QtCore import Qt
 from PyQt5.QtGui import QStandardItemModel, QStandardItem
 
 from qutebrowser.utils.usertypes import enum
+from qutebrowser.utils.misc import qt_ensure_valid
 
 
 Role = enum('marks', 'sort', start=Qt.UserRole)
@@ -73,6 +74,7 @@ class BaseCompletionModel(QStandardItemModel):
             index: A QModelIndex of the item to mark.
             needle: The string to mark.
         """
+        qt_ensure_valid(index)
         haystack = self.data(index)
         marks = self._get_marks(needle, haystack)
         self.setData(index, marks, Role.marks)
@@ -128,8 +130,7 @@ class BaseCompletionModel(QStandardItemModel):
         Return:
             The item flags, or Qt.NoItemFlags on error.
         """
-        if not index.isValid():
+        qt_ensure_valid(index)
-            return Qt.NoItemFlags
         if index.parent().isValid():
             # item
             return Qt.ItemIsEnabled | Qt.ItemIsSelectable

qutebrowser/models/downloadmodel.py

@@ -26,6 +26,7 @@ from PyQt5.QtWidgets import QApplication
 import qutebrowser.config.config as config
 from qutebrowser.utils.usertypes import enum
 from qutebrowser.utils.log import downloads as logger
+from qutebrowser.utils.misc import qt_ensure_valid
 
 
 Role = enum('item', start=Qt.UserRole)
@@ -73,9 +74,8 @@ class DownloadModel(QAbstractListModel):
 
     def data(self, index, role):
         """Download data from DownloadManager."""
-        if not index.isValid():
+        qt_ensure_valid(index)
-            return QVariant()
+        if index.parent().isValid() or index.column() != 0:
-        elif index.parent().isValid() or index.column() != 0:
             return QVariant()
 
         try:

qutebrowser/utils/misc.py

@@ -331,10 +331,8 @@ def interpolate_color(start, end, percent, colorspace=QColor.Rgb):
     Raise:
         ValueError if invalid parameters are passed.
     """
-    if not start.isValid():
+    qt_ensure_valid(start)
-        raise ValueError("Invalid start color")
+    qt_ensure_valid(end)
-    if not end.isValid():
-        raise ValueError("Invalid end color")
     out = QColor()
     if colorspace == QColor.Rgb:
         a_c1, a_c2, a_c3, _alpha = start.getRgb()
@@ -356,7 +354,9 @@ def interpolate_color(start, end, percent, colorspace=QColor.Rgb):
         out.setHsl(*components)
     else:
         raise ValueError("Invalid colorspace!")
-    return out.convertTo(start.spec())
+    out = out.convertTo(start.spec())
+    qt_ensure_valid(out)
+    return out
 
 
 def format_seconds(total_seconds):
@@ -395,6 +395,31 @@ def check_print_compat():
     return not (os.name == 'nt' and qt_version_check('5.3.0', operator.lt))
 
 
+def qt_ensure_valid(obj):
+    """Ensure a Qt object with an .isValid() method is valid.
+
+    Raise:
+        QtValueError if the object is invalid.
+    """
+    if not obj.isValid():
+        raise QtValueError(obj)
+
+
+class QtValueError(ValueError):
+
+    """Exception which gets raised by qt_ensure_valid."""
+
+    def __init__(self, obj):
+        try:
+            self.reason = obj.errorString()
+        except AttributeError:
+            self.reason = None
+        err = "{} is not valid".format(obj)
+        if self.reason:
+            err += ": {}".format(self.reason)
+        super().__init__(err)
+
+
 class EventLoop(QEventLoop):
 
     """A thin wrapper around QEventLoop.

qutebrowser/utils/url.py

@@ -28,6 +28,7 @@ from PyQt5.QtNetwork import QHostInfo
 
 import qutebrowser.config.config as config
 from qutebrowser.utils.log import url as logger
+from qutebrowser.utils.misc import qt_ensure_valid
 
 
 # FIXME: we probably could raise some exceptions on invalid URLs
@@ -63,7 +64,9 @@ def _get_search_url(txt):
         logger.debug("engine: default, term '{}'".format(txt))
     if not term:
         raise FuzzyUrlError("No search term given")
-    return QUrl.fromUserInput(template.format(urllib.parse.quote(term)))
+    url = QUrl.fromUserInput(template.format(urllib.parse.quote(term)))
+    qt_ensure_valid(url)
+    return url
 
 
 def _is_url_naive(urlstr):
@@ -131,6 +134,7 @@ def fuzzy_url(urlstr):
             url = QUrl.fromUserInput(urlstr)
     logger.debug("Converting fuzzy term {} to URL -> {}".format(
                  urlstr, url.toDisplayString()))
+    qt_ensure_valid(url)
     return url
 
 

qutebrowser/utils/webelem.py

@@ -94,8 +94,10 @@ def is_visible(elem, mainframe):
     # Then check if it's visible in its frame if it's not in the main frame.
     elem_frame = elem.webFrame()
     elem_rect = elem.geometry()
-    if elem_frame.parentFrame() is not None:
     framegeom = QRect(elem_frame.geometry())
+    if not framegeom.isValid():
+        visible_in_frame = False
+    elif elem_frame.parentFrame() is not None:
         framegeom.moveTo(0, 0)
         framegeom.translate(elem_frame.scrollPosition())
         if elem_rect.isValid():

qutebrowser/widgets/completiondelegate.py

@@ -26,7 +26,7 @@ import html
 from PyQt5.QtWidgets import QStyle, QStyleOptionViewItem, QStyledItemDelegate
 from PyQt5.QtCore import QRectF, QSize, Qt
 from PyQt5.QtGui import (QIcon, QPalette, QTextDocument, QTextOption,
-                         QTextCursor, QColor, QAbstractTextDocumentLayout)
+                         QTextCursor, QAbstractTextDocumentLayout)
 
 import qutebrowser.config.config as config
 from qutebrowser.models.basecompletion import Role
@@ -147,9 +147,9 @@ class CompletionItemDelegate(QStyledItemDelegate):
         else:
             option = 'completion.fg'
         try:
-            self._painter.setPen(QColor(config.get('colors', option)))
+            self._painter.setPen(config.get('colors', option))
         except config.NoOptionError:
-            self._painter.setPen(QColor(config.get('colors', 'completion.fg')))
+            self._painter.setPen(config.get('colors', 'completion.fg'))
         ctx = QAbstractTextDocumentLayout.PaintContext()
         ctx.palette.setColor(QPalette.Text, self._painter.pen().color())
         if clip.isValid():

qutebrowser/widgets/downloads.py

@@ -24,6 +24,7 @@ from PyQt5.QtWidgets import QListView, QSizePolicy, QMenu
 
 from qutebrowser.models.downloadmodel import DownloadModel, Role
 from qutebrowser.config.style import set_register_stylesheet
+from qutebrowser.utils.misc import qt_ensure_valid
 
 
 class DownloadView(QListView):
@@ -87,6 +88,8 @@ class DownloadView(QListView):
         idx = self.model().last_index()
         height = self.visualRect(idx).bottom()
         if height != -1:
-            return QSize(0, height + 2)
+            size = QSize(0, height + 2)
         else:
-            return QSize(0, 0)
+            size = QSize(0, 0)
+        qt_ensure_valid(size)
+        return size

qutebrowser/widgets/mainwindow.py

@@ -129,7 +129,9 @@ class MainWindow(QWidget):
         topleft_y = utils.check_overflow(topleft_y, 'int', fatal=False)
         topleft = QPoint(0, topleft_y)
         bottomright = self.status.geometry().topRight()
-        self.completion.setGeometry(QRect(topleft, bottomright))
+        rect = QRect(topleft, bottomright)
+        utils.qt_ensure_valid(rect)
+        self.completion.setGeometry(rect)
 
     @cmdutils.register(instance='mainwindow', name=['quit', 'q'], nargs=0)
     def close(self):

qutebrowser/widgets/tabbedbrowser.py

@@ -32,6 +32,7 @@ from qutebrowser.widgets.tabwidget import TabWidget, EmptyTabIcon
 from qutebrowser.widgets.webview import WebView
 from qutebrowser.browser.signalfilter import SignalFilter
 from qutebrowser.browser.commands import CommandDispatcher
+from qutebrowser.utils.misc import qt_ensure_valid
 
 
 class TabbedBrowser(TabWidget):
@@ -247,6 +248,7 @@ class TabbedBrowser(TabWidget):
             url: The URL to open as QUrl.
             newtab: True to open URL in a new tab, False otherwise.
         """
+        qt_ensure_valid(url)
         if newtab:
             self.tabopen(url, background=False)
         else:
@@ -277,6 +279,7 @@ class TabbedBrowser(TabWidget):
         Return:
             The opened WebView instance.
         """
+        qt_ensure_valid(url)
         log.webview.debug("Creating new tab with URL {}".format(url))
         tab = WebView(self)
         self._connect_tab_signals(tab)

qutebrowser/widgets/webview.py

@@ -31,7 +31,7 @@ import qutebrowser.keyinput.modeman as modeman
 import qutebrowser.utils.message as message
 import qutebrowser.utils.webelem as webelem
 import qutebrowser.utils.log as log
-from qutebrowser.utils.misc import elide
+from qutebrowser.utils.misc import elide, qt_ensure_valid
 from qutebrowser.browser.webpage import BrowserPage
 from qutebrowser.browser.hints import HintManager
 from qutebrowser.utils.usertypes import NeighborList, enum
@@ -293,6 +293,7 @@ class WebView(QWebView):
         Emit:
             titleChanged
         """
+        qt_ensure_valid(url)
         urlstr = url.toDisplayString()
         log.webview.debug("New title: {}".format(urlstr))
         self.titleChanged.emit(urlstr)
@@ -379,6 +380,7 @@ class WebView(QWebView):
     @pyqtSlot('QUrl')
     def on_url_changed(self, url):
         """Update url_text when URL has changed."""
+        qt_ensure_valid(url)
         self.url_text = url.toString()
 
     @pyqtSlot(str)
@@ -394,6 +396,7 @@ class WebView(QWebView):
         url = QUrl(urlstr)
         if not url.isValid():
             message.error("Invalid link {} clicked!".format(urlstr))
+            log.webview.debug(url.errorString())
             return
         if self._open_target == Target.tab:
             self.tabbedbrowser.tabopen(url, False)