protocol: catch create_connection UnicodeErrors

The unknown config value will be displayed without warning or anything
to the user in the help page, which can be confusing e.g. when mixing
dash and underscore in keys. oh well

BOARD.txt

@@ -29,13 +29,14 @@ handle big files (e.g. gemini://tilde.team/~tomasino/irc/log.txt)
 allow encoding overrides (useful for gopher i guess)
 config for web browser, default to webbrowser module
 use pubkeys instead of the whole DER hash for TOFU
-reload after forgetting old cert
 specify external commands interface (exec, run, pipes)
 table of contents
 better default editor than vim
 search engine
 auto-open some media types
-use about scheme instead of bebop
+don't store explicit port 1965 in URL prefixes (e.g. for identities)
+fix shifted line detection when text goes belong the horizontal limit
+auto-rewrite missing "/" in empty URL paths
 
 
 

README.md

@@ -54,9 +54,8 @@ have it installed, check out this Gemini link `gemini://dece.space/dev/faq/using
 
 [py-faq-http]: https://portal.mozz.us/gemini/dece.space/dev/faq/using-python-programs.gmi
 
-The easier installation method is using Pip, either user or system-wide
+The recommended installation method is using Pipx, but using Pip is fine, either
-installation. I recommend user installation, but a system-wide installation
+user or system-wide installation.
-should not cause issues as there are no dependencies.
 
 ```bash
 # User installation:
@@ -124,6 +123,16 @@ Here is a list of plugins I did, available on PyPI:
 [plugin-finger]: plugins/finger/README.md
 [plugin-gopher]: plugins/gopher/README.md
 
+Plugins have to be installed in the same Python environment as Bebop. If you
+installed Bebop using Pipx, this is done using the `inject` command.
+
+```bash
+# Installating the Gopher plugin in the user environment:
+pip3 install --user bebop-browser-gopher
+# Installating the Gopher plugin for a Pipx installation:
+pipx inject bebop-browser bebop-browser-gopher
+```
+
 
 
 Usage

bebop/browser/browser.py

@@ -19,7 +19,7 @@ from bebop.bookmarks import (
 )
 from bebop.colors import A_ITALIC, ColorPair, init_colors
 from bebop.command_line import CommandLine
-from bebop.external import open_external_program, substitute_external_command
+from bebop.external import open_external_program
 from bebop.fs import get_capsule_prefs_path, get_identities_list_path
 from bebop.help import get_help
 from bebop.history import History
@@ -69,6 +69,7 @@ class Browser:
 
     SEARCH_NEXT = 0
     SEARCH_PREVIOUS = 1
+    MAX_REDIRECTIONS = 5
 
     def __init__(self, config, cert_stash):
         self.config = config
@@ -268,8 +269,6 @@ class Browser:
             self.move_to_search_result(Browser.SEARCH_PREVIOUS)
         elif char == ord("i"):
             self.show_page_info()
-        elif char == ord("!"):
-            self.quick_command("exec")
         elif curses.ascii.isdigit(char):
             self.handle_digit_input(char)
         elif char == curses.KEY_MOUSE:
@@ -386,8 +385,6 @@ class Browser:
         else:
             if command in ("o", "open"):
                 self.open_url(words[1])
-            elif command == "exec":
-                self.exec_external_command(" ".join(words[1:]))
             elif command == "forget-certificate":
                 from bebop.browser.gemini import forget_certificate
                 forget_certificate(self, words[1])
@@ -426,7 +423,7 @@ class Browser:
         - history: whether the URL should be pushed to history on success.
         - use_cache: whether we should look for an already cached document.
         """
-        if redirects > 5:
+        if redirects > self.MAX_REDIRECTIONS:
             self.set_status_error(f"Too many redirections ({url}).")
             return
 
@@ -792,7 +789,7 @@ class Browser:
         encoding = page.encoding or "unk. encoding"
         size = f"{len(page.source)} chars"
         lines = f"{len(page.metalines)} lines"
-        info = f"{mime}; {encoding}; {size}; {lines}"
+        info = f"{mime}  {encoding}  {size}  {lines}"
         self.set_status(info)
 
     def set_render_mode(self, mode):
@@ -893,16 +890,3 @@ class Browser:
                 continue
 
             logging.info(f"Loaded plugin {plugin_name}.")
-
-    def exec_external_command(self, command):
-        if not self.current_page:
-            return
-        command = substitute_external_command(
-                command, self.current_url, self.current_page)
-        self.set_status(f"Launching `{command}`…")
-        success = open_external_program(command)
-        if success:
-            self.reset_status()
-        else:
-            self.set_status_error("Command failed.")
-        self.refresh_windows()

bebop/browser/gemini.py

@@ -24,8 +24,8 @@ MAX_URL_LEN = 1024
 def open_gemini_url(
     browser: Browser,
     url: str,
-    redirects: int =0,
+    redirects: int = 0,
-    use_cache: bool =False,
+    use_cache: bool = False,
     cert_and_key=None
 ) -> Optional[str]:
     """Open a Gemini URL and set the formatted response as content.
@@ -41,9 +41,9 @@ def open_gemini_url(
       present the user the problems found and let her decide whether to trust
       temporarily the certificate or not BUT we currently do not parse the
       certificate's fields, not even the pubkey, so this state is never used.
-    - STATE_UNKNOWN_CERT: the certificate is valid but has not been seen before;
+    - STATE_UNKNOWN_CERT: the certificate is valid but has not been seen
-      as we're doing TOFU here, we could automatically trust it or let the user
+      before; as we're doing TOFU here, we could automatically trust it or let
-      choose. For simplicity, we always trust it permanently.
+      the user choose. For simplicity, we always trust it permanently.
 
     Arguments:
     - browser: Browser object making the request.
@@ -111,16 +111,21 @@ def open_gemini_url(
             trust_always=True
         )
 
+    try:
         data = req.proceed()
+    except OSError:
+        browser.set_status_error(f"Connection error ({url}).")
+        return None
     if not data:
-        browser.set_status_error(f"Server did not respond in time ({url}).")
+        browser.set_status_error(f"Response empty or timed out ({url}).")
         return None
     response = Response.parse(data)
     if not response:
-        browser.set_status_error(f"Server response parsing failed ({url}).")
+        browser.set_status_error(f"Response parsing failed ({url}).")
         return None
 
-    return _handle_response(browser, response, url, redirects)
+    return _handle_response(browser, response, url, redirects,
+                            used_cert=cert_and_key is not None)
 
 
 def _handle_untrusted_cert(browser: Browser, request: Request):
@@ -147,7 +152,8 @@ def _handle_response(
     browser: Browser,
     response: Response,
     url: str,
-    redirects: int
+    redirects: int,
+    used_cert: bool = False,
 ) -> Optional[str]:
     """Handle a response from a Gemini server.
 
@@ -166,11 +172,15 @@ def _handle_response(
             redirects=redirects + 1
         )
     elif response.generic_code in (40, 50):
-        error = f"Server error: {response.meta or response.code.name}"
+        error = f"Server error: {response.meta or Response.code.name}"
         browser.set_status_error(error)
     elif response.generic_code == 10:
         return _handle_input_request(browser, url, response.meta)
     elif response.code == 60:
+        if used_cert:
+            error = "Server ignored our certificate."
+            browser.set_status_error(error)
+        else:
             return _handle_cert_required(browser, response, url, redirects)
     elif response.code in (61, 62):
         details = response.meta or Response.code.name
@@ -182,7 +192,11 @@ def _handle_response(
     return None
 
 
-def _handle_successful_response(browser: Browser, response: Response, url: str):
+def _handle_successful_response(
+    browser: Browser,
+    response: Response,
+    url: str
+):
     """Handle a successful response content from a Gemini server.
 
     According to the MIME type received or inferred, the response is either
@@ -215,7 +229,8 @@ def _handle_successful_response(browser: Browser, response: Response, url: str):
                 error = f"Unknown encoding {encoding}."
             else:
                 render_opts = get_render_options(browser.config)
-                pref_mode = get_url_render_mode_pref(browser.capsule_prefs, url)
+                pref_mode = get_url_render_mode_pref(
+                    browser.capsule_prefs, url)
                 if pref_mode:
                     render_opts.mode = pref_mode
                 page = Page.from_gemtext(text, render_opts)
@@ -255,7 +270,7 @@ def _handle_successful_response(browser: Browser, response: Response, url: str):
 def _handle_input_request(
     browser: Browser,
     from_url: str,
-    message: str =None
+    message: str = None
 ) -> Optional[str]:
     """Focus command-line to pass input to the server.
 
@@ -311,11 +326,12 @@ def _handle_cert_required(
 
 def select_identity(identities: list):
     """Let user select the appropriate identity among candidates."""
-    # TODO support multiple identities; for now we just use the first available.
+    # TODO support multiple identities; for now we just use the first
+    # available.
     return identities[0] if identities else None
 
 
-def create_identity(browser: Browser, url: str, reason: Optional[str] =None):
+def create_identity(browser: Browser, url: str, reason: Optional[str] = None):
     """Walk the user through identity creation.
 
     Returns:
@@ -352,7 +368,7 @@ def create_identity(browser: Browser, url: str, reason: Optional[str] =None):
 
 
 def forget_certificate(browser: Browser, hostname: str):
-    """Remove the fingerprint associated to this hostname for the cert stash."""
+    """Remove the fingerprint for this hostname from the cert stash."""
     key = browser.prompt(f"Remove fingerprint for {hostname}?")
     if key != "y":
         browser.reset_status()
@@ -360,4 +376,5 @@ def forget_certificate(browser: Browser, hostname: str):
     if untrust_fingerprint(browser.stash, hostname):
         browser.set_status(f"Known certificate for {hostname} removed.")
     else:
-        browser.set_status_error(f"Known certificate for {hostname} not found.")
+        browser.set_status_error(
+            f"Known certificate for {hostname} not found.")

bebop/external.py

@@ -2,24 +2,7 @@
 
 import curses
 import logging
-import re
 import subprocess
-import tempfile
-
-from bebop.page import Page
-
-
-def _pre_exec():
-    curses.nocbreak()
-    curses.echo()
-    curses.curs_set(1)
-
-
-def _post_exec():
-    curses.mousemask(curses.ALL_MOUSE_EVENTS)
-    curses.curs_set(0)
-    curses.noecho()
-    curses.cbreak()
 
 
 def open_external_program(command):
@@ -31,48 +14,17 @@ def open_external_program(command):
     Returns:
     True if no exception occured.
     """
-    _pre_exec()
+    curses.nocbreak()
+    curses.echo()
+    curses.curs_set(1)
     result = True
     try:
         subprocess.run(command)
     except OSError as exc:
         logging.error(f"Failed to run '{command}': {exc}")
         result = False
-    _post_exec()
+    curses.mousemask(curses.ALL_MOUSE_EVENTS)
+    curses.curs_set(0)
+    curses.noecho()
+    curses.cbreak()
     return result
-
-
-SUB_URL_RE = re.compile(r"(?<!\$)\$u")
-SUB_SRC_RE = re.compile(r"(?<!\$)\$s")
-SUB_LINK_RE = re.compile(r"(?<!\$)\$(\d+)")
-SUB_LITERAL_RE = re.compile(r"\$\$")
-
-
-def substitute_external_command(command: str, url: str, page: Page):
-    """Substitute "$" parts of the command with corresponding values.
-
-    Valid substitutions are:
-    - $u = current url
-    - $n (with n any positive number) = link url
-    - $s = current page source temp file
-    - $$ = $
-
-    Returns:
-    The command with all the template parts replaced with the corresponding
-    strings.
-
-    Raises:
-    ValueError if a substitution is wrong, e.g. a link ID which does not exist.
-    """
-    # URL substitution.
-    command = SUB_URL_RE.sub(url, command)
-    # Source file substitution.
-    if SUB_SRC_RE.search(command):
-        with tempfile.NamedTemporaryFile("wt", delete=False) as source_file:
-            source_file.write(page.source)
-            command = SUB_SRC_RE.sub(source_file.name, command)
-    # Link ID substitution.
-    command = SUB_LINK_RE.sub(lambda m: page.links[int(m.group(1))], command)
-    # Literal dollar sign.
-    command = SUB_LITERAL_RE.sub("$", command)
-    return command

bebop/gemtext.py

@@ -102,7 +102,6 @@ def parse_gemtext(text: str, dumb=False) -> ParsedGemtext:
         match = Blockquote.RE.match(line)
         if match:
             text = match.groups()[0]
-            if text or dumb:
             elements.append(Blockquote(text))
             continue
 

bebop/help.py

@@ -104,7 +104,7 @@ def get_help(config, plugins):
     config_list = "\n".join(
         (
             f"* {key} = {config[key]} (default {repr(DEFAULT_CONFIG[key])})"
-            if config[key] != DEFAULT_CONFIG[key]
+            if key in DEFAULT_CONFIG and config[key] != DEFAULT_CONFIG[key]
             else f"* {key} = {config[key]}"
         )
         for key in sorted(config)

bebop/protocol.py

@@ -26,8 +26,8 @@ class Request:
     sending the request header and receiving the response:
 
     1. Instantiate a Request.
-    2. `connect` opens the connection and aborts it or leaves the caller free to
+    2. `connect` opens the connection and aborts it or leaves the caller free
-       check stuff.
+      to check stuff.
     3. `proceed` or `abort` can be called.
 
     Attributes:
@@ -35,8 +35,8 @@ class Request:
     - cert_stash: certificate stash to use an possibly update.
     - state: request state.
     - hostname: hostname derived from url, stored when `connect` is called.
-    - payload: bytes object of the payload request; build during `connect`, used
+    - payload: bytes object of the payload request; build during `connect`,
-      during `proceed`.
+      used during `proceed`.
     - ssock: TLS-wrapped socket.
     - cert_validation: validation results dict, set after certificate has been
       reviewed.
@@ -79,12 +79,12 @@ class Request:
         certificate status is not CertStatus.VALID (Request.STATE_OK).
 
         If connect returns False, the secure socket is aborted before return so
-        there is no need to call `abort`. If connect returns True, it is up to the
+        there is no need to call `abort`. If connect returns True, it is up to
-        caller to decide whether to continue (call `proceed`) the connection or
+        the caller to decide whether to continue (call `proceed`) the
-        abort it (call `abort`).
+        connection or abort it (call `abort`).
 
-        The request `state` is updated to reflect the connection state after the
+        The request `state` is updated to reflect the connection state after
-        function returns. The following list describes states related to
+        the function returns. The following list describes states related to
         connection failure (False returned):
 
         - STATE_INVALID_URL: URL is not valid.
@@ -95,8 +95,8 @@ class Request:
         For all request states from now on, the `cert_validation` attribute is
         updated with the result of the certificate validation.
 
-        The following list describes states related to validation failure (False
+        The following list describes states related to validation failure
-        returned):
+        (False returned):
 
         - STATE_ERROR_CERT: server certificate could not be validated at all.
         - STATE_UNTRUSTED_CERT: server certificate mismatched the known
@@ -117,7 +117,8 @@ class Request:
 
         - The DER hash is compared against the fingerprint for this hostname
           *and port*; the specification does not tell much about that, but we
-          are slightly more restrictive here by adding the port in the equation.
+          are slightly more restrictive here by adding the port in the
+          equation.
         - The state STATE_INVALID_CERT is actually never used in Bebop because
           of the current tendency to ignore any certificate fields and only
           check the whole cert fingerprint. Here it is considered the same as a
@@ -152,6 +153,11 @@ class Request:
             self.state = Request.STATE_CONNECTION_FAILED
             self.error = exc.strerror
             return False
+        except ValueError as exc:
+            self.state = Request.STATE_INVALID_URL
+            self.error = "Some connection parameter is wrong, check again."
+            logging.error(f"ValueError during connection creation: {exc}")
+            return False
 
         # Setup TLS.
         context = Request.get_ssl_context()

bebop/tests/test_external.py

@@ -1,45 +0,0 @@
-from bebop.metalines import RenderOptions
-import unittest
-
-from ..external import substitute_external_command
-from ..page import Page
-
-
-URL = "gemini://example.com"
-GEMTEXT = """\
-# Test page
-
-Blablabla
-
-=> gemini://example.com/index.gmi Link to index
-=> gemini://example.com/sub/gemlog.gmi Link to gemlog
-"""
-PAGE = Page.from_gemtext(GEMTEXT, RenderOptions(80, "fancy", "- "))
-
-
-class TestExternal(unittest.TestCase):
-
-    def test_substitute_external_command(self):
-        # Replace URLs occurences.
-        command = "gmni $u | grep $u"  # Grep for a page's own URL.
-        result = substitute_external_command(command, URL, PAGE)
-        self.assertEqual(result, "gmni {u} | grep {u}".format(u=URL))
-
-        # Replace link ID's with the target URL.
-        command = "gmni $1 && gmni $2"  # Get both links
-        result = substitute_external_command(command, URL, PAGE)
-        expected = (
-            "gmni gemini://example.com/index.gmi"
-            " && gmni gemini://example.com/sub/gemlog.gmi"
-        )
-        self.assertEqual(result, expected)
-
-        # Invalid link ID raise a ValueError.
-        command = "gmni $8"
-        with self.assertRaises(Exception):
-            substitute_external_command(command, URL, PAGE)
-
-        # Replace escaped $$ with literal $.
-        command = "grep ^iamaregex$$ | echo dollar $"  # Do nothing with last.
-        result = substitute_external_command(command, URL, PAGE)
-        self.assertEqual(result, "grep ^iamaregex$ | echo dollar $")