diff --git a/CHANGELOG.md b/CHANGELOG.md index ccd5f2f..c6022d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # Changelog +## 0.1.2 + +### Fixed +- Restored compatibility with the declared `python-dateutil>=2.8.0` floor: + the date-parse error handler no longer references `parser.ParserError` + (introduced in dateutil 2.8.1). +- Non-JSON error responses (e.g. HTML gateway 502s) and non-object JSON + error payloads now raise `CurrentsAPIError` instead of leaking + `JSONDecodeError`/`AttributeError`. The exception now carries the real + HTTP status code via its `status` property; the payload's `status` value + is only used when no HTTP status is available. +- `search()` now rejects falsey invalid arguments (`keywords=0`, empty/ + whitespace keywords) instead of silently omitting them. +- Naive `datetime` inputs are rejected with a clear `ValueError` (they were + silently labeled UTC before); `date` objects are now explicitly formatted + as UTC midnight. +- `examples/source_linked_briefing` now escapes publisher-controlled + Markdown text and validates URL schemes, matching the protection the + company-monitor example already had (blocks `javascript:` link injection). + ## 0.1.1 ### Fixed diff --git a/currentsapi/__init__.py b/currentsapi/__init__.py index 0e7c8dd..abbcb87 100644 --- a/currentsapi/__init__.py +++ b/currentsapi/__init__.py @@ -1,7 +1,7 @@ import sys __project__ = "currentsapi" -__version__ = "0.1.1" +__version__ = "0.1.2" __repo__ = "https://github.com/currentslab/currentsapi-python" from .client import CurrentsAPI diff --git a/currentsapi/client.py b/currentsapi/client.py index 6f17dda..e11db31 100644 --- a/currentsapi/client.py +++ b/currentsapi/client.py @@ -9,19 +9,25 @@ class CurrentsAPIError(Exception): """Raised when the Currents API returns an error response.""" - def __init__(self, response): + def __init__(self, response, http_status=None): + if not isinstance(response, dict): + response = {} self.response = response - self._status = response.get("status") + self._http_status = http_status self._code = response.get("code") self._message = response.get("message") or response.get("msg") - super().__init__(self._message or str(response)) + super().__init__(self._message or (str(response) if response else "Unknown API error")) @property def status(self): + """HTTP status code as int; payload status used only as last resort.""" + if self._http_status is not None: + return self._http_status + payload_status = self.response.get("status") try: - return int(self._status) + return int(payload_status) except (TypeError, ValueError): - return self._status + return payload_status @property def code(self): @@ -58,17 +64,36 @@ def _get(self, endpoint, params=None): params=params or {}, ) if r.status_code != requests.codes.ok: - raise CurrentsAPIError(r.json()) - return r.json() + raise self._error_from_response(r) + try: + payload = r.json() + except ValueError: + raise CurrentsAPIError( + {"message": "Response body is not valid JSON"}, + http_status=r.status_code, + ) + return payload + + @staticmethod + def _error_from_response(r): + try: + payload = r.json() + except ValueError: + payload = {} + if not isinstance(payload, dict): + payload = { + "message": "API returned a non-object error payload", + "details": payload, + } + return CurrentsAPIError(payload, http_status=r.status_code) def latest_news(self, language=None): params = {} - if language: + if language is not None: if not isinstance(language, str): raise ValueError("language must be a string") params["language"] = language return self._get(self.latest_endpoint, params) - def search( self, language=None, @@ -80,32 +105,38 @@ def search( ): params = {} - if keywords: + if keywords is not None: if not isinstance(keywords, str): raise ValueError("keywords must be a string") + if not keywords.strip(): + raise ValueError("keywords must not be empty") params["keywords"] = keywords - if country: + if country is not None: if not isinstance(country, str): raise ValueError("country must be a string") params["country"] = country - if language: + if language is not None: if not isinstance(language, str): raise ValueError("language must be a string") params["language"] = language - if category: + if category is not None: if not isinstance(category, str): raise ValueError("category must be a string") params["category"] = category - if start_date: - date = self._normalize_date(self._parse_date(start_date, "start_date")) + if start_date is not None: + date = self._normalize_date( + self._parse_date(start_date, "start_date"), "start_date" + ) params["start_date"] = date.strftime("%Y-%m-%dT%H:%M:%SZ") - if end_date: - date = self._normalize_date(self._parse_date(end_date, "end_date")) + if end_date is not None: + date = self._normalize_date( + self._parse_date(end_date, "end_date"), "end_date" + ) params["end_date"] = date.strftime("%Y-%m-%dT%H:%M:%SZ") return self._get(self.search_endpoint, params) @@ -124,7 +155,7 @@ def _parse_date(date_value, param_name): if isinstance(date_value, str): try: return parser.parse(date_value) - except (parser.ParserError, OverflowError, ValueError) as exc: + except (OverflowError, ValueError) as exc: raise ValueError( "{} is not a parsable date: {}".format(param_name, exc) ) from exc @@ -138,7 +169,18 @@ def _parse_date(date_value, param_name): ) @staticmethod - def _normalize_date(date_value): - if isinstance(date_value, datetime.datetime) and date_value.tzinfo is not None: + def _normalize_date(date_value, param_name): + if isinstance(date_value, datetime.datetime): + if date_value.tzinfo is None: + raise ValueError( + "{} datetime must be timezone-aware; attach a tzinfo " + "(naive datetimes are ambiguous and are NOT assumed to be " + "UTC)".format(param_name) + ) return date_value.astimezone(datetime.timezone.utc) + if isinstance(date_value, datetime.date): + return datetime.datetime( + date_value.year, date_value.month, date_value.day, + tzinfo=datetime.timezone.utc, + ) return date_value diff --git a/examples/_example_utils.py b/examples/_example_utils.py new file mode 100644 index 0000000..0b67305 --- /dev/null +++ b/examples/_example_utils.py @@ -0,0 +1,32 @@ +"""Shared helpers for the shipped Currents API examples.""" + +import re + + +_MARKDOWN_ESCAPES = re.compile(r"([\\`*_\[\]()<>#!|{}])") + + +def escape_markdown_text(value): + """Escape Markdown-significant characters in publisher-controlled text.""" + return _MARKDOWN_ESCAPES.sub(r"\\\1", str(value)) + + +def safe_markdown_url(url, allowed_schemes=("http", "https")): + """Return a URL safe to embed in Markdown, or ``None`` if unsafe. + + Rejects empty values, non-string values, unknown/unsafe schemes + (e.g. ``javascript:``), and whitespace/control characters that could + break out of a Markdown link target. + """ + if not isinstance(url, str): + return None + candidate = url.strip() + if not candidate: + return None + if any(ch.isspace() or ord(ch) < 0x20 for ch in candidate): + return None + match = re.match(r"^([A-Za-z][A-Za-z0-9+.-]*):", candidate) + if not match or match.group(1).lower() not in allowed_schemes: + return None + # Escape parentheses so the URL cannot break out of [..](..) syntax. + return candidate.replace("(", "%28").replace(")", "%29") diff --git a/examples/source_linked_briefing/briefing.py b/examples/source_linked_briefing/briefing.py index acef1eb..fa6e0ce 100644 --- a/examples/source_linked_briefing/briefing.py +++ b/examples/source_linked_briefing/briefing.py @@ -4,9 +4,15 @@ import argparse import json import os +import sys from datetime import datetime, timezone from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +from _example_utils import escape_markdown_text, safe_markdown_url + +sys.path.pop(0) + def parse_args(): parser = argparse.ArgumentParser(description=__doc__) @@ -100,17 +106,17 @@ def resolve_generated_at(args, response): def build_output(response, generated_at): validate_response(response) articles = [normalize_article(article) for article in response["news"]] - articles.sort(key=lambda article: article["title"].casefold()) + articles.sort(key=lambda article: escape_markdown_text(article["title"]).casefold()) articles.sort(key=lambda article: article["published"], reverse=True) - lines = ["# Source-Linked News Briefing", "", "Generated at: {}".format(generated_at), ""] + lines = ["# Source-Linked News Briefing", "", "Generated at: {}".format(escape_markdown_text(generated_at)), ""] for article in articles: - title = article["title"] - url = article["url"] + title = escape_markdown_text(article["title"]) + url = safe_markdown_url(article["url"]) lines.append("- {} - <{}>".format(title, url) if url else "- {}".format(title)) if article["published"]: - lines.append(" - Published: {}".format(article["published"])) + lines.append(" - Published: {}".format(escape_markdown_text(article["published"]))) if article["description"]: - lines.append(" - {}".format(article["description"])) + lines.append(" - {}".format(escape_markdown_text(article["description"]))) return "\n".join(lines) + "\n", { "generated_at": generated_at, "articles": articles, diff --git a/tests/test_client.py b/tests/test_client.py index 5a480db..ba038aa 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -71,8 +71,8 @@ def test_search_all_params(self, mock_get): language="en", country="US", category="technology", - start_date="2024-01-15", - end_date="2024-06-30", + start_date="2024-01-15T00:00:00Z", + end_date="2024-06-30T00:00:00Z", ) args, kwargs = mock_get.call_args self.assertEqual( @@ -146,9 +146,10 @@ def test_api_error_raises_exception(self, mock_get): api = CurrentsAPI("key") with self.assertRaises(CurrentsAPIError) as ctx: api.latest_news() - self.assertEqual(ctx.exception.status, "error") + self.assertEqual(ctx.exception.status, 401) self.assertEqual(ctx.exception.code, "Unauthorized") self.assertEqual(ctx.exception.message, "Invalid key") + self.assertEqual(ctx.exception.response["status"], "error") def test_invalid_keywords_type(self): api = CurrentsAPI("key") @@ -209,5 +210,57 @@ def test_impossible_date_string_raises_valueerror(self): with self.assertRaises(ValueError): api.search(start_date="2026-13-45") + @patch("currentsapi.client.requests.get") + def test_non_json_502_raises_currents_api_error(self, mock_get): + mock_get.return_value = Mock(status_code=502) + mock_get.return_value.json.side_effect = ValueError("Expecting value") + api = CurrentsAPI("key") + with self.assertRaises(CurrentsAPIError) as ctx: + api.latest_news() + self.assertEqual(ctx.exception.status, 502) + + @patch("currentsapi.client.requests.get") + def test_non_object_error_payload_raises_currents_api_error(self, mock_get): + mock_get.return_value = Mock(status_code=401) + mock_get.return_value.json.return_value = [] + api = CurrentsAPI("key") + with self.assertRaises(CurrentsAPIError) as ctx: + api.latest_news() + self.assertEqual(ctx.exception.status, 401) + self.assertIn("non-object", ctx.exception.message) + + def test_falsey_keywords_rejected(self): + api = CurrentsAPI("key") + with self.assertRaises(ValueError): + api.search(keywords=0) + with self.assertRaises(ValueError): + api.search(keywords=" ") + + def test_naive_datetime_rejected(self): + api = CurrentsAPI("key") + with self.assertRaises(ValueError): + api.search(start_date=datetime.datetime(2024, 6, 1, 12, 0)) + + def test_date_object_becomes_utc_midnight(self): + mock_response = Mock(status_code=200, json=Mock(return_value={"status": "ok"})) + with patch("currentsapi.client.requests.get", return_value=mock_response) as mock_get: + api = CurrentsAPI("key") + api.search(start_date=datetime.date(2024, 6, 1)) + kwargs = mock_get.call_args.kwargs + self.assertEqual(kwargs["params"]["start_date"], "2024-06-01T00:00:00Z") + + def test_status_prefers_http_code_over_payload(self): + exc = CurrentsAPIError({"status": "error"}, http_status=401) + self.assertEqual(exc.status, 401) + exc2 = CurrentsAPIError({"status": "404"}) + self.assertEqual(exc2.status, 404) + exc3 = CurrentsAPIError({"status": "error"}) + self.assertEqual(exc3.status, "error") + + def test_exception_accepts_non_dict_payload(self): + exc = CurrentsAPIError(["unexpected"]) + self.assertIsNone(exc.code) + self.assertEqual(str(exc), "Unknown API error") + if __name__ == "__main__": unittest.main()