Let nikola serve work together with non-root BASE_URL or SITE_URL. Fixes #3726

CHANGES.txt

@@ -12,6 +12,7 @@ Bugfixes
 * Restore `annotation_helper.tmpl` with dummy content - fix themes still mentioning it
   (Issue #3764, #3773)
 * Fix compatibility with watchdog 4 (Issue #3766)
+* `nikola serve` now works with non-root SITE_URL.
 
 New in v8.3.1
 =============

nikola/plugins/command/auto/__init__.py

@@ -35,14 +35,14 @@
 import subprocess
 import sys
 import typing
-import urllib.parse
 import webbrowser
 from pathlib import Path
 
 import blinker
 
 from nikola.plugin_categories import Command
 from nikola.utils import dns_sd, req_missing, get_theme_path, makedirs, pkg_resources_path
+from nikola.plugins.command.basepath_helper import base_path_from_siteuri
 
 try:
     import aiohttp
@@ -67,17 +67,6 @@
 IDLE_REFRESH_DELAY = 0.05
 
 
-def base_path_from_siteuri(siteuri: str) -> str:
-    """Extract the path part from a URI such as site['SITE_URL'].
-
-    The path never ends with a "/". (If only "/" is intended, it is empty.)
-    """
-    path = urllib.parse.urlsplit(siteuri).path
-    if path.endswith("/"):
-        path = path[:-1]
-    return path
-
-
 class CommandAuto(Command):
     """Automatic rebuilds for Nikola."""
 

nikola/plugins/command/basepath_helper.py

@@ -0,0 +1,14 @@
+"""Extracting the base path from site['SITE_URL'] or site['BASE_URL']."""
+
+import urllib
+
+
+def base_path_from_siteuri(siteuri: str) -> str:
+    """Extract the path part from a URI such as site['SITE_URL'].
+
+    The path returned doesn't end with a "/". (If only "/" is intended, it is empty.)
+    """
+    path = urllib.parse.urlsplit(siteuri).path
+    if path.endswith("/"):
+        path = path[:-1]
+    return path

nikola/plugins/command/serve.py

@@ -25,19 +25,23 @@
 # SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 """Start test server."""
-
+import atexit
 import os
 import sys
 import re
 import signal
 import socket
+import threading
 import webbrowser
 from http.server import HTTPServer
 from http.server import SimpleHTTPRequestHandler
 from io import BytesIO as StringIO
+from threading import Thread, current_thread
+from typing import Callable, Optional
 
 from nikola.plugin_categories import Command
 from nikola.utils import dns_sd
+from nikola.plugins.command.basepath_helper import base_path_from_siteuri
 
 
 class IPv6Server(HTTPServer):
@@ -52,7 +56,8 @@ class CommandServe(Command):
     name = "serve"
     doc_usage = "[options]"
     doc_purpose = "start the test webserver"
-    dns_sd = None
+    httpd: Optional[HTTPServer] = None
+    httpd_serving_thread: Optional[Thread] = None
 
     cmd_options = (
         {
@@ -98,13 +103,21 @@ class CommandServe(Command):
     )
 
     def shutdown(self, signum=None, _frame=None):
-        """Shut down the server that is running detached."""
-        if self.dns_sd:
-            self.dns_sd.Reset()
+        """Shut down the server."""
         if os.path.exists(self.serve_pidfile):
             os.remove(self.serve_pidfile)
-        if not self.detached:
-            self.logger.info("Server is shutting down.")
+
+        # Deal with the non-detached state:
+        if self.httpd is not None and self.httpd_serving_thread is not None and self.httpd_serving_thread != current_thread():
+            shut_me_down = self.httpd
+            self.httpd = None
+            self.httpd_serving_thread = None
+            self.logger.info("Web server is shutting down.")
+            shut_me_down.shutdown()
+        else:
+            self.logger.debug("No need to shut down the web server.")
+
+        # If this was called as a signal handler, shut down the entire application:
         if signum:
             sys.exit(0)
 
@@ -127,29 +140,33 @@ def _execute(self, options, args):
                 ipv6 = False
                 OurHTTP = HTTPServer
 
-            httpd = OurHTTP((options['address'], options['port']),
-                            OurHTTPRequestHandler)
-            sa = httpd.socket.getsockname()
+            base_path = base_path_from_siteuri(self.site.config['BASE_URL'])
+            if base_path == "":
+                handler_factory = OurHTTPRequestHandler
+            else:
+                handler_factory = _create_RequestHandler_removing_basepath(base_path)
+            self.httpd = OurHTTP((options['address'], options['port']), handler_factory)
+
+            sa = self.httpd.socket.getsockname()
             if ipv6:
-                server_url = "http://[{0}]:{1}/".format(*sa)
+                server_url = "http://[{0}]:{1}/".format(*sa) + base_path
             else:
-                server_url = "http://{0}:{1}/".format(*sa)
+                server_url = "http://{0}:{1}/".format(*sa) + base_path
             self.logger.info("Serving on {0} ...".format(server_url))
 
             if options['browser']:
                 # Some browsers fail to load 0.0.0.0 (Issue #2755)
                 if sa[0] == '0.0.0.0':
-                    server_url = "http://127.0.0.1:{1}/".format(*sa)
+                    server_url = "http://127.0.0.1:{1}/".format(*sa) + base_path
                 self.logger.info("Opening {0} in the default web browser...".format(server_url))
                 webbrowser.open(server_url)
             if options['detach']:
-                self.detached = True
                 OurHTTPRequestHandler.quiet = True
                 try:
                     pid = os.fork()
                     if pid == 0:
                         signal.signal(signal.SIGTERM, self.shutdown)
-                        httpd.serve_forever()
+                        self.httpd.serve_forever()
                     else:
                         with open(self.serve_pidfile, 'w') as fh:
                             fh.write('{0}\n'.format(pid))
@@ -160,11 +177,26 @@ def _execute(self, options, args):
                     else:
                         raise
             else:
-                self.detached = False
                 try:
-                    self.dns_sd = dns_sd(options['port'], (options['ipv6'] or '::' in options['address']))
-                    signal.signal(signal.SIGTERM, self.shutdown)
-                    httpd.serve_forever()
+                    dns_socket_publication = dns_sd(options['port'], (options['ipv6'] or '::' in options['address']))
+                    try:
+                        self.httpd_serving_thread = threading.current_thread()
+                        if threading.main_thread() == self.httpd_serving_thread:
+                            # If we are running as the main thread,
+                            # likely no other threads are running and nothing else will run after us.
+                            # In this special case, we take some responsibility for the application whole
+                            # (not really the job of any single plugin).
+                            # Clean up the socket publication on exit (if we actually had a socket publication):
+                            if dns_socket_publication is not None:
+                                atexit.register(dns_socket_publication.Reset)
+                            # Enable application shutdown via SIGTERM:
+                            signal.signal(signal.SIGTERM, self.shutdown)
+                        self.logger.info("Starting web server.")
+                        self.httpd.serve_forever()
+                        self.logger.info("Web server has shut down.")
+                    finally:
+                        if dns_socket_publication is not None:
+                            dns_socket_publication.Reset()
                 except KeyboardInterrupt:
                     self.shutdown()
                     return 130
@@ -186,7 +218,7 @@ def log_message(self, *args):
 
     # NOTICE: this is a patched version of send_head() to disable all sorts of
     # caching.  `nikola serve` is a development server, hence caching should
-    # not happen to have access to the newest resources.
+    # not happen, instead, we should give access to the newest resources.
     #
     # The original code was copy-pasted from Python 2.7.  Python 3.3 contains
     # the same code, missing the binary mode comment.
@@ -205,6 +237,7 @@ def send_head(self):
 
         """
         path = self.translate_path(self.path)
+
         f = None
         if os.path.isdir(path):
             path_parts = list(self.path.partition('?'))
@@ -277,3 +310,29 @@ def send_head(self):
         # end no-cache patch
         self.end_headers()
         return f
+
+
+def _omit_basepath_component(base_path_with_slash: str, path: str) -> str:
+    if path.startswith(base_path_with_slash):
+        return path[len(base_path_with_slash) - 1:]
+    elif path == base_path_with_slash[:-1]:
+        return "/"
+    else:
+        # Somewhat dubious. We should not really get asked this, normally.
+        return path
+
+
+def _create_RequestHandler_removing_basepath(base_path: str) -> Callable:
+    """Create a new subclass of OurHTTPRequestHandler that removes a trailing base path from the path.
+
+    Returns that class (used as a factory for objects).
+    Better return type should be Callable[[...], OurHTTPRequestHandler], but Python 3.8 doesn't understand that.
+    """
+    base_path_with_slash = base_path if base_path.endswith("/") else f"{base_path}/"
+
+    class OmitBasepathRequestHandler(OurHTTPRequestHandler):
+
+        def translate_path(self, path: str) -> str:
+            return super().translate_path(_omit_basepath_component(base_path_with_slash, path))
+
+    return OmitBasepathRequestHandler

nikola/utils.py

@@ -1862,7 +1862,7 @@ def color_hsl_adjust_hex(hexstr, adjust_h=None, adjust_s=None, adjust_l=None):
 
 
 def dns_sd(port, inet6):
-    """Optimistically publish a HTTP service to the local network over DNS-SD.
+    """Optimistically publish an HTTP service to the local network over DNS-SD.
 
     Works only on Linux/FreeBSD.  Requires the `avahi` and `dbus` modules (symlinks in virtualenvs)
     """

tests/integration/dev_server_test_helper.py

@@ -0,0 +1,43 @@
+import pathlib
+import socket
+from typing import Dict, Any
+
+from ..helper import FakeSite
+from nikola.utils import get_logger
+
+SERVER_ADDRESS = "localhost"
+TEST_MAX_DURATION = 10  # Watchdog: Give up the test if it did not succeed during this time span.
+
+# Folder that has the fixture file we expect the server to serve:
+OUTPUT_FOLDER = pathlib.Path(__file__).parent.parent / "data" / "dev_server_sample_output_folder"
+
+LOGGER = get_logger("test_dev_server")
+
+
+def find_unused_port() -> int:
+    """Ask the OS for a currently unused port number.
+
+    (More precisely, a port that can be used for a TCP server servicing SERVER_ADDRESS.)
+    We use a method here rather than a fixture to minimize side effects of failing tests.
+    """
+    s = socket.socket()
+    try:
+        ANY_PORT = 0
+        s.bind((SERVER_ADDRESS, ANY_PORT))
+        address, port = s.getsockname()
+        LOGGER.info("Trying to set up dev server on http://%s:%i/", address, port)
+        return port
+    finally:
+        s.close()
+
+
+class MyFakeSite(FakeSite):
+    def __init__(self, config: Dict[str, Any], configuration_filename="conf.py"):
+        super(MyFakeSite, self).__init__()
+        self.configured = True
+        self.debug = True
+        self.THEMES = []
+        self._plugin_places = []
+        self.registered_auto_watched_folders = set()
+        self.config = config
+        self.configuration_filename = configuration_filename

tests/integration/test_dev_server.py → tests/integration/test_dev_server_auto.py

@@ -1,50 +1,13 @@
 import asyncio
-import nikola.plugins.command.auto as auto
-from nikola.utils import get_logger
+from typing import Optional, Tuple
+
 import pytest
-import pathlib
 import requests
-import socket
-from typing import Optional, Tuple, Any, Dict
-
-from ..helper import FakeSite
-
-SERVER_ADDRESS = "localhost"
-TEST_MAX_DURATION = 10  # Watchdog: Give up the test if it did not succeed during this time span.
-
-# Folder that has the fixture file we expect the server to serve:
-OUTPUT_FOLDER = pathlib.Path(__file__).parent.parent / "data" / "dev_server_sample_output_folder"
-
-LOGGER = get_logger("test_dev_server")
 
-
-def find_unused_port() -> int:
-    """Ask the OS for a currently unused port number.
-
-    (More precisely, a port that can be used for a TCP server servicing SERVER_ADDRESS.)
-    We use a method here rather than a fixture to minimize side effects of failing tests.
-    """
-    s = socket.socket()
-    try:
-        ANY_PORT = 0
-        s.bind((SERVER_ADDRESS, ANY_PORT))
-        address, port = s.getsockname()
-        LOGGER.info("Trying to set up dev server on http://%s:%i/", address, port)
-        return port
-    finally:
-        s.close()
-
-
-class MyFakeSite(FakeSite):
-    def __init__(self, config: Dict[str, Any], configuration_filename="conf.py"):
-        super(MyFakeSite, self).__init__()
-        self.configured = True
-        self.debug = True
-        self.THEMES = []
-        self._plugin_places = []
-        self.registered_auto_watched_folders = set()
-        self.config = config
-        self.configuration_filename = configuration_filename
+import nikola.plugins.command.auto as auto
+from nikola.plugins.command.basepath_helper import base_path_from_siteuri
+from .dev_server_test_helper import MyFakeSite, SERVER_ADDRESS, find_unused_port, TEST_MAX_DURATION, LOGGER, \
+    OUTPUT_FOLDER
 
 
 def test_serves_root_dir(
@@ -157,7 +120,7 @@ def site_and_base_path(request) -> Tuple[MyFakeSite, str]:
         "SITE_URL": request.param,
         "OUTPUT_FOLDER": OUTPUT_FOLDER.as_posix(),
     }
-    return MyFakeSite(config), auto.base_path_from_siteuri(request.param)
+    return MyFakeSite(config), base_path_from_siteuri(request.param)
 
 
 @pytest.fixture(scope="module")
@@ -170,27 +133,3 @@ def expected_text():
     with open(OUTPUT_FOLDER / "index.html", encoding="utf-8") as html_file:
         all_html = html_file.read()
         return all_html[all_html.find("<body>"):]
-
-
-@pytest.mark.parametrize(("uri", "expected_basepath"), [
-    ("http://localhost", ""),
-    ("http://local.host", ""),
-    ("http://localhost/", ""),
-    ("http://local.host/", ""),
-    ("http://localhost:123/", ""),
-    ("http://local.host:456/", ""),
-    ("https://localhost", ""),
-    ("https://local.host", ""),
-    ("https://localhost/", ""),
-    ("https://local.host/", ""),
-    ("https://localhost:123/", ""),
-    ("https://local.host:456/", ""),
-    ("http://example.org/blog", "/blog"),
-    ("https://lorem.ipsum/dolet/", "/dolet"),
-    ("http://example.org:124/blog", "/blog"),
-    ("http://example.org:124/Deep/Rab_bit/hol.e/", "/Deep/Rab_bit/hol.e"),
-    # Would anybody in a sane mind actually do this?
-    ("http://example.org:124/blog?lorem=ipsum&dol=et", "/blog"),
-])
-def test_basepath(uri: str, expected_basepath: Optional[str]) -> None:
-    assert expected_basepath == auto.base_path_from_siteuri(uri)