[WIP] 91% test coverage of onionshare/common.py

.travis.yml

@@ -9,7 +9,8 @@ python:
   - "3.7-dev"
   - "nightly"
 # command to install dependencies
-before_install: "sudo apt-get update; sudo apt-get install -y python3-nose python3-flask python3-stem python3-pyqt5"
-install: ""
+# before_install: "sudo apt-get update; sudo apt-get install -y python3-pytest python3-flask python3-stem python3-pyqt5"
+install:
+  - pip3 install Flask==0.12 stem==1.5.4
 # command to run tests
-script: nosetests3
+script: pytest

CHANGELOG.md

@@ -5,7 +5,13 @@
 * OnionShare connects to Tor itself now, so opening Tor Browser in the background isn't required
 * In Windows and macOS, OnionShare alerts users about updates
 * Removed the menu bar, and adding a Settings button
-* Added desktop notifications
+* Added desktop notifications, and a system tray icon
+* Ability to add multiple files and folders with a single "Add" button
+* Ability to delete multiple files and folders at once with the "Delete" button
+* Hardened some response headers sent from the web server
+* Minor clarity improvements to the contents of the share's web page
+* Alert the user rather than share an empty archive if a file was unreadable
+* Prettier progress bars
 
 ## 1.0
 

onionshare/common.py

@@ -17,10 +17,22 @@
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """
-import sys, os, inspect, hashlib, base64, platform, zipfile, tempfile, math, time, socket, random
-from random import SystemRandom
+import base64
+import hashlib
+import inspect
+import math
+import os
+import platform
+import random
+import socket
+import sys
+import tempfile
+import time
+import zipfile
 
 debug = False
+
+
 def log(module, func, msg=None):
     """
     If debug mode is on, log error messages to stdout
@@ -34,6 +46,7 @@ def log(module, func, msg=None):
             final_msg = '{}: {}'.format(final_msg, msg)
         print(final_msg)
 
+
 def set_debug(new_debug):
     global debug
     debug = new_debug
@@ -71,6 +84,7 @@ def get_resource_path(filename):
 
     return os.path.join(prefix, filename)
 
+
 def get_tor_paths():
     p = get_platform()
     if p == 'Linux':
@@ -90,6 +104,7 @@ def get_tor_paths():
 
     return (tor_path, tor_geo_ip_file_path, tor_geo_ipv6_file_path)
 
+
 def get_version():
     """
     Returns the version of OnionShare that is running.
@@ -99,26 +114,6 @@ def get_version():
         return version
 
 
-def constant_time_compare(val1, val2):
-    """
-    Returns True if the two strings are equal, False otherwise.
-
-    The time taken is independent of the number of characters that match.
-
-    For the sake of simplicity, this function executes in constant time only
-    when the two strings have the same length. It short-circuits when they
-    have different lengths.
-
-    From: http://www.levigross.com/2014/02/07/constant-time-comparison-functions-in...-python-haskell-clojure-and-java/
-    """
-    if len(val1) != len(val2):
-        return False
-    result = 0
-    for x, y in zip(val1, val2):
-        result |= x ^ y
-    return result == 0
-
-
 def random_string(num_bytes, output_len=None):
     """
     Returns a random string with a specified number of bytes.
@@ -138,7 +133,7 @@ def build_slug():
     with open(get_resource_path('wordlist.txt')) as f:
         wordlist = f.read().split()
 
-    r = SystemRandom()
+    r = random.SystemRandom()
     return '-'.join(r.choice(wordlist) for _ in range(2))
 
 
@@ -160,30 +155,19 @@ def human_readable_filesize(b):
 
 def format_seconds(seconds):
     """Return a human-readable string of the format 1d2h3m4s"""
-    seconds_in_a_minute = 60
-    seconds_in_an_hour = seconds_in_a_minute * 60
-    seconds_in_a_day = seconds_in_an_hour * 24
-
-    days = math.floor(seconds / seconds_in_a_day)
-
-    hour_seconds = seconds % seconds_in_a_day
-    hours = math.floor(hour_seconds / seconds_in_an_hour)
-
-    minute_seconds = hour_seconds % seconds_in_an_hour
-    minutes = math.floor(minute_seconds / seconds_in_a_minute)
-
-    remaining_seconds = minute_seconds % seconds_in_a_minute
-    seconds = math.ceil(remaining_seconds)
+    days, seconds = divmod(seconds, 86400)
+    hours, seconds = divmod(seconds, 3600)
+    minutes, seconds = divmod(seconds, 60)
 
     human_readable = []
