From f789ad6e24d55fb1ce8b242056399a096f5c7b8e Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 13:22:30 +0000 Subject: [PATCH 1/8] companion: A start on adding CAPI killswitches --- companion.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/companion.py b/companion.py index 3aa085ae..b8b0a662 100644 --- a/companion.py +++ b/companion.py @@ -28,6 +28,7 @@ from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, OrderedDic import requests import config as conf_module +import killswitch import protocol from config import config, user_agent from edmc_data import companion_category_map as category_map @@ -325,6 +326,11 @@ class Auth(object): """ logger.debug(f'Trying for "{self.cmdr}"') + should_return, new_data = killswitch.check_killswitch('capi.auth', {}) + if should_return: + logger.warning('capi.auth has been disabled via killswitch. Returning.') + return None + self.verifier = None cmdrs = config.get_list('cmdrs', default=[]) logger.debug(f'Cmdrs: {cmdrs}') @@ -393,9 +399,11 @@ class Auth(object): return None def authorize(self, payload: str) -> str: # noqa: CCR001 - """Handle oAuth authorization callback. + """ + Handle oAuth authorization callback. - :return: access token if successful, otherwise raises CredentialsError. + :return: access token if successful + :raises CredentialsError """ logger.debug('Checking oAuth authorization callback') if '?' not in payload: @@ -655,6 +663,11 @@ class Session(object): :return: True if login succeeded, False if re-authorization initiated. """ + should_return, new_data = killswitch.check_killswitch('capi.auth', {}) + if should_return: + logger.warning('capi.auth has been disabled via killswitch. Returning.') + return False + if not Auth.CLIENT_ID: logger.error('self.CLIENT_ID is None') raise CredentialsError('cannot login without a valid Client ID') @@ -946,7 +959,8 @@ class Session(object): # event too, so assume it will be polling the response queue. if query.tk_response_event is not None: logger.trace_if('capi.worker', 'Sending <>') - self.tk_master.event_generate('<>') + if self.tk_master is not None: + self.tk_master.event_generate('<>') logger.info('CAPI worker thread DONE') From 98527e7aa0d310090f47c6458a2c4a07069cc67a Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 13:24:50 +0000 Subject: [PATCH 2/8] pre-commit: Bump mypy to current latest version * The older version didn't like `check_untyped_defs = True` in .mypy.ini. * Trying `-repo: local` lead to an error about config options. * We really need a way to have this track the non-pre-commit version of mypy. --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 50d91e96..d841cda1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -46,7 +46,7 @@ repos: # mypy - static type checking # mypy --follow-imports skip - repo: https://github.com/pre-commit/mirrors-mypy - rev: 'v0.931' + rev: 'v0.991' hooks: - id: mypy # verbose: true From f1b2022aa23213945dfb64c30e4e28092269f32f Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 13:27:21 +0000 Subject: [PATCH 3/8] .mypy.ini: s/-/_/g on check-untyped-defs --- .mypy.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mypy.ini b/.mypy.ini index 38afbb67..10ef53b7 100644 --- a/.mypy.ini +++ b/.mypy.ini @@ -5,5 +5,5 @@ scripts_are_modules = True ; Without this bare `mypy ` may get warnings for e.g. ; ` = ` ; i.e. no typing info. -check-untyped-defs = True +check_untyped_defs = True ; platform = darwin From 12ee3deb59bed4f62ce9538154ec853860d94a1d Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 14:02:59 +0000 Subject: [PATCH 4/8] killswitches: Implement `--killswitches-file` CL arg * New CL arge `--killswitches-file`. This needs to reference a file either with an absolute path, or relative to the CWD of the process. * Internally if the argument is provided it is prefixed with `"file:"` in order to actually be loaded. This is because `requests` doesn't have an adapter for `file:` URLs. * Also fixes a visual bug with reporting of active killswitches. The entire SingleKill object was used instead of just its `reason` property. mypy type checks caught this. --- EDMarketConnector.py | 19 ++++++++++++++----- docs/Killswitches.md | 13 ++++++++++--- killswitch.py | 18 ++++++++++++++++-- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/EDMarketConnector.py b/EDMarketConnector.py index 21531679..470ead9c 100755 --- a/EDMarketConnector.py +++ b/EDMarketConnector.py @@ -168,6 +168,11 @@ if __name__ == '__main__': # noqa: C901 help='Have EDDN plugin show what it is tracking', action='store_true', ) + + parser.add_argument( + '--killswitches-file', + help='Specify a custom killswitches file', + ) ########################################################################### args = parser.parse_args() @@ -1859,10 +1864,13 @@ Locale LC_TIME: {locale.getlocale(locale.LC_TIME)}''' ) -def setup_killswitches(): +def setup_killswitches(filename: Optional[str]): """Download and setup the main killswitch list.""" logger.debug('fetching killswitches...') - killswitch.setup_main_list() + if filename is not None: + filename = "file:" + filename + + killswitch.setup_main_list(filename) def show_killswitch_poppup(root=None): @@ -1891,9 +1899,9 @@ def show_killswitch_poppup(root=None): for version in kills: tk.Label(frame, text=f'Version: {version.version}').grid(row=idx, sticky=tk.W) idx += 1 - for id, reason in version.kills.items(): + for id, kill in version.kills.items(): tk.Label(frame, text=id).grid(column=0, row=idx, sticky=tk.W, padx=(10, 0)) - tk.Label(frame, text=reason).grid(column=1, row=idx, sticky=tk.E, padx=(0, 10)) + tk.Label(frame, text=kill.reason).grid(column=1, row=idx, sticky=tk.E, padx=(0, 10)) idx += 1 idx += 1 @@ -2026,7 +2034,8 @@ sys.path: {sys.path}''' Translations.install(config.get_str('language')) # Can generate errors so wait til log set up - setup_killswitches() + setup_killswitches(args.killswitches_file) + root = tk.Tk(className=appname.lower()) if sys.platform != 'win32' and ((f := config.get_str('font')) is not None or f != ''): size = config.get_int('font_size', default=-1) diff --git a/docs/Killswitches.md b/docs/Killswitches.md index d90607a1..bae03c2d 100644 --- a/docs/Killswitches.md +++ b/docs/Killswitches.md @@ -119,10 +119,17 @@ JSON primitives and their python equivalents json compound types (`object -- {}` and `array -- []`) may be set. ### Testing +You can supply a custom killswitches file for testing against +EDMarketConnector: +```bash +python EDMarketConnector.py --killswitches-file +``` +This will be relative to the CWD of the process. -Killswitch files can be tested using the script in `scripts/killswitch_test.py`. -Providing a file as an argument or `-` for stdin will output the behaviour of -the provided file, including indicating typos, if applicable. +Alternatively, killswitch files can be independently tested using the script in +`scripts/killswitch_test.py`. Providing a file as an argument or `-` for stdin +will output the behaviour of the provided file, including indicating typos, if +applicable. ### Versions diff --git a/killswitch.py b/killswitch.py index a7701739..42d02844 100644 --- a/killswitch.py +++ b/killswitch.py @@ -1,6 +1,7 @@ """Fetch kill switches from EDMC Repo.""" from __future__ import annotations +import json import threading from copy import deepcopy from typing import ( @@ -339,6 +340,16 @@ def fetch_kill_switches(target=DEFAULT_KILLSWITCH_URL) -> Optional[KillSwitchJSO :return: a list of dicts containing kill switch data, or None """ logger.info("Attempting to fetch kill switches") + if target.startswith('file:'): + target = target.replace('file:', '') + try: + with open(target, 'r') as t: + return json.load(t) + + except FileNotFoundError: + logger.warning(f"No such file '{target}'") + return None + try: data = requests.get(target, timeout=10).json() @@ -458,13 +469,16 @@ def get_kill_switches_thread( active: KillSwitchSet = KillSwitchSet([]) -def setup_main_list(): +def setup_main_list(filename: Optional[str]): """ Set up the global set of kill switches for querying. Plugins should NOT call this EVER. """ - if (data := get_kill_switches(DEFAULT_KILLSWITCH_URL, OLD_KILLSWITCH_URL)) is None: + if filename is None: + filename = DEFAULT_KILLSWITCH_URL + + if (data := get_kill_switches(filename, OLD_KILLSWITCH_URL)) is None: logger.warning("Unable to fetch kill switches. Setting global set to an empty set") return From 1cc4a9d0af5323a074ccac3511cc3cc679779eb9 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 14:17:00 +0000 Subject: [PATCH 5/8] CAPI killswitches: 'capi.auth' now preventing all CAPI actions. --- EDMarketConnector.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/EDMarketConnector.py b/EDMarketConnector.py index 470ead9c..c88fd162 100755 --- a/EDMarketConnector.py +++ b/EDMarketConnector.py @@ -912,6 +912,12 @@ class AppWindow(object): def login(self): """Initiate CAPI/Frontier login and set other necessary state.""" + should_return, new_data = killswitch.check_killswitch('capi.auth', {}) + if should_return: + logger.warning('capi.auth has been disabled via killswitch. Returning.') + self.status['text'] = 'CAPI auth disabled by killswitch' + return + if not self.status['text']: # LANG: Status - Attempting to get a Frontier Auth Access Token self.status['text'] = _('Logging in...') @@ -997,6 +1003,13 @@ class AppWindow(object): :param event: Tk generated event details. """ logger.trace_if('capi.worker', 'Begin') + should_return, new_data = killswitch.check_killswitch('capi.auth', {}) + if should_return: + logger.warning('capi.auth has been disabled via killswitch. Returning.') + self.status['text'] = 'CAPI auth disabled by killswitch' + hotkeymgr.play_bad() + return + auto_update = not event play_sound = (auto_update or int(event.type) == self.EVENT_VIRTUAL) and not config.get_int('hotkey_mute') @@ -1465,7 +1478,9 @@ class AppWindow(object): auto_update = True if auto_update: - self.w.after(int(SERVER_RETRY * 1000), self.capi_request_data) + should_return, new_data = killswitch.check_killswitch('capi.auth', {}) + if not should_return: + self.w.after(int(SERVER_RETRY * 1000), self.capi_request_data) if entry['event'] == 'ShutDown': # Enable WinSparkle automatic update checks From ba68397b3f1a79e46a71ab43979efbe770a803c2 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 15:06:01 +0000 Subject: [PATCH 6/8] CAPI killswitches: endpoint killswitches & eddn export ones too * Added `capi.request.` killswitches at appropriate call points. * Added `eddn.capi_export.` killswitches. This allows for killing just the EDDN export of such CAPI-derived data, without stopping the actual queries, as other plugins/functionality might still have harmless use of the data. * PLUGINS.md: Actually describe the contents of `data` passed to plugins, and point out it might not always contain market or shipyard data. This is not only because of the new killswitches, but could already have happened if the station/port docked at didn't have the services. * Some misc typing cleanups. --- EDMarketConnector.py | 13 +++++++++---- PLUGINS.md | 17 +++++++++++++---- companion.py | 21 +++++++++++++++++++-- plugins/eddn.py | 32 +++++++++++++++++++++++++++++++- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/EDMarketConnector.py b/EDMarketConnector.py index c88fd162..f8c676df 100755 --- a/EDMarketConnector.py +++ b/EDMarketConnector.py @@ -1228,10 +1228,15 @@ class AppWindow(object): if err: play_bad = True - # Export market data - if not self.export_market_data(capi_response.capi_data): - err = 'Error: Exporting Market data' - play_bad = True + should_return, new_data = killswitch.check_killswitch('capi.request./market', {}) + if should_return: + logger.warning("capi.request./market has been disabled by killswitch. Returning.") + + else: + # Export market data + if not self.export_market_data(capi_response.capi_data): + err = 'Error: Exporting Market data' + play_bad = True self.capi_query_holdoff_time = capi_response.query_time + companion.capi_query_cooldown diff --git a/PLUGINS.md b/PLUGINS.md index dc23752d..dd71ca28 100644 --- a/PLUGINS.md +++ b/PLUGINS.md @@ -910,10 +910,9 @@ constants. ### Commander Data from Frontier CAPI If a plugin has a `cmdr_data()` function it gets called when the application -has just fetched fresh Cmdr and station data from Frontier's servers, **but not -for the Legacy galaxy**. See `cmdr_data_legacy()` below for Legacy data -handling. - +has just fetched fresh Cmdr and station data from Frontier's CAPI servers, +**but not for the Legacy galaxy**. See `cmdr_data_legacy()` below for Legacy +data handling. ```python from companion import CAPIData, SERVER_LIVE, SERVER_LEGACY, SERVER_BETA @@ -948,6 +947,16 @@ have extra properties, such as `source_host`, as shown above. Plugin authors are free to use *that* property, **but MUST NOT rely on any other extra properties present in `CAPIData`, they are for internal use only.** +The contents of `data` will always have at least the data returned by a CAPI +`/profile` query. If the player is docked at a station, and the relevant +services are available then the `lastStarport` key's value will have been +augmented with `/market` and/or `/shipyard` data. **But do not assume this +will always be the case**. + +If there is a killswitch in effect for some of the CAPI endpoints, then the +data passed to this function might not be as complete as you expect. Code +defensively. + #### CAPI data for Legacy When CAPI data has been retrieved from the separate CAPI host for the Legacy diff --git a/companion.py b/companion.py index b8b0a662..9c1ec02c 100644 --- a/companion.py +++ b/companion.py @@ -601,7 +601,7 @@ class EDMCCAPIFailedRequest(EDMCCAPIReturn): ): super().__init__(query_time=query_time, play_sound=play_sound, auto_update=auto_update) self.message: str = message # User-friendly reason for failure. - self.exception: int = exception # Exception that recipient should raise. + self.exception: BaseException = exception # Exception that recipient should raise. class Session(object): @@ -772,7 +772,12 @@ class Session(object): :param timeout: requests query timeout to use. :return: The resulting CAPI data, of type CAPIData. """ - capi_data: CAPIData + capi_data: CAPIData = CAPIData() + should_return, new_data = killswitch.check_killswitch('capi.request.' + capi_endpoint, {}) + if should_return: + logger.warning(f"capi.request.{capi_endpoint} has been disabled by killswitch. Returning.") + return capi_data + try: logger.trace_if('capi.worker', 'Sending HTTP request...') if conf_module.capi_pretend_down: @@ -854,6 +859,10 @@ class Session(object): """ station_data = capi_single_query(capi_host, self.FRONTIER_CAPI_PATH_PROFILE, timeout=timeout) + if not station_data.get('commander'): + # If even this doesn't exist, probably killswitched. + return station_data + if not station_data['commander'].get('docked') and not monitor.state['OnFoot']: return station_data @@ -894,6 +903,10 @@ class Session(object): if services.get('commodities'): market_data = capi_single_query(capi_host, self.FRONTIER_CAPI_PATH_MARKET, timeout=timeout) + if not market_data.get('id'): + # Probably killswitched + return station_data + if last_starport_id != int(market_data['id']): logger.warning(f"{last_starport_id!r} != {int(market_data['id'])!r}") raise ServerLagging() @@ -904,6 +917,10 @@ class Session(object): if services.get('outfitting') or services.get('shipyard'): shipyard_data = capi_single_query(capi_host, self.FRONTIER_CAPI_PATH_SHIPYARD, timeout=timeout) + if not shipyard_data.get('id'): + # Probably killswitched + return station_data + if last_starport_id != int(shipyard_data['id']): logger.warning(f"{last_starport_id!r} != {int(shipyard_data['id'])!r}") raise ServerLagging() diff --git a/plugins/eddn.py b/plugins/eddn.py index 2f2ccc78..afe54047 100644 --- a/plugins/eddn.py +++ b/plugins/eddn.py @@ -636,6 +636,16 @@ class EDDN: :param data: a dict containing the starport data :param is_beta: whether or not we're currently in beta mode """ + should_return, new_data = killswitch.check_killswitch('capi.request./market', {}) + if should_return: + logger.warning("capi.request./market has been disabled by killswitch. Returning.") + return + + should_return, new_data = killswitch.check_killswitch('eddn.capi_export.commodities', {}) + if should_return: + logger.warning("eddn.capi_export.commodities has been disabled by killswitch. Returning.") + return + modules, ships = self.safe_modules_and_ships(data) horizons: bool = capi_is_horizons( data['lastStarport'].get('economies', {}), @@ -757,6 +767,16 @@ class EDDN: :param data: dict containing the outfitting data :param is_beta: whether or not we're currently in beta mode """ + should_return, new_data = killswitch.check_killswitch('capi.request./shipyard', {}) + if should_return: + logger.warning("capi.request./shipyard has been disabled by killswitch. Returning.") + return + + should_return, new_data = killswitch.check_killswitch('eddn.capi_export.outfitting', {}) + if should_return: + logger.warning("eddn.capi_export.outfitting has been disabled by killswitch. Returning.") + return + modules, ships = self.safe_modules_and_ships(data) # Horizons flag - will hit at least Int_PlanetApproachSuite other than at engineer bases ("Colony"), @@ -813,6 +833,16 @@ class EDDN: :param data: dict containing the shipyard data :param is_beta: whether or not we are in beta mode """ + should_return, new_data = killswitch.check_killswitch('capi.request./shipyard', {}) + if should_return: + logger.warning("capi.request./shipyard has been disabled by killswitch. Returning.") + return + + should_return, new_data = killswitch.check_killswitch('eddn.capi_export.shipyard', {}) + if should_return: + logger.warning("eddn.capi_export.shipyard has been disabled by killswitch. Returning.") + return + modules, ships = self.safe_modules_and_ships(data) horizons: bool = capi_is_horizons( @@ -1857,7 +1887,7 @@ class EDDN: match = self.CANONICALISE_RE.match(item) return match and match.group(1) or item - def capi_gameversion_from_host_endpoint(self, capi_host: str, capi_endpoint: str) -> str: + def capi_gameversion_from_host_endpoint(self, capi_host: Optional[str], capi_endpoint: str) -> str: """ Return the correct CAPI gameversion string for the given host/endpoint. From 729016cf38261f6b8df3df160560d9e332715291 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 15:35:59 +0000 Subject: [PATCH 7/8] companion.py: Fix exception type & initialise a requests_session * Use `Exception` not `BaseException` for `EDMCCAPIFailedRequest.exception`. * Initialise Auth.request_session in its `__init__()`. This *should* fix #1727 - as there'll then be no way for a `Session` to have methods invoked without this set up. --- companion.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/companion.py b/companion.py index 9c1ec02c..cab1640f 100644 --- a/companion.py +++ b/companion.py @@ -601,7 +601,7 @@ class EDMCCAPIFailedRequest(EDMCCAPIReturn): ): super().__init__(query_time=query_time, play_sound=play_sound, auto_update=auto_update) self.message: str = message # User-friendly reason for failure. - self.exception: BaseException = exception # Exception that recipient should raise. + self.exception: Exception = exception # Exception that recipient should raise. class Session(object): @@ -619,7 +619,7 @@ class Session(object): def __init__(self) -> None: self.state = Session.STATE_INIT self.credentials: Optional[Dict[str, Any]] = None - self.requests_session: Optional[requests.Session] = None + self.requests_session: Optional[requests.Session] = requests.Session() self.auth: Optional[Auth] = None self.retrying = False # Avoid infinite loop when successful auth / unsuccessful query self.tk_master: Optional[tk.Tk] = None From 5070c3686f6c18ee3f39a6519233bf416cecd1bf Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 16 Dec 2022 15:40:48 +0000 Subject: [PATCH 8/8] companion.Session: Don't double-initaliase requests_session I forgot to remove the `requests.Session()` call in `start_frontier_auth()`. And upon removing that we need a `None` trap to make mypy happy. --- companion.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/companion.py b/companion.py index cab1640f..1c68e38b 100644 --- a/companion.py +++ b/companion.py @@ -652,9 +652,10 @@ class Session(object): def start_frontier_auth(self, access_token: str) -> None: """Start an oAuth2 session.""" logger.debug('Starting session') - self.requests_session = requests.Session() - self.requests_session.headers['Authorization'] = f'Bearer {access_token}' - self.requests_session.headers['User-Agent'] = user_agent + if self.requests_session is not None: + self.requests_session.headers['Authorization'] = f'Bearer {access_token}' + self.requests_session.headers['User-Agent'] = user_agent + self.state = Session.STATE_OK def login(self, cmdr: Optional[str] = None, is_beta: Optional[bool] = None) -> bool: