fix: address review findings on shlink shortener and template parser

Security: a shlink deployment with short_url_website unset POSTed the operator's
API key to v.gd. _normalize_base falls back to the public default and the shlink
branch guarded only on a missing key, never on a missing base. shlink now requires
an explicit base and refuses a v.gd/is.gd host outright rather than sending an
X-Api-Key there.

Correctness: _shorten_url_with_gd lost its response.ok guard when the backends were
split out, so a 502 whose body starts with http was returned as the short URL and
transmitted over RF. Restored, with a warning. The regression test that should have
caught this passed only because it left mock_resp.text as a MagicMock; it now uses
a realistic proxy maintenance body.

_shorten_url_with_shlink never checked the status either. Shlink reports failures as
RFC 7807 problem details, which parse as JSON and simply lack shortUrl, so a bad API
key was indistinguishable from an unshortenable URL at DEBUG. It now warns on a
non-OK status, a non-JSON body, and a missing shortUrl. Dropped the shortUrlSlug
fallback: that is a request field, not a response field, and returning it puts a
bare slug where a link belongs.

Timeouts and connection errors are back on their own DEBUG handler. They had fallen
through to the broad handler, whose level rose to ERROR in the same diff, so a
routine intermittent uplink logged "Unexpected error shortening URL" on every reply.

Parser: _GREEDY_ARG_FILTERS was tested before the quoted-argument branch, so
prefix_if_nonempty, the one filter already in shipped configs, could not use the
quoted syntax this PR adds. {path_distance|prefix_if_nonempty:"Dist {sender}: "}
rendered raw template text. A quote immediately after the ':' now selects the quoted
grammar; anything else stays greedy, so config.ini.example's
`prefix_if_nonempty: | Path Dist: ` keeps its pipe and its whitespace.

Blocking HTTP on the event loop: the path command rendered its reply prefix inline
from async code, so a slow shortener stalled radio RX, MQTT and every other handler
for the full 5s timeout. Added format_piped_template_async and switched the path
command to it. The test command still renders synchronously through the sync
check_keywords dispatcher; making that async is a separate change, so the filter now
warns once when it runs on the loop.

Naming: one operation should not have two names in an operator-facing DSL. shorten
and shorten_url are aliases in both response templates and feed formats, and
if_nonempty is canonical with if_notempty as an alias, so a filter chain copied
between a feed format and a response_format works either way.

Also: reduced _build_create_shlink_url to the one parameter it uses and dropped its
dead query/startswith lines, renamed the shlink POST callable from `get`, documented
the config argument on format_piped_template, stopped gating the render trace on an
unrelated parameter and logging field values (sender IDs, user phrases) with it, and
moved the changelog entry from Fixed to Added and Changed.

CHANGELOG.md

@@ -8,11 +8,6 @@ semantic versioning.
 
 ### Fixed
 
