Skip to content

Commit 9a1d5e7

Browse files
authored
Merge pull request #171 from thawn/copilot/fix-pdf-creation-error
Fix PDF generation in Docker containers by adding --no-sandbox flag
2 parents 66ab67d + 42b532b commit 9a1d5e7

File tree

2 files changed

+219
-1
lines changed

2 files changed

+219
-1
lines changed

src/ttmp32gme/print_handler.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,30 @@
1919
logger = logging.getLogger(__name__)
2020

2121

22+
def is_running_in_container() -> bool:
23+
"""Check if the application is running inside a container (Docker/Podman).
24+
25+
Returns:
26+
True if running in a container, False otherwise
27+
"""
28+
# Check for .dockerenv file (Docker)
29+
if Path("/.dockerenv").exists():
30+
return True
31+
32+
# Check /proc/1/cgroup for container runtime indicators
33+
try:
34+
with open("/proc/1/cgroup", "r") as f:
35+
content = f.read()
36+
# Look for docker, containerd, or other container runtimes
37+
if "docker" in content or "containerd" in content or "lxc" in content:
38+
return True
39+
except (FileNotFoundError, PermissionError):
40+
# /proc/1/cgroup doesn't exist or can't be read (not Linux or no permissions)
41+
pass
42+
43+
return False
44+
45+
2246
def format_tracks(
2347
album: Dict[str, Any], oid_map: Dict[str, Dict[str, int]], db_handler: DBHandler
2448
) -> str:
@@ -296,6 +320,14 @@ def create_pdf(
296320
f"http://localhost:{port}/pdf",
297321
]
298322

323+
# Add --no-sandbox flag when running in containers (Docker/Podman)
324+
# This is necessary because containers typically don't have the required
325+
# Linux capabilities for Chromium's sandbox to work
326+
if is_running_in_container():
327+
# Insert after chromium path (first element) but before other args
328+
args.insert(1, "--no-sandbox")
329+
logger.debug("Running in container, adding --no-sandbox flag to Chromium")
330+
299331
logger.info(f"Creating PDF with {found_name}: {' '.join(args)}")
300332

301333
try:

tests/unit/test_print_handler.py

Lines changed: 187 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import sqlite3
44
import tempfile
55
from pathlib import Path
6-
from unittest.mock import MagicMock, Mock, patch
6+
from unittest.mock import MagicMock, Mock, mock_open, patch
77

88
import pytest
99

@@ -17,6 +17,7 @@
1717
format_print_button,
1818
format_track_control,
1919
format_tracks,
20+
is_running_in_container,
2021
)
2122

2223

