From f202f338a47d7e2a7e14bb7b14e5c934356ad66e Mon Sep 17 00:00:00 2001 From: Jakub Roztocil Date: Mon, 2 Dec 2019 17:43:16 +0100 Subject: [PATCH] Remove automatic config file creation to avoid concurrency issues. Close #788 Close #812 --- CHANGELOG.rst | 2 +- README.rst | 56 ++++++++++++++++++++++++++--------------- httpie/cli/argparser.py | 3 +-- httpie/client.py | 2 +- httpie/config.py | 54 +++++++++++++++++++++------------------ httpie/context.py | 32 +++++++++++++++-------- httpie/core.py | 49 ++++++++++++++++-------------------- httpie/sessions.py | 9 +++---- tests/test_config.py | 44 +++++++++++++++++++++----------- tests/test_errors.py | 37 +++++++++++---------------- tests/utils.py | 31 +++++++++++++++-------- 11 files changed, 178 insertions(+), 141 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9e15219c..c16e5708 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,6 +11,7 @@ This project adheres to `Semantic Versioning `_. * Removed Python 2.7 support (`EOL Jan 2020 `_). * Removed the default 30-second connection ``--timeout`` limit. * Removed Python’s default limit of 100 response headers. +* Removed automatic config file creation to avoid concurrency issues. * Replaced the old collect-all-then-process handling of HTTP communication with one-by-one processing of each HTTP request or response as they become available. This means that you can see headers immediately, @@ -26,7 +27,6 @@ This project adheres to `Semantic Versioning `_. * Added ``tests/`` to the PyPi package for the convenience of downstream package maintainers. * Fixed an error when ``stdin`` was a closed fd. -* Fixed an error when the config directory was not writeable. * Improved ``--debug`` output formatting. diff --git a/README.rst b/README.rst index daed9888..6a32f0ff 100644 --- a/README.rst +++ b/README.rst @@ -1487,7 +1487,8 @@ To create or reuse a different session, simple specify a different name: $ http --session=user2 -a user2:password example.org X-Bar:Foo -Named sessions' data is stored in JSON files in the directory +Named sessions’s data is stored in JSON files in the the ``sessions`` +subdirectory of the `config`_ directory: ``~/.httpie/sessions//.json`` (``%APPDATA%\httpie\sessions\\.json`` on Windows). @@ -1517,46 +1518,61 @@ exchange once it is created, specify the session name via Config ====== -HTTPie uses a simple JSON config file. +HTTPie uses a simple ``config.json`` file. The file doesn’t exist by default +but you can create it manually. - -Config file location --------------------- - +Config file directory +--------------------- The default location of the configuration file is ``~/.httpie/config.json`` -(or ``%APPDATA%\httpie\config.json`` on Windows). The config directory -location can be changed by setting the ``HTTPIE_CONFIG_DIR`` -environment variable. To view the exact location run ``http --debug``. +(or ``%APPDATA%\httpie\config.json`` on Windows). + +The config directory can be changed by setting the ``$HTTPIE_CONFIG_DIR`` +environment variable: + +.. code-block:: bash + + $ export HTTPIE_CONFIG_DIR=/tmp/httpie + $ http example.org + +To view the exact location run ``http --debug``. + Configurable options -------------------- -The JSON file contains an object with the following keys: +Currently HTTPie offers a single configurable option: ``default_options`` ~~~~~~~~~~~~~~~~~~~ - An ``Array`` (by default empty) of default options that should be applied to every invocation of HTTPie. -For instance, you can use this option to change the default style and output -options: ``"default_options": ["--style=fruity", "--body"]`` Another useful -default option could be ``"--session=default"`` to make HTTPie always -use `sessions`_ (one named ``default`` will automatically be used). -Or you could change the implicit request content type from JSON to form by -adding ``--form`` to the list. +For instance, you can use this config option to change your default color theme: -``__meta__`` -~~~~~~~~~~~~ +.. code-block:: bash -HTTPie automatically stores some of its metadata here. Please do not change. + $ cat ~/.httpie/config.json +.. code-block:: json + + { + "default_options": [ + "--style=fruity" + ] + } + + +Even though it is technically possible to include there any of HTTPie’s +options, it is not recommended to modify the default behaviour in a way +that would break your compatibility with the wider world as that can +generate a lot of confusion. + Un-setting previously specified options --------------------------------------- diff --git a/httpie/cli/argparser.py b/httpie/cli/argparser.py index fb7d567b..4924552e 100644 --- a/httpie/cli/argparser.py +++ b/httpie/cli/argparser.py @@ -61,7 +61,6 @@ class HTTPieArgumentParser(argparse.ArgumentParser): def parse_args( self, env: Environment, - program_name='http', args=None, namespace=None ) -> argparse.Namespace: @@ -89,7 +88,7 @@ class HTTPieArgumentParser(argparse.ArgumentParser): if self.has_stdin_data: self._body_from_file(self.env.stdin) if not URL_SCHEME_RE.match(self.args.url): - if os.path.basename(program_name) == 'https': + if os.path.basename(env.program_name) == 'https': scheme = 'https://' else: scheme = self.args.default_scheme + "://" diff --git a/httpie/client.py b/httpie/client.py index 175a89b3..34aa2f10 100644 --- a/httpie/client.py +++ b/httpie/client.py @@ -35,7 +35,7 @@ DEFAULT_UA = f'HTTPie/{__version__}' def collect_messages( args: argparse.Namespace, - config_dir: Path + config_dir: Path, ) -> Iterable[Union[requests.PreparedRequest, requests.Response]]: httpie_session = None httpie_session_headers = None diff --git a/httpie/config.py b/httpie/config.py index e7991f12..82193743 100644 --- a/httpie/config.py +++ b/httpie/config.py @@ -15,42 +15,43 @@ DEFAULT_CONFIG_DIR = Path(os.environ.get( )) +class ConfigFileError(Exception): + pass + + class BaseConfigDict(dict): name = None helpurl = None about = None - def _get_path(self) -> Path: - """Return the config file path without side-effects.""" - raise NotImplementedError() + def __init__(self, path: Path): + super().__init__() + self.path = path - def path(self) -> Path: - """Return the config file path creating basedir, if needed.""" - path = self._get_path() + def ensure_directory(self): try: - path.parent.mkdir(mode=0o700, parents=True) + self.path.parent.mkdir(mode=0o700, parents=True) except OSError as e: if e.errno != errno.EEXIST: raise - return path def is_new(self) -> bool: - return not self._get_path().exists() + return not self.path.exists() def load(self): + config_type = type(self).__name__.lower() try: - with self.path().open('rt') as f: + with self.path.open('rt') as f: try: data = json.load(f) except ValueError as e: - raise ValueError( - 'Invalid %s JSON: %s [%s]' % - (type(self).__name__, str(e), self.path()) + raise ConfigFileError( + f'invalid {config_type} file: {e} [{self.path}]' ) self.update(data) except IOError as e: if e.errno != errno.ENOENT: - raise + raise ConfigFileError(f'cannot read {config_type} file: {e}') def save(self, fail_silently=False): self['__meta__'] = { @@ -62,9 +63,17 @@ class BaseConfigDict(dict): if self.about: self['__meta__']['about'] = self.about + self.ensure_directory() + try: - with self.path().open('w') as f: - json.dump(self, f, indent=4, sort_keys=True, ensure_ascii=True) + with self.path.open('w') as f: + json.dump( + obj=self, + fp=f, + indent=4, + sort_keys=True, + ensure_ascii=True, + ) f.write('\n') except IOError: if not fail_silently: @@ -72,27 +81,22 @@ class BaseConfigDict(dict): def delete(self): try: - self.path().unlink() + self.path.unlink() except OSError as e: if e.errno != errno.ENOENT: raise class Config(BaseConfigDict): - name = 'config' - helpurl = 'https://httpie.org/doc#config' - about = 'HTTPie configuration file' + FILENAME = 'config.json' DEFAULTS = { 'default_options': [] } def __init__(self, directory: Union[str, Path] = DEFAULT_CONFIG_DIR): - super().__init__() - self.update(self.DEFAULTS) self.directory = Path(directory) - - def _get_path(self) -> Path: - return self.directory / (self.name + '.json') + super().__init__(path=self.directory / self.FILENAME) + self.update(self.DEFAULTS) @property def default_options(self) -> list: diff --git a/httpie/context.py b/httpie/context.py index 4c19fc2f..4cd8214b 100644 --- a/httpie/context.py +++ b/httpie/context.py @@ -1,3 +1,4 @@ +import os import sys from pathlib import Path from typing import Union, IO, Optional @@ -9,7 +10,7 @@ except ImportError: curses = None # Compiled w/o curses from httpie.compat import is_windows -from httpie.config import DEFAULT_CONFIG_DIR, Config +from httpie.config import DEFAULT_CONFIG_DIR, Config, ConfigFileError from httpie.utils import repr_dict @@ -35,6 +36,7 @@ class Environment: stderr: IO = sys.stderr stderr_isatty: bool = stderr.isatty() colors = 256 + program_name: str = 'http' if not is_windows: if curses: try: @@ -79,16 +81,6 @@ class Environment: self.stdout_encoding = getattr( actual_stdout, 'encoding', None) or 'utf8' - @property - def config(self) -> Config: - if not hasattr(self, '_config'): - self._config = Config(directory=self.config_dir) - if self._config.is_new(): - self._config.save(fail_silently=True) - else: - self._config.load() - return self._config - def __str__(self): defaults = dict(type(self).__dict__) actual = dict(defaults) @@ -102,3 +94,21 @@ class Environment: def __repr__(self): return f'<{type(self).__name__} {self}>' + + _config: Config = None + + @property + def config(self) -> Config: + config = self._config + if not config: + self._config = config = Config(directory=self.config_dir) + if not config.is_new(): + try: + config.load() + except ConfigFileError as e: + self.log_error(e, level='warning') + return config + + def log_error(self, msg, level='error'): + assert level in ['error', 'warning'] + self.stderr.write(f'\n{self.program_name}: {level}: {msg}\n\n') diff --git a/httpie/core.py b/httpie/core.py index cf1f16f7..575257c1 100644 --- a/httpie/core.py +++ b/httpie/core.py @@ -1,25 +1,25 @@ import argparse +import os import platform import sys -from typing import Callable, List, Union +from typing import List, Union import requests from pygments import __version__ as pygments_version from requests import __version__ as requests_version from httpie import __version__ as httpie_version -from httpie.status import ExitStatus, http_status_to_exit_status from httpie.client import collect_messages from httpie.context import Environment from httpie.downloads import Downloader from httpie.output.writer import write_message, write_stream from httpie.plugins import plugin_manager +from httpie.status import ExitStatus, http_status_to_exit_status def main( args: List[Union[str, bytes]] = sys.argv, env=Environment(), - custom_log_error: Callable = None ) -> ExitStatus: """ The main function. @@ -30,22 +30,16 @@ def main( Return exit status code. """ - args = decode_raw_args(args, env.stdin_encoding) program_name, *args = args + env.program_name = os.path.basename(program_name) + args = decode_raw_args(args, env.stdin_encoding) plugin_manager.load_installed_plugins() - def log_error(msg, level='error'): - assert level in ['error', 'warning'] - env.stderr.write(f'\n{program_name}: {level}: {msg}\n') - from httpie.cli.definition import parser if env.config.default_options: args = env.config.default_options + args - if custom_log_error: - log_error = custom_log_error - include_debug_info = '--debug' in args include_traceback = include_debug_info or '--traceback' in args @@ -59,7 +53,6 @@ def main( try: parsed_args = parser.parse_args( args=args, - program_name=program_name, env=env, ) except KeyboardInterrupt: @@ -78,7 +71,6 @@ def main( exit_status = program( args=parsed_args, env=env, - log_error=log_error, ) except KeyboardInterrupt: env.stderr.write('\n') @@ -93,10 +85,10 @@ def main( exit_status = ExitStatus.ERROR except requests.Timeout: exit_status = ExitStatus.ERROR_TIMEOUT - log_error(f'Request timed out ({parsed_args.timeout}s).') + env.log_error(f'Request timed out ({parsed_args.timeout}s).') except requests.TooManyRedirects: exit_status = ExitStatus.ERROR_TOO_MANY_REDIRECTS - log_error( + env.log_error( f'Too many redirects' f' (--max-redirects=parsed_args.max_redirects).' ) @@ -110,7 +102,7 @@ def main( f'{msg} while doing a {request.method}' f' request to URL: {request.url}' ) - log_error(f'{type(e).__name__}: {msg}') + env.log_error(f'{type(e).__name__}: {msg}') if include_traceback: raise exit_status = ExitStatus.ERROR @@ -121,7 +113,6 @@ def main( def program( args: argparse.Namespace, env: Environment, - log_error: Callable ) -> ExitStatus: """ The main program without error handling. @@ -159,8 +150,9 @@ def program( http_status=message.status_code, follow=args.follow ) - if not env.stdout_isatty and exit_status != ExitStatus.SUCCESS: - log_error( + if (not env.stdout_isatty + and exit_status != ExitStatus.SUCCESS): + env.log_error( f'HTTP {message.raw.status} {message.raw.reason}', level='warning' ) @@ -179,10 +171,11 @@ def program( downloader.finish() if downloader.interrupted: exit_status = ExitStatus.ERROR - log_error('Incomplete download: size=%d; downloaded=%d' % ( - downloader.status.total_size, - downloader.status.downloaded - )) + env.log_error( + 'Incomplete download: size=%d; downloaded=%d' % ( + downloader.status.total_size, + downloader.status.downloaded + )) return exit_status finally: @@ -196,11 +189,11 @@ def program( def print_debug_info(env: Environment): env.stderr.writelines([ - 'HTTPie %s\n' % httpie_version, - 'Requests %s\n' % requests_version, - 'Pygments %s\n' % pygments_version, - 'Python %s\n%s\n' % (sys.version, sys.executable), - '%s %s' % (platform.system(), platform.release()), + f'HTTPie {httpie_version}\n', + f'Requests {requests_version}\n', + f'Pygments {pygments_version}\n', + f'Python {sys.version}\n{sys.executable}\n', + f'{platform.system()} {platform.release()}', ]) env.stderr.write('\n\n') env.stderr.write(repr(env)) diff --git a/httpie/sessions.py b/httpie/sessions.py index f1def677..7d75583e 100644 --- a/httpie/sessions.py +++ b/httpie/sessions.py @@ -1,4 +1,5 @@ -"""Persistent, JSON-serialized sessions. +""" +Persistent, JSON-serialized sessions. """ import os @@ -53,8 +54,7 @@ class Session(BaseConfigDict): about = 'HTTPie session file' def __init__(self, path: Union[str, Path]): - super().__init__() - self._path = Path(path) + super().__init__(path=Path(path)) self['headers'] = {} self['cookies'] = {} self['auth'] = { @@ -63,9 +63,6 @@ class Session(BaseConfigDict): 'password': None } - def _get_path(self) -> Path: - return self._path - def update_headers(self, request_headers: RequestHeadersDict): """ Update the session headers with the request ones while ignoring diff --git a/tests/test_config.py b/tests/test_config.py index 08cfe5de..45dd86de 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,6 +1,5 @@ -from httpie import __version__ -from utils import MockEnvironment, http, HTTP_OK -from httpie.context import Environment +from httpie.config import Config +from utils import HTTP_OK, MockEnvironment, http def test_default_options(httpbin): @@ -8,15 +7,33 @@ def test_default_options(httpbin): env.config['default_options'] = ['--form'] env.config.save() r = http(httpbin.url + '/post', 'foo=bar', env=env) - assert r.json['form'] == {"foo": "bar"} + assert r.json['form'] == { + "foo": "bar" + } -def test_config_dir_not_writeable(httpbin): - r = http(httpbin + '/get', env=MockEnvironment( - config_dir='/', - create_temp_config_dir=False, - )) +def test_config_file_not_valid(httpbin): + env = MockEnvironment() + env.create_temp_config_dir() + with (env.config_dir / Config.FILENAME).open('w') as f: + f.write('{invalid json}') + r = http(httpbin + '/get', env=env) assert HTTP_OK in r + assert 'http: warning' in r.stderr + assert 'invalid config file' in r.stderr + + +def test_config_file_not_inaccessible(httpbin): + env = MockEnvironment() + env.create_temp_config_dir() + config_path = env.config_dir / Config.FILENAME + assert not config_path.exists() + config_path.touch(0o000) + assert config_path.exists() + r = http(httpbin + '/get', env=env) + assert HTTP_OK in r + assert 'http: warning' in r.stderr + assert 'cannot read config file' in r.stderr def test_default_options_overwrite(httpbin): @@ -24,9 +41,6 @@ def test_default_options_overwrite(httpbin): env.config['default_options'] = ['--form'] env.config.save() r = http('--json', httpbin.url + '/post', 'foo=bar', env=env) - assert r.json['json'] == {"foo": "bar"} - - -def test_current_version(): - version = MockEnvironment().config['__meta__']['httpie'] - assert version == __version__ + assert r.json['json'] == { + "foo": "bar" + } diff --git a/tests/test_errors.py b/tests/test_errors.py index 93594a80..4f066a8a 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -4,37 +4,30 @@ from requests import Request from requests.exceptions import ConnectionError from httpie.status import ExitStatus -from httpie.core import main from utils import HTTP_OK, http -error_msg = None +@mock.patch('httpie.core.program') +def test_error(program): + exc = ConnectionError('Connection aborted') + exc.request = Request(method='GET', url='http://www.google.com') + program.side_effect = exc + r = http('www.google.com', tolerate_error_exit_status=True) + assert r.exit_status == ExitStatus.ERROR + assert ( + 'ConnectionError: ' + 'Connection aborted while doing a GET request to URL: ' + 'http://www.google.com' + ) in r.stderr @mock.patch('httpie.core.program') -def test_error(get_response): - def error(msg, *args, **kwargs): - global error_msg - error_msg = msg % args - +def test_error_traceback(program): exc = ConnectionError('Connection aborted') exc.request = Request(method='GET', url='http://www.google.com') - get_response.side_effect = exc - ret = main(['http', '--ignore-stdin', 'www.google.com'], custom_log_error=error) - assert ret == ExitStatus.ERROR - assert error_msg == ( - 'ConnectionError: ' - 'Connection aborted while doing a GET request to URL: ' - 'http://www.google.com') - - -@mock.patch('httpie.core.program') -def test_error_traceback(get_response): - exc = ConnectionError('Connection aborted') - exc.request = Request(method='GET', url='http://www.google.com') - get_response.side_effect = exc + program.side_effect = exc with raises(ConnectionError): - main(['http', '--ignore-stdin', '--traceback', 'www.google.com']) + http('--traceback', 'www.google.com') def test_max_headers_limit(httpbin_both): diff --git a/tests/utils.py b/tests/utils.py index 0bfce07e..34f61803 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -6,7 +6,7 @@ import time import json import tempfile from pathlib import Path -from typing import Optional +from typing import Optional, Union from httpie.status import ExitStatus from httpie.config import Config @@ -18,6 +18,7 @@ TESTS_ROOT = os.path.abspath(os.path.dirname(__file__)) CRLF = '\r\n' COLOR = '\x1b[' HTTP_OK = '200 OK' +# noinspection GrazieInspection HTTP_OK_COLOR = ( 'HTTP\x1b[39m\x1b[38;5;245m/\x1b[39m\x1b' '[38;5;37m1.1\x1b[39m\x1b[38;5;245m \x1b[39m\x1b[38;5;37m200' @@ -62,10 +63,13 @@ class MockEnvironment(Environment): def config(self) -> Config: if (self._create_temp_config_dir and self._temp_dir not in self.config_dir.parents): - self.config_dir = mk_config_dir() - self._delete_config_dir = True + self.create_temp_config_dir() return super().config + def create_temp_config_dir(self): + self.config_dir = mk_config_dir() + self._delete_config_dir = True + def cleanup(self): self.stdout.close() self.stderr.close() @@ -75,6 +79,7 @@ class MockEnvironment(Environment): rmtree(self.config_dir) def __del__(self): + # noinspection PyBroadException try: self.cleanup() except Exception: @@ -83,7 +88,7 @@ class MockEnvironment(Environment): class BaseCLIResponse: """ - Represents the result of simulated `$ http' invocation via `http()`. + Represents the result of simulated `$ http' invocation via `http()`. Holds and provides access to: @@ -113,8 +118,8 @@ class StrCLIResponse(str, BaseCLIResponse): @property def json(self) -> Optional[dict]: """ - Return deserialized JSON body, if one included in the output - and is parsable. + Return deserialized the request or response JSON body, + if one (and only one) included in the output and is parsable. """ if not hasattr(self, '_json'): @@ -147,7 +152,12 @@ class ExitStatusError(Exception): pass -def http(*args, program_name='http', **kwargs): +def http( + *args, + program_name='http', + tolerate_error_exit_status=False, + **kwargs, +) -> Union[StrCLIResponse, BytesCLIResponse]: # noinspection PyUnresolvedReferences """ Run HTTPie and capture stderr/out and exit status. @@ -188,7 +198,6 @@ def http(*args, program_name='http', **kwargs): True """ - tolerate_error_exit_status = kwargs.pop('tolerate_error_exit_status', False) env = kwargs.get('env') if not env: env = kwargs['env'] = MockEnvironment() @@ -200,7 +209,8 @@ def http(*args, program_name='http', **kwargs): args_with_config_defaults = args + env.config.default_options add_to_args = [] if '--debug' not in args_with_config_defaults: - if not tolerate_error_exit_status and '--traceback' not in args_with_config_defaults: + if (not tolerate_error_exit_status + and '--traceback' not in args_with_config_defaults): add_to_args.append('--traceback') if not any('--timeout' in arg for arg in args_with_config_defaults): add_to_args.append('--timeout=3') @@ -228,7 +238,8 @@ def http(*args, program_name='http', **kwargs): sys.stderr.write(stderr.read()) raise else: - if not tolerate_error_exit_status and exit_status != ExitStatus.SUCCESS: + if (not tolerate_error_exit_status + and exit_status != ExitStatus.SUCCESS): dump_stderr() raise ExitStatusError( 'httpie.core.main() unexpectedly returned'