From ea8321d66900c06a0b6dc348fa31ff0af4f855c2 Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Wed, 29 Jul 2020 15:30:38 -0700 Subject: [PATCH] helper: Secure Linux & MacOS notifications & remove WSL for now. Tests amended to updated lengths and xfail. --- tests/helper/test_helper.py | 10 ++++++---- zulipterminal/helper.py | 35 ++++++++--------------------------- 2 files changed, 14 insertions(+), 31 deletions(-) diff --git a/tests/helper/test_helper.py b/tests/helper/test_helper.py index 39906b1739..05bb0cbc5a 100644 --- a/tests/helper/test_helper.py +++ b/tests/helper/test_helper.py @@ -236,7 +236,8 @@ def test_invalid_color_format(mocker, color): @pytest.mark.parametrize('OS, is_notification_sent', [ - ([True, False, False], True), # OS: [WSL, MACOS, LINUX] + pytest.param([True, False, False], True, # OS: [WSL, MACOS, LINUX] + marks=pytest.mark.xfail(reason="WSL notify disabled")), ([False, True, False], True), ([False, False, True], True), ([False, False, False], False), # Unsupported OS @@ -259,9 +260,10 @@ def test_notify(mocker, OS, is_notification_sent): "X", 'Spaced title', "'", '"' ], ids=["X", "spaced_title", "single", "double"]) @pytest.mark.parametrize('OS, cmd_length', [ - ('LINUX', 3), - ('MACOS', 3), - ('WSL', 2) + ('LINUX', 4), + ('MACOS', 10), + pytest.param('WSL', 2, + marks=pytest.mark.xfail(reason="WSL notify disabled")) ]) def test_notify_quotes(monkeypatch, mocker, OS, cmd_length, title, text): diff --git a/zulipterminal/helper.py b/zulipterminal/helper.py index d5ccbacfab..cfb69d63a5 100644 --- a/zulipterminal/helper.py +++ b/zulipterminal/helper.py @@ -1,6 +1,5 @@ import os import platform -import shlex import subprocess import time from collections import OrderedDict, defaultdict @@ -591,39 +590,21 @@ def canonicalize_color(color: str) -> str: def notify(title: str, html_text: str) -> str: document = lxml.html.document_fromstring(html_text) text = document.text_content() - quoted_text = shlex.quote(text) - - quoted_title = shlex.quote(title) command_list = None - if WSL: # NOTE Tested and should work! - # Escaping of quotes in powershell is done using ` instead of \ - escaped_text = text.replace('\'', '`\'').replace('\"', '`\"') - quoted_text = '\"' + escaped_text + '\"' + if MACOS: command_list = [ - 'powershell.exe', - "New-BurntToastNotification -Text {}, {}" - .format(quoted_title, quoted_text) + "osascript", + "-e", "on run(argv)", + "-e", "return display notification item 1 of argv with title " + 'item 2 of argv sound name "ZT_NOTIFICATION_SOUND"', + "-e", "end", + "--", text, title ] - expected_length = 2 - elif MACOS: # NOTE Tested and should work! - command_list = shlex.split( - "osascript -e " - "'display notification \"\'{}\'\" with title \"\'{}\'\" " - " sound name \"ZT_NOTIFICATION_SOUND\"'" - .format(quoted_text, quoted_title) - ) - expected_length = 3 elif LINUX: - command_list = shlex.split( - 'notify-send {} {}'.format(quoted_title, quoted_text) - ) - expected_length = 3 + command_list = ["notify-send", "--", title, text] if command_list is not None: - # NOTE: We assert this in tests, but this signals unexpected breakage - assert len(command_list) == expected_length - try: subprocess.run(command_list, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)