From 7bb3bf7c74b9d783ace1653da8342e80abe50c0b Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sun, 4 Dec 2016 15:44:57 -0800 Subject: [PATCH] fix regression from commit 20bcbcc2 There were two places in the code that used the anti-pattern of returning True on success else an error message. In python you should always be able to replace `if x == True:` with just `if x:`. Which is what the lint tool recommended. Unfortunately I didn't notice how the return value was being used. This fixes that by changing the two affected functions to return an error message or None on success. This also adds `from __future__ import print_function` since the code uses the `print(msg)` function form rather than the `print msg` statement form. The former works by accident on python2 because the parens are interpreted as creating parenthesized expression that devolves to the single string inside the parens. So while the future import isn't strictly speaking necessary it will help avoid mistakes in the future if more complex `print()` calls are added. Partial fix for #3620 --- share/tools/web_config/webconfig.py | 33 +++++++++++++++-------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/share/tools/web_config/webconfig.py b/share/tools/web_config/webconfig.py index 87c276453..fcf79e0af 100755 --- a/share/tools/web_config/webconfig.py +++ b/share/tools/web_config/webconfig.py @@ -1,6 +1,7 @@ #!/usr/bin/env python from __future__ import unicode_literals +from __future__ import print_function import binascii import cgi import glob @@ -40,7 +41,6 @@ except ImportError: def run_fish_cmd(text): - from subprocess import PIPE # Ensure that fish is using UTF-8. ctype = os.environ.get("LC_ALL", os.environ.get("LC_CTYPE", os.environ.get("LANG"))) @@ -51,12 +51,13 @@ def run_fish_cmd(text): # Fish makes the same assumption in config.fish env = os.environ.copy() env.update(LC_CTYPE="en_US.UTF-8", LANG="en_US.UTF-8") - p = subprocess.Popen([FISH_BIN_PATH], stdin=PIPE, stdout=PIPE, stderr=PIPE, + p = subprocess.Popen([FISH_BIN_PATH], stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env) out, err = p.communicate(text.encode('utf-8')) out = out.decode('utf-8', 'replace') err = err.decode('utf-8', 'replace') - return(out, err) + return out, err def escape_fish_cmd(text): @@ -836,18 +837,18 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): def do_remove_abbreviation(self, abbreviation): out, err = run_fish_cmd('abbr --erase %s' % abbreviation['word']) - if out or err: + if err: return err else: - return True + return None def do_save_abbreviation(self, abbreviation): out, err = run_fish_cmd('abbr --add \'%s\' \'%s\'' % ( abbreviation['word'], abbreviation['phrase'])) - if out or err: - return out + if err: + return err else: - return True + return None def secure_startswith(self, haystack, needle): if len(haystack) < len(needle): @@ -975,17 +976,17 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): else: output = ["Unable to set prompt"] elif p == '/save_abbreviation/': - r = self.do_save_abbreviation(postvars) - if r: - output = ["OK"] + errmsg = self.do_save_abbreviation(postvars) + if errmsg: + output = [errmsg] else: - output = [r] + output = ["OK"] elif p == '/remove_abbreviation/': - r = self.do_remove_abbreviation(postvars) - if r: - output = ["OK"] + errmsg = self.do_remove_abbreviation(postvars) + if errmsg: + output = [errmsg] else: - output = [r] + output = ["OK"] else: return self.send_error(404)