-- Refactor response_template parsing to use finite state machine when processing
-  templates adding additional flexibility such as the use of nested fields. Added
-  support for shlink to External_Data configuration. Added url_shortener filter and
-  if_notempty filter to response_template.
-
 - `path` no longer answers "No path information available in current message" on a
   busy mesh (#255). Verifying a channel message against the RF cache only ever
   checked the newest row, which assumes the RF log row and the decoded CHAN event
@@ -159,6 +154,11 @@ semantic versioning.
 
 ### Changed
 
+- Response templates are parsed by a character-by-character state machine rather
+  than by splitting on delimiters. Placeholders can now nest (`{"Dist: {d|hops_min:1}"}`)
+  and filter arguments can be quoted. Field values are substituted into the output
+  and never re-scanned, so a sender-supplied phrase still cannot inject a placeholder.
+
 - Web viewer navigation is grouped: Radio, Scheduled Messages, Greeter, Feeds, Plugins
   and Configuration now sit under a single **Settings** gear menu, leaving Dashboard,
   Real-time, Contacts, Mesh Graph and Logs on the bar. The current page is highlighted,
@@ -166,6 +166,28 @@ semantic versioning.
 
 ### Added
 
+- Shlink is now supported as a URL shortener alongside v.gd / is.gd, selected with
+  `short_url_website_service = shlink` under `[External_Data]`. It authenticates with
+  `short_url_website_api_key` in an `X-Api-Key` header and needs `short_url_website`
+  set to your own instance — there is no default, and the bot skips shortening rather
+  than sending the key to a host you did not configure.
+
+- `shorten` and `if_nonempty` response-template filters. `shorten` runs a value
+  through the configured shortener and falls back to the original URL when shortening
+  fails, so a clause is never lost to a network error. `if_nonempty:L` replaces a
+  non-empty value with literal `L` and clears otherwise, which is how a whole clause
+  is hidden rather than labelled: `{packet_hash|if_nonempty:"https://…/{packet_hash}"|shorten}`
+  prints nothing at all when RF correlation fails, instead of a broken link. Both
+  filters also answer to their other spellings — `shorten_url` in a template,
+  `shorten_url` in a feed format, `if_notempty` — so a chain copied between a feed
+  format and a command `response_format` works unchanged either way.
+
+- Response-template filter arguments may be double-quoted, and a quoted argument may
+  contain nested `{field}` placeholders: `{d|prefix_if_nonempty:"Dist {sender}: "}`.
+  The quote ends the argument, so further filters can follow it. An unquoted
+  `prefix_if_nonempty` argument still consumes the rest of the placeholder, which is
+  what lets its literal contain `|`, so that form must stay last in its chain.
+
 - `mqttN_keepalive` (default 60) sets the MQTT PINGREQ interval per broker. It was
   hardcoded at 60 before, which is long for websockets through a proxy that drops
   idle connections.

config.ini.example

@@ -644,12 +644,15 @@ log_max_bytes = 5242880
 # log_backup_count: number of rotated backup files to keep (e.g. meshcore_bot.log.1 … .3)
 log_backup_count = 3
 [External_Data]
-# URL shortener Service (gd,shlink). Default: gd
+# URL shortener service: gd (v.gd / is.gd-compatible) or shlink. Default: gd
 short_url_website_service = gd
-# URL shortener API base (v.gd / is.gd-compatible create.php). Default: https://v.gd
-# See https://v.gd/apishorteningreference.php
+# URL shortener API base. For gd this is the create.php host (default https://v.gd,
+# see https://v.gd/apishorteningreference.php). For shlink it is your own instance's
+# base URL and is REQUIRED -- there is no default, and the bot refuses to shorten
+# rather than send your API key to a host you did not configure.
 short_url_website = https://v.gd
-# Optional API key for alternate shortener hosts (unused for public v.gd/is.gd)
+# API key. Optional for gd (unused for public v.gd/is.gd, appended as ?key= for
+# alternate hosts). Required for shlink, where it is sent as an X-Api-Key header.
 short_url_website_api_key = 
 
 # Weather API key (future feature)
@@ -1045,6 +1048,8 @@ enable_p_shortcut = true
 # Only the first RF chunk includes the prefix when the reply is split.
 # Example: @[{sender}]
 # Example: {path_distance|prefix_if_nonempty:\U0001F4CF }
+# Filter arguments may be double-quoted to end them explicitly and allow further
+# filters afterwards: {path_distance|prefix_if_nonempty:"\U0001F4CF "|hops_min:1}
 reply_prefix =
 
 # Bytes per hop required before resolving repeater names from the database.
@@ -1559,7 +1564,9 @@ require_path_bytes_failure_response =
 # {firstlast_distance} requires at least two path nodes with stored repeater/roomserver coordinates.
 # Pipe filters (feed-style), e.g. only show cumulative path distance when hops are multibyte (2+ bytes per hop):
 # response_format = ack @[{sender}]{phrase_part} | {connection_info}{path_distance|pathbytes_min:2|prefix_if_nonempty: | Path Dist: }| F/L Dist: {firstlast_distance} | {elapsed} | Rec: {timestamp}
-# Filters: pathbytes_min:N (alias pathbytes:N) clears the field unless bytes_per_hop >= N; hops_min:N clears it unless the message travelled at least N hops; prefix_if_nonempty:L prepends literal L when value non-empty after prior filters. If L contains |, put prefix_if_nonempty last in that placeholder (it consumes the rest of the chain as its literal).
+# Filters: pathbytes_min:N (alias pathbytes:N) clears the field unless bytes_per_hop >= N; hops_min:N clears it unless the message travelled at least N hops; prefix_if_nonempty:L prepends literal L when value non-empty after prior filters; if_nonempty:L (alias if_notempty:L) replaces the value with literal L when non-empty; shorten (alias shorten_url) runs the value through the [External_Data] shortener, falling back to the original on failure.
+# A filter argument may be double-quoted, which ends it at the closing quote so more filters can follow, and may contain nested {field} placeholders: prefix_if_nonempty:"Dist {sender}: "|hops_min:1
+# An UNQUOTED prefix_if_nonempty:L consumes the rest of the chain as its literal, so L may contain | but the filter must come last in that placeholder.
 # hops_min asks about the route, pathbytes_min about how it is encoded. Use hops_min:1 to drop a
 # distance clause on a direct message without also dropping a measurable one-byte multi-hop path.
 # The distance placeholders render "N/A" on a direct message, which is non-empty, so

docs/FEEDS.md

@@ -67,7 +67,7 @@ shorten_urls = false
 # Or shorten only where the format says {link|shorten} (see placeholders below)
 ```
 
-Per-output-format URL shortening: use `{link|shorten}` for a single shortened link, or `{link|shorten|truncate:N}` to shorten then cap length. `shorten_urls = true` shortens every plain `{link}`.
+Per-output-format URL shortening: use `{link|shorten}` for a single shortened link, or `{link|shorten|truncate:N}` to shorten then cap length. `shorten_urls = true` shortens every plain `{link}`. `shorten_url` is accepted as an alias, so a filter chain copied from a command `response_format` works here unchanged.
 
 ## RSS Feed Configuration
 

docs/path-command-config.md

@@ -27,17 +27,18 @@ These options only affect the **path** command’s reply text and whether repeat
 reply_prefix = "{path_distance|prefix_if_nonempty:📏 }\n"
 ```
 - `hops_min:N` clears a field unless the message actually travelled at least N hops. `{path_distance}` renders `N/A` on a direct message, which `prefix_if_nonempty` treats as a value, so gate it first: `{path_distance|hops_min:1|prefix_if_nonempty:📏 }`. Unlike `pathbytes_min:N`, which asks how the path is *encoded*, this keeps a measurable one-byte multi-hop path.
-- `if_notempty:LITERAL` renders `LITERAL` when the value is non-empty after prior filters, and clears entirely otherwise — the opposite pairing of `prefix_if_nonempty`, useful when the whole output should be a fixed (or field-built) literal rather than the value with a label prepended. Since `{packet_hash}` is empty whenever RF correlation fails, gating on it hides the whole clause instead of printing a broken link:
+- `if_nonempty:LITERAL` (alias `if_notempty`) renders `LITERAL` when the value is non-empty after prior filters, and clears entirely otherwise — the opposite pairing of `prefix_if_nonempty`, useful when the whole output should be a fixed (or field-built) literal rather than the value with a label prepended. Since `{packet_hash}` is empty whenever RF correlation fails, gating on it hides the whole clause instead of printing a broken link:
 
 ```ini
-reply_prefix = {packet_hash|if_notempty:"https://scope.example.net/#/packets/{packet_hash}"}
+reply_prefix = {packet_hash|if_nonempty:"https://scope.example.net/#/packets/{packet_hash}"}
 ```
-  The `LITERAL` argument may itself be a double-quoted string containing nested `{field}` placeholders (expanded before the filter runs), so the link above still carries the packet hash even though the field being gated on (`packet_hash`) and the field inside the literal are the same one.
-- `shorten_url` runs the value through the shared shortener configured under `[External_Data]` (`short_url_website`, `short_url_website_service` — `gd` for v.gd/is.gd-compatible or `shlink`, and `short_url_website_api_key` where required). It falls back to the original, unshortened value if shortening fails or isn't configured, so a clause never silently disappears because of a network error. Chain it after building the link so only the final URL is sent over RF:
+  The `LITERAL` argument may itself be a double-quoted string containing nested `{field}` placeholders (expanded before the filter runs), so the link above still carries the packet hash even though the field being gated on (`packet_hash`) and the field inside the literal are the same one. Quoting also ends the argument at the closing quote, so more filters can follow it — including after `prefix_if_nonempty`, whose *unquoted* argument still swallows the rest of the chain so that a literal may contain `|`.
+- `shorten` (alias `shorten_url`, the same filter feed formats document) runs the value through the shared shortener configured under `[External_Data]` (`short_url_website`, `short_url_website_service` — `gd` for v.gd/is.gd-compatible or `shlink`, and `short_url_website_api_key` where required). It falls back to the original, unshortened value if shortening fails or isn't configured, so a clause never silently disappears because of a network error. Chain it after building the link so only the final URL is sent over RF:
 
 ```ini
-reply_prefix = {packet_hash|if_notempty:"https://scope.example.net/#/packets/{packet_hash}"|shorten_url}
+reply_prefix = {packet_hash|if_nonempty:"https://scope.example.net/#/packets/{packet_hash}"|shorten}
 ```
+  For `shlink`, `short_url_website` is required — with no base configured the bot skips shortening rather than sending your API key to the default `gd` host.
 
 **`minimum_path_bytes`** (integer `0`–`3`, default `0`)
 

modules/commands/path_command.py

@@ -15,7 +15,7 @@ from ..path_inference import (
     select_node_repeater,
     select_repeater_by_graph,
 )
-from ..response_template import format_piped_template
+from ..response_template import format_piped_template_async
 from ..utils import (
     bytes_per_hop_from_routing_and_nodes,
     calculate_distance,
@@ -415,19 +415,24 @@ class PathCommand(BaseCommand):
             return ''
         return f"{distance:.1f}km"
 
-    def _format_path_reply_prefix(self, message: MeshMessage) -> str:
+    async def _format_path_reply_prefix(self, message: MeshMessage) -> str:
+        """Render the configured reply prefix off the event loop.
+
+        Async because the prefix may carry a ``shorten`` filter, whose HTTP call
+        would otherwise block radio RX and every other handler for its full timeout.
+        """
         if not self.path_reply_prefix:
             return ''
         fields = self.get_standard_placeholder_fields(message)
         fields['path_distance'] = self._format_path_distance(message)
-        formatted = format_piped_template(
+        formatted = (await format_piped_template_async(
             self.path_reply_prefix,
             {k: str(v) for k, v in fields.items()},
             message=message,
             logger=self.logger,
             config=self.bot.config,
             prefix_hex_chars=getattr(self.bot, 'prefix_hex_chars', 2),
-        ).rstrip()
+        )).rstrip()
         if not formatted:
             return ''
         return formatted + '\n'
@@ -1057,7 +1062,7 @@ class PathCommand(BaseCommand):
 
     async def _send_path_response(self, message: MeshMessage, response: str):
         """Send path response, splitting into multiple messages if necessary"""
-        prefix = self._format_path_reply_prefix(message)
+        prefix = await self._format_path_reply_prefix(message)
         self.last_response = prefix + response if prefix else response
 
         max_length = self.get_max_message_length(message)

modules/feed_format.py

@@ -107,6 +107,20 @@ def clean_feed_html_body(body: str) -> str:
     return body.strip()
 
 
+def _canonical_shorten_name(function: str) -> str:
+    """Accept the response_template spelling ``shorten_url`` for ``shorten``.
+
+    Feed formats and response templates are one operator-facing DSL as far as anyone
+    configuring the bot is concerned; the same operation answering to a different
+    name in each is a standing source of config mistakes.
+    """
+    if function == "shorten_url":
+        return "shorten"
+    if function.startswith("shorten_url|"):
+        return "shorten|" + function.split("|", 1)[1]
+    return function
+
+
 def apply_feed_field_function(
     text: str,
     function: str,
@@ -117,7 +131,7 @@ def apply_feed_field_function(
     """Apply a shortening, parsing, or conditional function to text.
 
     Supported functions:
-    - shorten - URL-shorten via [External_Data] short_url_website (v.gd / is.gd API)
+    - shorten (alias shorten_url) - URL-shorten via [External_Data] short_url_website
     - shorten|truncate:N (etc.) - shorten first, then apply the rest
     - truncate:N / truncate_hard:N / substr:N[,M] / word_wrap:N / first_words:N
     - regex:… / if_regex:… / switch:… / regex_cond:…
@@ -125,6 +139,7 @@ def apply_feed_field_function(
     if not function or not str(function).strip():
         return text or ""
     function = str(function).strip()
+    function = _canonical_shorten_name(function)
 
     def _debug(msg: str) -> None:
         if logger is not None:
@@ -441,7 +456,7 @@ def format_feed_message(
 
             if field_name == "link":
                 value = link_original
-                fn = function
+                fn = _canonical_shorten_name(function)
                 if shorten_feed_urls and fn != "shorten" and not fn.startswith("shorten|"):
                     s = shorten_url_sync(
                         link_original,

modules/response_template.py

@@ -11,6 +11,8 @@ chain, evaluated left to right.
 
 from __future__ import annotations
 
+import asyncio
+from functools import partial
 from typing import Any, Callable
 
 from .url_shortener import shorten_url_sync
@@ -70,19 +72,52 @@ def _filter_prefix_if_nonempty(value: str, ctx: dict[str, Any], args: str) -> st
         return ''
     return args + value
 
+
+# The event-loop warning below is worth saying once, not once per reply.
+_warned_blocking_render = False
+
+
+def _on_event_loop() -> bool:
+    """True when called on a thread with a running asyncio loop.
+
+    Rendering in the default executor (see :func:`format_piped_template_async`) puts
+    the work on a worker thread, where this is False. It is True only when a blocking
+    filter is about to stall the bot, which is worth saying out loud.
+    """
+    try:
+        asyncio.get_running_loop()
+    except RuntimeError:
+        return False
+    return True
+
+
 def _filter_shorten_url(value: str, ctx: dict[str, Any], args: str) -> str:
-    """Shorten *value* URL using configured URL shortener (v.gd / is.gd compatible)."""
+    """Shorten *value* through the configured shortener (v.gd / is.gd, or Shlink).
+
+    Falls back to *value* unchanged whenever shortening is unavailable or fails, so a
+    misconfigured shortener costs a longer message rather than a broken one. Needs
+    ``config`` on the render call; without it the value passes through untouched.
+
+    Blocking: this issues an HTTP request. Async callers must reach it through
+    :func:`format_piped_template_async`, not :func:`format_piped_template`.
+    """
     logger = ctx.get('logger')
     config = ctx.get('config')
-    if logger is not None:
-        logger.debug("Shortening URL %r", value)
     if config is None or value == '':
         if logger is not None:
-            logger.debug("Abandoning shorten url due to empty value or config")
+            logger.debug("Not shortening: no config on the render call, or empty value")
         return value
+    global _warned_blocking_render
+    if logger is not None and not _warned_blocking_render and _on_event_loop():
+        _warned_blocking_render = True
+        logger.warning(
+            "Rendering a shorten filter on the event loop; the bot will stall for up "
+            "to the shortener timeout on each reply. Render via "
+            "format_piped_template_async. This is logged once."
+        )
     return shorten_url_sync(value, config=config, logger=logger) or value
 
-def _filter_if_notempty(value: str, ctx: dict[str, Any], args: str) -> str:
+def _filter_if_nonempty(value: str, ctx: dict[str, Any], args: str) -> str:
     """Return *args* literal only when *value* is non-empty after prior filters."""
     if not value:
         return ''
@@ -93,14 +128,22 @@ RESPONSE_TEMPLATE_FILTERS: dict[str, FilterFn] = {
     'pathbytes': _filter_pathbytes_min,
     'hops_min': _filter_hops_min,
     'prefix_if_nonempty': _filter_prefix_if_nonempty,
-    'if_notempty': _filter_if_notempty,
+    'if_nonempty': _filter_if_nonempty,
+    # `if_notempty` and `shorten_url` are aliases. One operation should not have two
+    # names in an operator-facing DSL, but feed formats already document `shorten`
+    # (docs/FEEDS.md) and this module already ships `prefix_if_nonempty`, so both
+    # spellings resolve rather than silently passing the value through.
+    'if_notempty': _filter_if_nonempty,
+    'shorten': _filter_shorten_url,
     'shorten_url': _filter_shorten_url,
 }
 
-# prefix_if_nonempty's literal argument may itself contain '|', so once the parser
-# sees this filter name it stops splitting on '|' and takes everything up to the
-# placeholder's closing '}' as one literal argument. It must therefore be last in
-# a chain whenever its literal needs a pipe.
+# prefix_if_nonempty's literal argument may itself contain '|', and shipped configs
+# rely on that (config.ini.example: `prefix_if_nonempty: | Path Dist: `), so for an
+# *unquoted* argument the parser stops splitting on '|' and takes everything up to
+# the placeholder's closing '}' as one literal. Such a filter must be last in its
+# chain. Quoting the argument (`prefix_if_nonempty:"L | "`) is the way to keep
+# filtering afterwards; a quote immediately after the ':' selects that branch.
 _GREEDY_ARG_FILTERS = frozenset({'prefix_if_nonempty'})
 
 
@@ -226,6 +269,14 @@ class _TemplateParser:
             return name, '', i
         i += 1  # consume ':'
         if name in _GREEDY_ARG_FILTERS:
+            # A quote immediately after ':' opts into the quoted-argument grammar.
+            # Anything else stays greedy, so existing unquoted literals keep their
+            # pipes and their leading/trailing whitespace exactly as written.
+            if i < self.n and self.s[i] == '"':
+                value, j = self._parse_quoted_string(i)
+                if value is None:
+                    return None, '', j
+                return name, value, j
             close = self.s.find('}', i)
             if close == -1:
                 return None, '', self.n
@@ -268,10 +319,15 @@ def format_piped_template(
             lets ``prefix_if_nonempty`` drop its literal label too.
         message: Triggering mesh message; required for ``pathbytes`` / ``pathbytes_min`` filters.
         logger: Optional logger for unknown filter warnings.
+        config: Bot config, required by ``shorten`` / ``shorten_url``. Without it those
+            filters pass the long URL through unchanged.
         prefix_hex_chars: Bot prefix width for inferring bytes per hop from legacy path text.
 
     Returns:
         Fully expanded string.
+
+    Blocking: ``shorten`` issues an HTTP request. Call
+    :func:`format_piped_template_async` from async code so the event loop keeps running.
     """
     ctx: dict[str, Any] = {
         'message': message,
@@ -279,6 +335,39 @@ def format_piped_template(
         'prefix_hex_chars': prefix_hex_chars,
         'config': config,
     }
-    if (logger is not None) and (config is not None):
-        logger.debug("Rendering response template %r with fields %r", template, fields)
+    if logger is not None:
+        # Field values carry sender IDs and user-supplied phrases; log the template
+        # being rendered and the field names, not the values.
+        logger.debug(
+            "Rendering response template %r with fields %s", template, sorted(fields)
+        )
     return _TemplateParser(template, fields, ctx, logger).render()
+
+
+async def format_piped_template_async(
+    template: str,
+    fields: dict[str, Any],
+    *,
+    message: Any = None,
+    logger: Any = None,
+    config: Any = None,
+    prefix_hex_chars: int = 2,
+) -> str:
+    """Async wrapper for :func:`format_piped_template`, run in the default executor.
+
+    The ``shorten`` filter makes a blocking HTTP call with a 5s timeout. Rendering a
+    template inline on the event loop stalls radio RX, other command handlers, MQTT
+    and heartbeats for that whole timeout, and the reply misses its RF window.
+    """
+    return await asyncio.get_running_loop().run_in_executor(
+        None,
+        partial(
+            format_piped_template,
+            template,
+            fields,
+            message=message,
+            logger=logger,
+            config=config,
+            prefix_hex_chars=prefix_hex_chars,
+        ),
+    )

modules/url_shortener.py

@@ -2,8 +2,11 @@
 """
 Shared URL shortening for MeshCore Bot and web viewer.
 
-Uses the v.gd / is.gd-compatible API (GET .../create.php?format=simple&url=...).
-Configure base URL and optional API key under [External_Data] in config.ini.
+Two backends are supported, selected by ``short_url_website_service``:
+``gd`` (default) uses the v.gd / is.gd-compatible API
+(GET .../create.php?format=simple&url=...), and ``shlink`` POSTs to a self-hosted
+Shlink instance's /rest/v3/short-urls with an ``X-Api-Key`` header. Configure the
+base URL, service, and optional API key under [External_Data] in config.ini.
 """
 
 from __future__ import annotations
@@ -66,10 +69,25 @@ def _normalize_base(base: str) -> str:
     return b if b else DEFAULT_SHORT_URL_BASE
 
 
+def _is_vgd_compat_host(host: str) -> bool:
+    """True for the public v.gd / is.gd hosts, which take no API key."""
+    return (host or "").lower().split(":")[0] in _VGD_COMPAT_HOSTS
+
+
 def _host_allows_key_in_query(host: str) -> bool:
     """True if we may append api_key for this host. v.gd/is.gd public API: False."""
-    h = (host or "").lower().split(":")[0]
-    return h not in _VGD_COMPAT_HOSTS
+    return not _is_vgd_compat_host(host)
+
+
+def _base_host(base: str) -> str:
+    """Hostname of *base*, tolerating a scheme-less value like ``example.com/x``."""
+    from urllib.parse import urlparse
+
+    root = (base or "").strip()
+    if "://" not in root:
+        root = f"https://{root}"
+    parsed = urlparse(root)
+    return (parsed.hostname or "").lower()
 
 
 def _parse_simple_response(body: str) -> str | None:
@@ -104,7 +122,12 @@ def _build_create_gd_url(long_url: str, base: str, api_key: str) -> str:
     return rebuilt
 
 
-def _build_create_shlink_url(long_url: str, base: str, api_key: str) -> str:
+def _build_create_shlink_url(base: str) -> str:
+    """Build the Shlink create endpoint from *base*.
+
+    Only the base is needed: the long URL travels in the POST body and the API key
+    in an ``X-Api-Key`` header, never in the URL.
+    """
     from urllib.parse import urlparse, urlunparse
 
     root = _normalize_base(base)
@@ -115,11 +138,7 @@ def _build_create_shlink_url(long_url: str, base: str, api_key: str) -> str:
     if not netloc and parsed.path:
         netloc = parsed.path.split("/")[0]
     path = (parsed.path or "").rstrip("/") + "/rest/v3/short-urls"
-    if not path.startswith("/"):
-        path = "/" + path
-    query = ""
-    rebuilt = urlunparse((parsed.scheme or "https", netloc, path, "", query, ""))
-    return rebuilt
+    return urlunparse((parsed.scheme or "https", netloc, path, "", "", ""))
 
 
 def _shorten_url_with_shlink(
@@ -133,7 +152,7 @@ def _shorten_url_with_shlink(
     """Shorten a URL using Shlink API."""
     import json
 
-    shortener_url = _build_create_shlink_url(long_url, base, api_key)
+    shortener_url = _build_create_shlink_url(base)
     headers = {
         "Content-Type": "application/json",
         "X-Api-Key": api_key,
@@ -142,16 +161,36 @@ def _shorten_url_with_shlink(
         {"longUrl": long_url, "findIfExists": True, "tags": ["meshcore-bot"]}
     )
 
-    get = session.post if session is not None else requests.post
-    response = get(shortener_url, headers=headers, data=payload, timeout=timeout)
+    post = session.post if session is not None else requests.post
+    response = post(shortener_url, headers=headers, data=payload, timeout=timeout)
     if logger:
         logger.debug("Shlink response: %s", response.text)
-    data = response.json()
-    short_url = data.get("shortUrl") or data.get("shortUrlSlug")
 
+    # Shlink reports failures as RFC 7807 problem details, which parse as JSON just
+    # fine and simply lack shortUrl. Without this check a bad API key looks
+    # identical to a URL that could not be shortened, at DEBUG only.
+    if not response.ok:
+        if logger:
+            logger.warning(
+                "Shlink shortener returned HTTP %s: %s",
+                getattr(response, "status_code", "?"),
+                (response.text or "")[:200],
+            )
+        return ""
+
+    try:
+        data = response.json()
+    except ValueError:
+        if logger:
+            logger.warning("Shlink shortener returned a non-JSON body; not shortening.")
+        return ""
+
+    short_url = (data or {}).get("shortUrl")
     if short_url:
-        return short_url
+        return str(short_url)
 
+    if logger:
+        logger.warning("Shlink response carried no shortUrl; not shortening.")
     return ""
 
 
@@ -169,6 +208,15 @@ def _shorten_url_with_gd(
     get = session.get if session is not None else requests.get
 
     response = get(shortener_url, timeout=timeout)
+    # A failing proxy or maintenance page can return a body that looks like a URL.
+    # Without this check that body is returned as the short link and transmitted.
+    if not response.ok:
+        if logger:
+            logger.warning(
+                "URL shortener returned HTTP %s", getattr(response, "status_code", "?")
+            )
+        return ""
+
     short = _parse_simple_response(response.text)
     if short:
         return short
@@ -193,7 +241,7 @@ def shorten_url_sync(
         if not url_str:
             return ""
 
-        base = _safe_config_get(config, "External_Data", "short_url_website", "")
+        raw_base = _safe_config_get(config, "External_Data", "short_url_website", "")
         service = (
             _safe_config_get(config, "External_Data", "short_url_website_service", "gd")
             .strip()
@@ -203,15 +251,32 @@ def shorten_url_sync(
             _safe_config_get(config, "External_Data", "short_url_website_api_key", "")
             or ""
         ).strip()
-        base = _normalize_base(base)
 
         if service == "shlink":
+            # Deliberately not _normalize_base: its v.gd fallback would POST the
+            # operator's API key to an unrelated third party when the base is unset.
+            base = (raw_base or "").strip().rstrip("/")
+            if not base:
+                if logger:
+                    logger.warning(
+                        "short_url_website_service=shlink requires short_url_website "
+                        "(there is no default Shlink instance); skipping."
+                    )
+                return ""
             if not api_key:
                 if logger:
                     logger.warning(
                         "short_url_website_service=shlink requires short_url_website_api_key; skipping."
                     )
                 return ""
+            if _is_vgd_compat_host(_base_host(base)):
+                if logger:
+                    logger.warning(
+                        "short_url_website_service=shlink points at the public %s API, "
+                        "which is not Shlink; skipping rather than sending the API key there.",
+                        _base_host(base),
+                    )
+                return ""
             return _shorten_url_with_shlink(
                 url_str,
                 base,
@@ -225,13 +290,19 @@ def shorten_url_sync(
         # only appended for self-hosted alternates via _host_allows_key_in_query).
         return _shorten_url_with_gd(
             url_str,
-            base,
+            _normalize_base(raw_base),
             api_key,
             session=session,
             timeout=timeout,
             logger=logger,
         )
 
+    except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as e:
+        # Routine on a mesh node with an intermittent uplink. Logging these at ERROR
+        # as "unexpected" floods the log and buries the errors that do need triage.
+        if logger:
+            logger.debug("URL shortener unreachable: %s", e)
+        return ""
     except Exception as e:
         if logger:
             logger.error("Unexpected error shortening URL: %s", e)

tests/test_url_shortener.py

@@ -43,21 +43,20 @@ class TestBuildCreateGdUrl:
 
 class TestBuildCreateShlinkUrl:
     def test_appends_rest_v3_short_urls_path(self):
-        u = _build_create_shlink_url("http://a.com", "https://short.example", "k1")
+        u = _build_create_shlink_url("https://short.example")
         assert u == "https://short.example/rest/v3/short-urls"
 
     def test_strips_trailing_slash_on_base(self):
-        u = _build_create_shlink_url("http://a.com", "https://short.example/", "k1")
+        u = _build_create_shlink_url("https://short.example/")
         assert u == "https://short.example/rest/v3/short-urls"
 
     def test_bare_hostname_gets_https_scheme(self):
-        u = _build_create_shlink_url("http://a.com", "short.example", "k1")
+        u = _build_create_shlink_url("short.example")
         assert u == "https://short.example/rest/v3/short-urls"
 
-    def test_api_key_never_appears_in_the_url(self):
-        """Shlink authenticates via the X-Api-Key header, not a query param."""
-        u = _build_create_shlink_url("http://a.com", "https://short.example", "super-secret")
-        assert "super-secret" not in u
+    def test_preserves_a_path_prefix(self):
+        u = _build_create_shlink_url("https://short.example/shlink")
+        assert u == "https://short.example/shlink/rest/v3/short-urls"
 
 
 class TestCoerceUrlString:
@@ -185,15 +184,47 @@ class TestShortenUrlSync:
         assert call_url.startswith("https://v.gd/create.php")
 
     def test_http_error_returns_empty(self):
+        """A failing response body must never be returned as the short URL.
+
+        The body here is what a reverse proxy in front of a self-hosted is.gd-compatible
+        shortener actually serves on 502 -- it parses as a URL, so without the
+        response.ok guard it would be transmitted over RF as the shortened link.
+        """
         cfg = _minimal_config()
         mock_resp = MagicMock()
         mock_resp.ok = False
-        mock_resp.status_code = 503
+        mock_resp.status_code = 502
+        mock_resp.text = "http://short.example/maintenance"
         session = MagicMock()
         session.get.return_value = mock_resp
 
         assert shorten_url_sync("http://a.com", config=cfg, session=session) == ""
 
+    def test_http_error_is_logged_at_warning(self):
+        cfg = _minimal_config()
+        mock_resp = MagicMock()
+        mock_resp.ok = False
+        mock_resp.status_code = 502
+        mock_resp.text = "http://short.example/maintenance"
+        session = MagicMock()
+        session.get.return_value = mock_resp
+        logger = MagicMock()
+
+        shorten_url_sync("http://a.com", config=cfg, session=session, logger=logger)
+
+        assert logger.warning.called
+
+    def test_timeout_is_not_logged_as_an_unexpected_error(self):
+        """A shortener timeout is expected on an intermittent uplink, not an error."""
+        cfg = _minimal_config()
+        session = MagicMock()
+        session.get.side_effect = requests.exceptions.Timeout("timed out")
+        logger = MagicMock()
+
+        assert shorten_url_sync("http://a.com", config=cfg, session=session, logger=logger) == ""
+        logger.error.assert_not_called()
+        assert logger.debug.called
+
     def test_default_base_when_keys_missing(self):
         cfg = _minimal_config()
         mock_resp = MagicMock()
@@ -243,6 +274,7 @@ class TestShortenUrlSyncShlink:
     def test_success_returns_short_url(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"shortUrl": "https://short.example/abc123"}
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -255,6 +287,7 @@ class TestShortenUrlSyncShlink:
     def test_posts_to_rest_v3_short_urls_with_api_key_header(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"shortUrl": "https://short.example/abc123"}
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -269,19 +302,85 @@ class TestShortenUrlSyncShlink:
         assert payload["longUrl"] == "https://example.com/long/path"
         assert payload["findIfExists"] is True
 
-    def test_falls_back_to_short_url_slug(self):
+    def test_bare_slug_in_response_is_not_treated_as_a_url(self):
+        """Shlink's create response carries shortUrl; a slug is not a link.
+
+        Returning one would put an unclickable `abc123` where the reply expects a URL.
+        """
         cfg = self._shlink_config()
         mock_resp = MagicMock()
-        mock_resp.json.return_value = {"shortUrlSlug": "abc123"}
+        mock_resp.ok = True
+        mock_resp.json.return_value = {"shortUrlSlug": "abc123", "shortCode": "abc123"}
         session = MagicMock()
         session.post.return_value = mock_resp
 
-        out = shorten_url_sync("http://a.com", config=cfg, session=session)
-        assert out == "abc123"
+        assert shorten_url_sync("http://a.com", config=cfg, session=session) == ""
+
+    def test_missing_base_does_not_send_the_api_key_anywhere(self):
+        """Regression: an unset base must not fall back to the public v.gd default.
+
+        _normalize_base defaults to https://v.gd, so without an explicit guard a
+        shlink deployment with no short_url_website POSTs the operator's API key to
+        an unrelated third party.
+        """
+        cfg = self._shlink_config(short_url_website="")
+        session = MagicMock()
+        logger = MagicMock()
+
+        out = shorten_url_sync("http://a.com", config=cfg, session=session, logger=logger)
+
+        assert out == ""
+        session.post.assert_not_called()
+        assert logger.warning.called
+
+    def test_base_pointing_at_the_public_vgd_api_is_refused(self):
+        """v.gd is not Shlink; sending it an X-Api-Key only discloses the key."""
+        cfg = self._shlink_config(short_url_website="https://v.gd")
+        session = MagicMock()
+
+        assert shorten_url_sync("http://a.com", config=cfg, session=session) == ""
+        session.post.assert_not_called()
+
+    def test_http_error_returns_empty_and_warns(self):
+        """A bad API key must be diagnosable above DEBUG.
+
+        Shlink reports failures as RFC 7807 problem details, which parse as JSON and
+        simply lack shortUrl -- indistinguishable from an unshortenable URL without
+        checking the status.
+        """
+        cfg = self._shlink_config()
+        mock_resp = MagicMock()
+        mock_resp.ok = False
+        mock_resp.status_code = 401
+        mock_resp.text = '{"title": "Invalid API key", "status": 401}'
+        session = MagicMock()
+        session.post.return_value = mock_resp
+        logger = MagicMock()
+
+        out = shorten_url_sync("http://a.com", config=cfg, session=session, logger=logger)
+
+        assert out == ""
+        assert logger.warning.called
+
+    def test_http_error_does_not_leak_the_api_key_into_the_log(self):
+        cfg = self._shlink_config()
+        mock_resp = MagicMock()
+        mock_resp.ok = False
+        mock_resp.status_code = 401
+        mock_resp.text = "denied"
+        session = MagicMock()
+        session.post.return_value = mock_resp
+        logger = MagicMock()
+
+        shorten_url_sync("http://a.com", config=cfg, session=session, logger=logger)
+
+        logged = " ".join(str(c) for c in logger.warning.call_args_list)
+        assert "test-api-key" not in logged
 
     def test_missing_short_url_in_response_returns_empty(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"unexpected": "shape"}
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -309,6 +408,7 @@ class TestShortenUrlSyncShlink:
     def test_malformed_json_response_returns_empty(self):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.side_effect = ValueError("not json")
         session = MagicMock()
         session.post.return_value = mock_resp
@@ -319,6 +419,7 @@ class TestShortenUrlSyncShlink:
     def test_no_session_uses_requests_post(self, mock_post):
         cfg = self._shlink_config()
         mock_resp = MagicMock()
+        mock_resp.ok = True
         mock_resp.json.return_value = {"shortUrl": "https://short.example/xyz"}
         mock_post.return_value = mock_resp
 
@@ -352,6 +453,7 @@ async def test_shorten_url_async_shlink():
         short_url_website_api_key="test-api-key",
     )
     mock_resp = MagicMock()
+    mock_resp.ok = True
     mock_resp.json.return_value = {"shortUrl": "https://short.example/async1"}
     session = MagicMock()
     session.post.return_value = mock_resp

tests/unit/test_response_template.py

@@ -8,7 +8,7 @@ import pytest
 
 from modules.commands.test_command import TestCommand as MeshTestCommand
 from modules.models import MeshMessage
-from modules.response_template import format_piped_template
+from modules.response_template import format_piped_template, format_piped_template_async
 from modules.utils import message_path_bytes_per_hop
 
 
@@ -402,3 +402,181 @@ def test_quoted_filter_argument_with_a_nested_placeholder_does_not_close_early()
     assert format_piped_template(template, {"packet_hash": "ABCDEF12"}) == (
         "https://analyzer.example.net/#/packets/ABCDEF12?obs=1620457"
     )
+
+
+def _shortener_config(**external_data):
+    """Config a real shorten_url_sync call will accept, defaulting to v.gd."""
+    c = configparser.ConfigParser()
+    c["External_Data"] = {}
+    for k, v in external_data.items():
+        c["External_Data"][k] = v
+    return c
+
+
+@pytest.mark.unit
+def test_prefix_if_nonempty_accepts_a_quoted_argument_with_a_nested_placeholder():
+    """Regression: the greedy branch used to win over the quoted-argument grammar.
+
+    prefix_if_nonempty is the one filter already in shipped configs, so without this
+    the documented quoted syntax emitted raw template text over RF instead.
+    """
+    out = format_piped_template(
+        '{path_distance|prefix_if_nonempty:"Dist {sender}: "}',
+        {"path_distance": "5km", "sender": "y"},
+    )
+    assert out == "Dist y: 5km"
+
+
+@pytest.mark.unit
+def test_prefix_if_nonempty_with_a_quoted_argument_can_be_chained():
+    """A quoted arg ends at its closing quote, so a later filter is not swallowed."""
+    out = format_piped_template(
+        '{d|prefix_if_nonempty:"L "|if_nonempty:Z}',
+        {"d": "5km"},
+    )
+    assert out == "Z"
+
+
+@pytest.mark.unit
+def test_prefix_if_nonempty_keeps_greedy_parsing_for_unquoted_literals():
+    """config.ini.example ships `prefix_if_nonempty: | Path Dist: ` -- a literal
+    containing a pipe, which only parses if unquoted args stay greedy."""
+    out = format_piped_template(
+        "ack{path_distance|pathbytes_min:2|prefix_if_nonempty: | Path Dist: }",
+        {"path_distance": "12.4km"},
+        message=_msg(path="0102 (1 hop)", hops=1,
+                     routing_info={"path_length": 1, "bytes_per_hop": 2}),
+    )
+    assert out == "ack | Path Dist: 12.4km"
+
+
+@pytest.mark.unit
+def test_shorten_url_filter_shortens_through_the_configured_service():
+    """End-to-end: the filter reaches shorten_url_sync via ctx['config']."""
+    cfg = _shortener_config(short_url_website="https://v.gd")
+    resp = MagicMock()
+    resp.ok = True
+    resp.text = "https://v.gd/abc123"
+
+    with pytest.MonkeyPatch.context() as mp:
+        import modules.url_shortener as us
+        mp.setattr(us.requests, "get", lambda *a, **k: resp)
+        out = format_piped_template(
+            "{link|shorten_url}",
+            {"link": "https://example.com/a/very/long/path"},
+            config=cfg,
+        )
+    assert out == "https://v.gd/abc123"
+
+
+@pytest.mark.unit
+def test_shorten_is_an_alias_for_shorten_url():
+    """Feed formats document `shorten`; the same name must work here."""
+    cfg = _shortener_config(short_url_website="https://v.gd")
+    resp = MagicMock()
+    resp.ok = True
+    resp.text = "https://v.gd/abc123"
+
+    with pytest.MonkeyPatch.context() as mp:
+        import modules.url_shortener as us
+        mp.setattr(us.requests, "get", lambda *a, **k: resp)
+        out = format_piped_template(
+            "{link|shorten}", {"link": "https://example.com/long"}, config=cfg
+        )
+    assert out == "https://v.gd/abc123"
+
+
+@pytest.mark.unit
+def test_shorten_url_falls_back_to_the_long_url_when_shortening_fails():
+    """A failing shortener costs a longer message, never a broken one."""
+    cfg = _shortener_config(short_url_website="https://v.gd")
+    resp = MagicMock()
+    resp.ok = False
+    resp.status_code = 502
+    resp.text = "http://short.example/maintenance"
+
+    with pytest.MonkeyPatch.context() as mp:
+        import modules.url_shortener as us
+        mp.setattr(us.requests, "get", lambda *a, **k: resp)
+        out = format_piped_template(
+            "{link|shorten_url}", {"link": "https://example.com/long"}, config=cfg
+        )
+    assert out == "https://example.com/long"
+
+
+@pytest.mark.unit
+def test_shorten_url_without_a_config_passes_the_value_through():
+    out = format_piped_template("{link|shorten_url}", {"link": "https://example.com/long"})
+    assert out == "https://example.com/long"
+
+
+@pytest.mark.unit
+def test_if_notempty_is_an_alias_for_if_nonempty():
+    assert format_piped_template("{d|if_nonempty:Z}", {"d": "x"}) == "Z"
+    assert format_piped_template("{d|if_notempty:Z}", {"d": "x"}) == "Z"
+    assert format_piped_template("{d|if_nonempty:Z}", {"d": ""}) == ""
+    assert format_piped_template("{d|if_notempty:Z}", {"d": ""}) == ""
+
+
+@pytest.mark.asyncio
+async def test_format_piped_template_async_matches_the_sync_render():
+    """The async wrapper exists so a shorten filter cannot block the event loop."""
+    cfg = _shortener_config(short_url_website="https://v.gd")
+    resp = MagicMock()
+    resp.ok = True
+    resp.text = "https://v.gd/abc123"
+
+    with pytest.MonkeyPatch.context() as mp:
+        import modules.url_shortener as us
+        mp.setattr(us.requests, "get", lambda *a, **k: resp)
+        out = await format_piped_template_async(
+            "{link|shorten_url}", {"link": "https://example.com/long"}, config=cfg
+        )
+    assert out == "https://v.gd/abc123"
+
+
+@pytest.mark.asyncio
+async def test_shorten_url_warns_when_rendered_on_the_event_loop():
+    """A blocking HTTP call on the loop stalls radio RX and every other handler."""
+    cfg = _shortener_config(short_url_website="https://v.gd")
+    resp = MagicMock()
+    resp.ok = True
+    resp.text = "https://v.gd/abc123"
+    logger = MagicMock()
+
+    with pytest.MonkeyPatch.context() as mp:
+        import modules.response_template as rt
+        import modules.url_shortener as us
+        mp.setattr(us.requests, "get", lambda *a, **k: resp)
+        mp.setattr(rt, "_warned_blocking_render", False)
+        format_piped_template(
+            "{link|shorten_url}",
+            {"link": "https://example.com/long"},
+            config=cfg,
+            logger=logger,
+        )
+
+    warned = " ".join(str(c) for c in logger.warning.call_args_list)
+    assert "event loop" in warned
+
+
+@pytest.mark.asyncio
+async def test_async_render_does_not_warn_about_the_event_loop():
+    cfg = _shortener_config(short_url_website="https://v.gd")
+    resp = MagicMock()
+    resp.ok = True
+    resp.text = "https://v.gd/abc123"
+    logger = MagicMock()
+
+    with pytest.MonkeyPatch.context() as mp:
+        import modules.url_shortener as us
+        mp.setattr(us.requests, "get", lambda *a, **k: resp)
+        out = await format_piped_template_async(
+            "{link|shorten_url}",
+            {"link": "https://example.com/long"},
+            config=cfg,
+            logger=logger,
+        )
+
+    assert out == "https://v.gd/abc123"
+    logger.warning.assert_not_called()