-    if days > 0:
-        human_readable.append("{}d".format(int(days)))
-    if hours > 0:
-        human_readable.append("{}h".format(int(hours)))
-    if minutes > 0:
-        human_readable.append("{}m".format(int(minutes)))
-    if seconds > 0:
-        human_readable.append("{}s".format(int(seconds)))
+    if days:
+        human_readable.append("{:.0f}d".format(days))
+    if hours:
+        human_readable.append("{:.0f}h".format(hours))
+    if minutes:
+        human_readable.append("{:.0f}m".format(minutes))
+    if seconds or not human_readable:
+        human_readable.append("{:.0f}s".format(seconds))
     return ''.join(human_readable)
 
 
@@ -200,16 +184,14 @@ def get_available_port(min_port, max_port):
     """
     Find a random available port within the given range.
     """
-    tmpsock = socket.socket()
-    while True:
-        try:
-            tmpsock.bind(("127.0.0.1", random.randint(min_port, max_port)))
-            break
-        except OSError:
-            pass
-    port = tmpsock.getsockname()[1]
-    tmpsock.close()
-
+    with socket.socket() as tmpsock:
+        while True:
+            try:
+                tmpsock.bind(("127.0.0.1", random.randint(min_port, max_port)))
+                break
+            except OSError:
+                pass
+        _, port = tmpsock.getsockname()
     return port
 
 

onionshare/web.py

