browser/gemini: avoid inf. recursion on code 60

If the server would blindly respond with code 60 even though we present
a client certificate, it would trigger an infinite recursive call
(open_gemini_url → _handle_response → _handle_cert_required).

bebop/browser/browser.py

@@ -69,6 +69,7 @@ class Browser:
 
     SEARCH_NEXT = 0
     SEARCH_PREVIOUS = 1
+    MAX_REDIRECTIONS = 5
 
     def __init__(self, config, cert_stash):
         self.config = config
@@ -420,7 +421,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
 

bebop/browser/gemini.py

@@ -24,8 +24,8 @@ MAX_URL_LEN = 1024
 def open_gemini_url(
     browser: Browser,
     url: str,
-    redirects: int =0,
-    use_cache: bool =False,
+    redirects: int = 0,
+    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;
-      as we're doing TOFU here, we could automatically trust it or let the user
-      choose. For simplicity, we always trust it permanently.
+    - STATE_UNKNOWN_CERT: the certificate is valid but has not been seen
+      before; as we're doing TOFU here, we could automatically trust it or let
+      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
         )
 
-    data = req.proceed()
+    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.
 
@@ -171,7 +177,11 @@ def _handle_response(
     elif response.generic_code == 10:
         return _handle_input_request(browser, url, response.meta)
     elif response.code == 60:
-        return _handle_cert_required(browser, response, url, redirects)
+        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
         error = f"Client certificate error: {details}"
@@ -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/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
-       check stuff.
+    2. `connect` opens the connection and aborts it or leaves the caller free
+      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
-      during `proceed`.
+    - payload: bytes object of the payload request; build during `connect`,
+      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
-        caller to decide whether to continue (call `proceed`) the connection or
-        abort it (call `abort`).
+        there is no need to call `abort`. If connect returns True, it is up to
+        the caller to decide whether to continue (call `proceed`) the
+        connection or abort it (call `abort`).
 
-        The request `state` is updated to reflect the connection state after the
-        function returns. The following list describes states related to
+        The request `state` is updated to reflect the connection state after
+        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
-        returned):
+        The following list describes states related to validation failure
+        (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