@@ -407,6 +408,96 @@ def test_create_pdf_tries_multiple_names(
407408
assert result == pdf_file
408409
assert mock_popen.called
409410

411+
@patch("ttmp32gme.print_handler.is_running_in_container")
412+
@patch("ttmp32gme.print_handler.time.sleep")
413+
@patch("ttmp32gme.print_handler.get_executable_path")
414+
@patch("ttmp32gme.print_handler.subprocess.Popen")
415+
@patch("ttmp32gme.print_handler.tempfile.mkstemp")
416+
@patch("ttmp32gme.print_handler.os.close")
417+
def test_create_pdf_in_container_adds_no_sandbox(
418+
self,
419+
mock_os_close,
420+
mock_mkstemp,
421+
mock_popen,
422+
mock_get_exec,
423+
mock_sleep,
424+
mock_is_container,
425+
):
426+
"""Test PDF creation adds --no-sandbox flag when running in container."""
427+
chromium_path = "/usr/bin/chromium"
428+
mock_get_exec.return_value = chromium_path
429+
mock_is_container.return_value = True # Simulate running in container
430+
431+
# Mock the process - poll() returns None (still running)
432+
mock_process = MagicMock()
433+
mock_process.poll.return_value = None
434+
mock_process.stderr.read.return_value = "" # No errors in stderr
435+
mock_popen.return_value = mock_process
436+
437+
with tempfile.TemporaryDirectory() as tmpdir:
438+
# Mock tempfile.mkstemp to return a path in our temp directory
439+
pdf_file = Path(tmpdir) / "ttmp32gme_print_test.pdf"
440+
mock_mkstemp.return_value = (999, str(pdf_file))
441+
442+
# Create the PDF file so the function succeeds
443+
pdf_file.write_text("fake pdf content")
444+
445+
result = create_pdf(10020)
446+
447+
assert result is not None
448+
assert result == pdf_file
449+
assert mock_popen.called
450+
451+
# Verify --no-sandbox flag is present in arguments
452+
call_args = mock_popen.call_args[0][0]
453+
assert "--no-sandbox" in call_args
454+
# Verify it's positioned right after chromium path
455+
chromium_index = call_args.index(chromium_path)
456+
assert call_args[chromium_index + 1] == "--no-sandbox"
457+
458+
@patch("ttmp32gme.print_handler.is_running_in_container")
459+
@patch("ttmp32gme.print_handler.time.sleep")
460+
@patch("ttmp32gme.print_handler.get_executable_path")
461+
@patch("ttmp32gme.print_handler.subprocess.Popen")
462+
@patch("ttmp32gme.print_handler.tempfile.mkstemp")
463+
@patch("ttmp32gme.print_handler.os.close")
464+
def test_create_pdf_not_in_container_no_sandbox_flag(
465+
self,
466+
mock_os_close,
467+
mock_mkstemp,
468+
mock_popen,
469+
mock_get_exec,
470+
mock_sleep,
471+
mock_is_container,
472+
):
473+
"""Test PDF creation does NOT add --no-sandbox flag when not in container."""
474+
mock_get_exec.return_value = "/usr/bin/chromium"
475+
mock_is_container.return_value = False # Simulate NOT running in container
476+
477+
# Mock the process - poll() returns None (still running)
478+
mock_process = MagicMock()
479+
mock_process.poll.return_value = None
480+
mock_process.stderr.read.return_value = "" # No errors in stderr
481+
mock_popen.return_value = mock_process
482+
483+
with tempfile.TemporaryDirectory() as tmpdir:
484+
# Mock tempfile.mkstemp to return a path in our temp directory
485+
pdf_file = Path(tmpdir) / "ttmp32gme_print_test.pdf"
486+
mock_mkstemp.return_value = (999, str(pdf_file))
487+
488+
# Create the PDF file so the function succeeds
489+
pdf_file.write_text("fake pdf content")
490+
491+
result = create_pdf(10020)
492+
493+
assert result is not None
494+
assert result == pdf_file
495+
assert mock_popen.called
496+
497+
# Verify --no-sandbox flag is NOT present in arguments
498+
call_args = mock_popen.call_args[0][0]
499+
assert "--no-sandbox" not in call_args
500+
410501

411502
class TestFormatPrintButton:
412503
"""Test format_print_button function."""
@@ -444,3 +535,98 @@ def test_format_print_button_linux_with_chromium(self, mock_get_exec, mock_syste
444535

445536
assert "Print This Page</button>" in result
446537
assert "Save as PDF</button>" in result
538+
539+
540+
class TestIsRunningInContainer:
541+
"""Test is_running_in_container function."""
542+
543+
@patch("ttmp32gme.print_handler.Path")
544+
def test_dockerenv_exists(self, mock_path):
545+
"""Test container detection when .dockerenv file exists."""
546+
mock_dockerenv = Mock()
547+
mock_dockerenv.exists.return_value = True
548+
mock_path.return_value = mock_dockerenv
549+
550+
result = is_running_in_container()
551+
552+
assert result is True
553+
mock_path.assert_called_once_with("/.dockerenv")
554+
555+
@patch("builtins.open", new_callable=mock_open, read_data="12:pids:/docker/abc123")
556+
@patch("ttmp32gme.print_handler.Path")
557+
def test_docker_in_cgroup(self, mock_path, mock_file):
558+
"""Test container detection from /proc/1/cgroup with docker."""
559+
mock_dockerenv = Mock()
560+
mock_dockerenv.exists.return_value = False
561+
mock_path.return_value = mock_dockerenv
562+
563+
result = is_running_in_container()
564+
565+
assert result is True
566+
mock_file.assert_called_once_with("/proc/1/cgroup", "r")
567+
568+
@patch(
569+
"builtins.open",
570+
new_callable=mock_open,
571+
read_data="12:pids:/system.slice/containerd.service",
572+
)
573+
@patch("ttmp32gme.print_handler.Path")
574+
def test_containerd_in_cgroup(self, mock_path, mock_file):
575+
"""Test container detection from /proc/1/cgroup with containerd."""
576+
mock_dockerenv = Mock()
577+
mock_dockerenv.exists.return_value = False
578+
mock_path.return_value = mock_dockerenv
579+
580+
result = is_running_in_container()
581+
582+
assert result is True
583+
584+
@patch(
585+
"builtins.open", new_callable=mock_open, read_data="12:pids:/lxc/container123"
586+
)
587+
@patch("ttmp32gme.print_handler.Path")
588+
def test_lxc_in_cgroup(self, mock_path, mock_file):
589+
"""Test container detection from /proc/1/cgroup with lxc."""
590+
mock_dockerenv = Mock()
591+
mock_dockerenv.exists.return_value = False
592+
mock_path.return_value = mock_dockerenv
593+
594+
result = is_running_in_container()
595+
596+
assert result is True
597+
598+
@patch("builtins.open", new_callable=mock_open, read_data="12:pids:/init.scope")
599+
@patch("ttmp32gme.print_handler.Path")
600+
def test_not_in_container(self, mock_path, mock_file):
601+
"""Test container detection returns False on normal system."""
602+
mock_dockerenv = Mock()
603+
mock_dockerenv.exists.return_value = False
604+
mock_path.return_value = mock_dockerenv
605+
606+
result = is_running_in_container()
607+
608+
assert result is False
609+
610+
@patch("builtins.open", side_effect=FileNotFoundError())
611+
@patch("ttmp32gme.print_handler.Path")
612+
def test_cgroup_not_found(self, mock_path, mock_file):
613+
"""Test container detection when /proc/1/cgroup doesn't exist."""
614+
mock_dockerenv = Mock()
615+
mock_dockerenv.exists.return_value = False
616+
mock_path.return_value = mock_dockerenv
617+
618+
result = is_running_in_container()
619+
620+
assert result is False
621+
622+
@patch("builtins.open", side_effect=PermissionError())
623+
@patch("ttmp32gme.print_handler.Path")
624+
def test_cgroup_permission_error(self, mock_path, mock_file):
625+
"""Test container detection when /proc/1/cgroup can't be read."""
626+
mock_dockerenv = Mock()
627+
mock_dockerenv.exists.return_value = False
628+
mock_path.return_value = mock_dockerenv
629+
630+
result = is_running_in_container()
631+
632+
assert result is False

0 commit comments

Comments
 (0)