@@ -18,7 +18,7 @@
 along with this program.  If not, see <http://www.gnu.org/licenses/>.
 """
 from distutils.version import StrictVersion as Version
-import queue, mimetypes, platform, os, sys, socket, logging
+import queue, mimetypes, platform, os, sys, socket, logging, hmac
 from urllib.request import urlopen
 
 from flask import Flask, Response, request, render_template_string, abort, make_response
@@ -162,7 +162,7 @@ def check_slug_candidate(slug_candidate, slug_compare = None):
     global slug
     if not slug_compare:
         slug_compare = slug
-    if not common.constant_time_compare(slug_compare.encode('ascii'), slug_candidate.encode('ascii')):
+    if not hmac.compare_digest(slug_compare, slug_candidate):
         abort(404)
 
 

onionshare_gui/downloads.py

@@ -32,14 +32,24 @@ def __init__(self, download_id, total_bytes):
         self.downloaded_bytes = 0
 
         # make a new progress bar
+        cssStyleData ="""
+        QProgressBar {
+            border: 2px solid grey;
+            border-radius: 5px;
+            text-align: center;
+        }
+
+        QProgressBar::chunk {
+            background: qlineargradient(x1: 0.5, y1: 0, x2: 0.5, y2: 1, stop: 0 #b366ff, stop: 1 #d9b3ff);
+            width: 10px;
+        }"""
         self.progress_bar = QtWidgets.QProgressBar()
         self.progress_bar.setTextVisible(True)
         self.progress_bar.setAlignment(QtCore.Qt.AlignHCenter)
         self.progress_bar.setMinimum(0)
         self.progress_bar.setMaximum(total_bytes)
         self.progress_bar.setValue(0)
-        self.progress_bar.setStyleSheet(
-            "QProgressBar::chunk { background-color: #05B8CC; }")
+        self.progress_bar.setStyleSheet(cssStyleData)
         self.progress_bar.total_bytes = total_bytes
 
         # start at 0

onionshare_gui/file_selection.py

@@ -35,6 +35,7 @@ def __init__(self, parent=None):
         self.setAcceptDrops(True)
         self.setIconSize(QtCore.QSize(32, 32))
         self.setSortingEnabled(True)
+        self.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection)
 
         class DropHereLabel(QtWidgets.QLabel):
             """
@@ -170,15 +171,12 @@ def __init__(self):
         self.file_list.files_dropped.connect(self.update)
 
         # buttons
-        self.add_files_button = QtWidgets.QPushButton(strings._('gui_add_files', True))
-        self.add_files_button.clicked.connect(self.add_files)
-        self.add_dir_button = QtWidgets.QPushButton(strings._('gui_add_folder', True))
-        self.add_dir_button.clicked.connect(self.add_dir)
+        self.add_button = QtWidgets.QPushButton(strings._('gui_add', True))
+        self.add_button.clicked.connect(self.add)
         self.delete_button = QtWidgets.QPushButton(strings._('gui_delete', True))
-        self.delete_button.clicked.connect(self.delete_file)
+        self.delete_button.clicked.connect(self.delete)
         button_layout = QtWidgets.QHBoxLayout()
-        button_layout.addWidget(self.add_files_button)
-        button_layout.addWidget(self.add_dir_button)
+        button_layout.addWidget(self.add_button)
         button_layout.addWidget(self.delete_button)
 
         # add the widgets
@@ -193,12 +191,10 @@ def update(self):
         """
         # all buttons should be disabled if the server is on
         if self.server_on:
-            self.add_files_button.setEnabled(False)
-            self.add_dir_button.setEnabled(False)
+            self.add_button.setEnabled(False)
             self.delete_button.setEnabled(False)
         else:
-            self.add_files_button.setEnabled(True)
-            self.add_dir_button.setEnabled(True)
+            self.add_button.setEnabled(True)
 
             # delete button should be disabled if item isn't selected
             current_item = self.file_list.currentItem()
@@ -210,52 +206,26 @@ def update(self):
         # update the file list
         self.file_list.update()
 
-    def add_files(self):
+    def add(self):
         """
-        Add files button clicked.
+        Add button clicked.
         """
-        file_dialog = QtWidgets.QFileDialog(caption=strings._('gui_choose_files', True))
-        file_dialog.setFileMode(QtWidgets.QFileDialog.ExistingFiles)
-        file_dialog.setOption(QtWidgets.QFileDialog.DontUseNativeDialog, True)
-        file_dialog.setOption(QtWidgets.QFileDialog.ReadOnly, True)
-        tree_view = file_dialog.findChild(QtWidgets.QTreeView)
-        tree_view.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection)
-        list_view = file_dialog.findChild(QtWidgets.QListView, "listView")
-        list_view.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection)
-
-        if file_dialog.exec_() == QtWidgets.QDialog.Accepted:
-          for filename in file_dialog.selectedFiles():
-              self.file_list.add_file(filename)
-
-        self.update()
-
-    def add_dir(self):
-        """
-        Add folder button clicked.
-        """
-        file_dialog = QtWidgets.QFileDialog(caption=strings._('gui_choose_folder', True))
-        file_dialog.setFileMode(QtWidgets.QFileDialog.Directory)
-        file_dialog.setOption(QtWidgets.QFileDialog.DontUseNativeDialog, True)
-        file_dialog.setOption(QtWidgets.QFileDialog.ReadOnly, True)
-        file_dialog.setOption(QtWidgets.QFileDialog.ShowDirsOnly, True)
-        tree_view = file_dialog.findChild(QtWidgets.QTreeView)
-        tree_view.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection)
-        list_view = file_dialog.findChild(QtWidgets.QListView, "listView")
-        list_view.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection)
-
+        file_dialog = FileDialog(caption=strings._('gui_choose_items', True))
         if file_dialog.exec_() == QtWidgets.QDialog.Accepted:
-          for filename in file_dialog.selectedFiles():
-              self.file_list.add_file(filename)
+            for filename in file_dialog.selectedFiles():
+                self.file_list.add_file(filename)
 
         self.update()
 
-    def delete_file(self):
+    def delete(self):
         """
         Delete button clicked
         """
-        current_row = self.file_list.currentRow()
-        self.file_list.filenames.pop(current_row)
-        self.file_list.takeItem(current_row)
+        selected = self.file_list.selectedItems()
+        for item in selected:
+            itemrow = self.file_list.row(item)
+            self.file_list.filenames.pop(itemrow)
+            self.file_list.takeItem(itemrow)
         self.update()
 
     def server_started(self):
@@ -285,3 +255,22 @@ def setFocus(self):
         Set the Qt app focus on the file selection box.
         """
         self.file_list.setFocus()
+
+class FileDialog(QtWidgets.QFileDialog):
+    """
+    Overridden version of QFileDialog which allows us to select
+    folders as well as, or instead of, files.
+    """
+    def __init__(self, *args, **kwargs):
+        QtWidgets.QFileDialog.__init__(self, *args, **kwargs)
+        self.setOption(self.DontUseNativeDialog, True)
+        self.setOption(self.ReadOnly, True)
+        self.setOption(self.ShowDirsOnly, False)
+        self.setFileMode(self.ExistingFiles)
+        tree_view = self.findChild(QtWidgets.QTreeView)
+        tree_view.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection)
+        list_view = self.findChild(QtWidgets.QListView, "listView")
+        list_view.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection)
+
+    def accept(self):
+        QtWidgets.QDialog.accept(self)