fix: bypass recordings, welib wrong-md5 links, footer build sha (#1364) (#1373)

Debug screen recordings never started. Every bypass logged "Capturearea
1540x1050 at position 0.0 outside the screen size 1440x1880".We ask
ffmpeg for the fingerprint screen size plus margin, the size wealso pass
SeleniumBase as xvfb_metrics. SeleniumBase builds thatdisplay with
use_xauth=True, the image ships no xauth binary, so itfalls back to a
fixed 1440x1880 Xvfb and the requested size neverexists. Drop
-video_size so x11grab records the whole screen, whateversize it turned
out to be.

welib could hand back a link for a different book. Welib
answers/md5/<md5> with a search for that md5; when it does not have the
file,the resolver took the first "Download" on the results page
(md5a2c1dc0c... resolved to auto_download/9c8cf85d...). On an
/md5/<md5>page a GET/Download link is now only taken when its href names
thatmd5; otherwise the source is reported as not having the file.
Alsoremoves _get_download_urls_from_welib and _is_source_enabled:
themd5-template branch in _get_urls_for_source always handles welib
first,so that resolver could never run.

The footer showed the build date instead of the commit. CI
stampsBUILD_VERSION as <yyyy-mm-dd>-<sha> (pr-<sha> for PR images) and
thefooter kept its first seven characters, so dev images read
"Shelfmarkmain (2026-09)". Take the trailing commit sha instead: "main
(1a5b37d)".The full BUILD_VERSION stays in the hover title
This commit is contained in:
CaliBrain
2026-09-21 00:15:41 -04:00
committed by GitHub
parent d978896142
commit e1c3f057ab
7 changed files with 169 additions and 86 deletions
+5 -6
View File
@@ -1491,17 +1491,16 @@ def _start_ffmpeg_recording(display: str) -> None:
timestamp = datetime.now(UTC).strftime("%y%m%d-%H%M%S")
output_file = RECORDING_DIR / f"screen_recording_{timestamp}.mp4"
screen_width, screen_height = get_screen_size()
display_width = screen_width + 100
display_height = screen_height + 150
# No -video_size: x11grab then captures the whole screen, whatever size it is. The
# size we ask SeleniumBase for (xvfb_metrics) is not the size we get. It builds that
# display with use_xauth=True, the image ships no xauth binary, so it falls back to a
# fixed 1440x1880 Xvfb. Asking ffmpeg for the fingerprint size plus margin then asked
# for an area larger than the screen, and every recording died at startup (#1364).
ffmpeg_cmd = [
"ffmpeg",
"-y",
"-f",
"x11grab",
"-video_size",
f"{display_width}x{display_height}",
"-i",
display,
"-c:v",
@@ -131,19 +131,34 @@ def _find_first_anchor_with_text(
text: str,
*,
contains: bool = False,
href_contains: str | None = None,
) -> Tag | None:
"""Find the first anchor whose text matches the requested value."""
"""Find the first anchor whose text matches the requested value.
With ``href_contains``, anchors whose href does not include it are skipped.
"""
expected = text.lower()
for anchor in container.find_all("a", href=True):
anchor_text = anchor.get_text(strip=True)
if not anchor_text:
continue
if href_contains and href_contains.lower() not in (get_attr(anchor, "href") or "").lower():
continue
candidate = anchor_text.lower()
if candidate == expected or (contains and expected in candidate):
return anchor
return None
_MD5_PAGE_PATH = re.compile(r"/md5/([0-9a-f]{32})(?:/|$)", re.IGNORECASE)
def _md5_from_page_url(url: str) -> str | None:
"""Return the md5 an ``/md5/<md5>`` page URL is for, or None for any other URL."""
match = _MD5_PAGE_PATH.search(urlparse(url).path)
return match.group(1).lower() if match else None
def _find_text_node(container: BeautifulSoup | Tag, needle: str) -> NavigableString | None:
"""Find a text node containing a case-insensitive substring."""
expected = needle.lower()
@@ -417,17 +432,6 @@ def _get_source_priority() -> list[SourcePriorityEntry]:
return fast_sources + slow_sources
def _is_source_enabled(source_id: str) -> bool:
"""Check if a source is enabled in the priority config.
Returns False for unknown sources.
"""
for item in _get_source_priority():
if item["id"] == source_id:
return item.get("enabled", True)
return False
def get_unavailable_reason() -> str | None:
"""Return a user-facing reason when Direct Download cannot be used."""
from shelfmark.core import mirrors
@@ -1314,17 +1318,6 @@ def _get_urls_for_source(
urls.append(url)
return urls
# Welib - fetch page and parse for slow_download links
if source_id == "welib":
if status_callback:
status_callback("resolving", "Fetching welib sources")
return _get_download_urls_from_welib(
book_info.id,
selector=selector,
cancel_flag=cancel_flag,
status_callback=status_callback,
)
# AA page sources - fetch AA page if not already done
if source_id in _AA_PAGE_SOURCES:
if not urls_by_source:
@@ -1404,53 +1397,6 @@ def _try_download_url(
return download_url
def _get_download_urls_from_welib(
book_id: str,
selector: network.AAMirrorSelector | None = None,
cancel_flag: Event | None = None,
status_callback: Callable[[str, str | None], None] | None = None,
) -> list[str]:
"""Get download URLs from welib.org (bypasser required)."""
from shelfmark.core import mirrors
if not _is_source_enabled("welib"):
return []
template = mirrors.get_welib_url_template()
if not template:
return []
url = template.format(md5=book_id)
logger.info("Fetching welib download URLs for %s", book_id)
try:
html = downloader.html_get_page(
url,
use_bypasser=True,
selector=selector or network.AAMirrorSelector(),
cancel_flag=cancel_flag,
status_callback=status_callback,
)
except (
SearchUnavailableError,
requests.exceptions.RequestException,
RuntimeError,
ValueError,
TypeError,
AttributeError,
) as exc:
logger.error_trace(f"Welib fetch failed for {book_id}: {exc}")
return []
if not html:
logger.warning("Welib page empty for %s", book_id)
return []
soup = BeautifulSoup(html_response_text(html), "html.parser")
links = [
downloader.get_absolute_url(url, href)
for a in soup.find_all("a", href=True)
if (href := get_attr(a, "href")) and "/slow_download/" in href
]
return list(dict.fromkeys(links)) # Dedupe while preserving order
def _extract_libgen_download_url(link: str, cancel_flag: Event | None = None) -> str:
"""Extract download URL from Libgen ads.php page using direct HTTP."""
if cancel_flag and cancel_flag.is_set():
@@ -1680,14 +1626,19 @@ def _get_download_url(
)
else:
get_btn = _find_first_anchor_with_text(soup, "GET") or _find_first_anchor_with_text(
soup, "Download"
)
# Welib answers /md5/<md5> with a search for that md5. When it does not have the
# file, the first "Download" on that page belongs to whichever book ranked first,
# so a link is only taken when it names the md5 we asked for (#1364).
md5 = _md5_from_page_url(link)
get_btn = _find_first_anchor_with_text(
soup, "GET", href_contains=md5
) or _find_first_anchor_with_text(soup, "Download", href_contains=md5)
if get_btn:
url = get_attr(get_btn, "href") or ""
elif md5:
logger.info("No download link for md5 %s on %s", md5, link)
else:
logger.warning("Unknown source type, couldn't find download link: %s", link)
url = ""
return downloader.get_absolute_url(link, url)
+4 -6
View File
@@ -1,3 +1,5 @@
import { shortBuildId } from '../utils/buildVersion';
interface FooterProps {
buildVersion?: string;
releaseVersion?: string;
@@ -8,11 +10,7 @@ export const Footer = ({ buildVersion, releaseVersion, debug }: FooterProps) =>
// Determine version display - show "dev" if no version is set
const versionDisplay = releaseVersion && releaseVersion !== 'N/A' ? releaseVersion : 'dev';
// Truncate long build versions (e.g., git hashes) to 7 chars
let truncatedBuild: string | null = null;
if (buildVersion && buildVersion !== 'N/A') {
truncatedBuild = buildVersion.length > 7 ? buildVersion.slice(0, 7) : buildVersion;
}
const buildId = shortBuildId(buildVersion);
return (
<footer
@@ -35,7 +33,7 @@ export const Footer = ({ buildVersion, releaseVersion, debug }: FooterProps) =>
title={buildVersion && buildVersion !== 'N/A' ? `Build: ${buildVersion}` : undefined}
>
{versionDisplay}
{truncatedBuild && ` (${truncatedBuild})`}
{buildId && ` (${buildId})`}
</span>
{debug && (
<span
@@ -0,0 +1,30 @@
import { describe, it, expect } from 'vitest';
import { shortBuildId } from '../utils/buildVersion';
const SHA = '1a5b37d0c2e4f6a8b0c2d4e6f8a0b2c4d6e8f0a2';
describe('buildVersion.shortBuildId', () => {
it('shows the commit of a date-stamped image, not the date', () => {
expect(shortBuildId(`2026-09-20-${SHA}`)).toBe('1a5b37d');
});
it('shows the commit of a PR image', () => {
expect(shortBuildId(`pr-${SHA}`)).toBe('1a5b37d');
});
it('shortens a bare commit sha', () => {
expect(shortBuildId(SHA)).toBe('1a5b37d');
});
it('keeps any other stamp to its first seven characters', () => {
expect(shortBuildId('local-build')).toBe('local-b');
expect(shortBuildId('abc')).toBe('abc');
});
it('returns null for an unstamped build', () => {
expect(shortBuildId(undefined)).toBe(null);
expect(shortBuildId('')).toBe(null);
expect(shortBuildId('N/A')).toBe(null);
});
});
+15
View File
@@ -0,0 +1,15 @@
const COMMIT_SHA = /[0-9a-f]{40}$/i;
/**
* The short commit id a build was made from, or null when the build is unstamped.
*
* CI stamps BUILD_VERSION as `<yyyy-mm-dd>-<sha>` for published images and `pr-<sha>` for
* PR images, so its first seven characters are the date ("2026-09"), not the commit.
*/
export const shortBuildId = (buildVersion?: string): string | null => {
if (!buildVersion || buildVersion === 'N/A') {
return null;
}
const sha = COMMIT_SHA.exec(buildVersion)?.[0];
return (sha ?? buildVersion).slice(0, 7);
};
@@ -60,6 +60,31 @@ def test_ffmpeg_errors_are_captured_to_a_file_beside_the_recording(monkeypatch,
assert error_log.name.startswith("screen_recording_")
def test_capture_is_not_pinned_to_the_fingerprint_size(monkeypatch, tmp_path):
"""Issue #1364: every recording died on "Capture area ... outside the screen size".
The capture size was the fingerprint size plus margin, but the Xvfb is not built at
that size (SeleniumBase falls back to a fixed 1440x1880 screen when xauth is missing),
so any fingerprint wider than 1340px asked for more than the screen had. Left unset,
x11grab records the whole screen, whatever it turned out to be.
"""
monkeypatch.setattr(ib, "RECORDING_DIR", tmp_path)
captured: dict[str, object] = {}
def fake_popen(cmd, **kwargs):
captured["cmd"] = cmd
return _Proc(None)
monkeypatch.setattr(ib.subprocess, "Popen", fake_popen)
ib._start_ffmpeg_recording(display=":99")
cmd = captured["cmd"]
assert "-video_size" not in cmd
assert cmd[cmd.index("-f") + 1] == "x11grab"
assert cmd[cmd.index("-i") + 1] == ":99"
def test_an_early_exit_is_reported_with_ffmpegs_own_reason(monkeypatch, tmp_path, caplog):
reason = "[x11grab @ 0x1] Cannot open display :99, error 1."
error_log = tmp_path / "screen_recording_x.ffmpeg.log"
@@ -0,0 +1,65 @@
"""Welib must only hand back a download link for the md5 that was asked for.
Issue #1364: welib answers /md5/<md5> with a search for that md5. It did not have the
file, and the resolver took the first "Download" on the results page - a link for another
book (auto_download/9c8cf85d... for md5 a2c1dc0c...). That download happened to fail, but
nothing stopped it from succeeding with the wrong file.
"""
from unittest.mock import patch
import pytest
from shelfmark.release_sources.direct_download import annas_archive
WANTED = "a2c1dc0c1fb3422b5cf7a7473bc56010"
OTHER = "9c8cf85d9d805ac89b8d3c581598a615"
WELIB_PAGE = f"https://welib.org/md5/{WANTED}"
def _resolve(link: str, page_html: str) -> str:
with (
patch.object(annas_archive.downloader, "html_get_page", return_value=page_html),
patch.object(
annas_archive.network, "get_aa_base_url", return_value="https://annas-archive.gl"
),
patch.object(annas_archive, "_is_configured_zlib_link", return_value=False),
):
return annas_archive._get_download_url(link, "Crown Me Dead", selector=object()) # type: ignore[arg-type]
def test_a_search_page_for_another_book_resolves_to_nothing():
page = f'<a href="/auto_download/{OTHER}/0/0">Download</a>'
assert _resolve(WELIB_PAGE, page) == ""
def test_the_link_for_the_requested_md5_is_picked_over_an_earlier_one():
page = (
f'<a href="/auto_download/{OTHER}/0/0">Download</a>'
f'<a href="/auto_download/{WANTED.upper()}/0/0">Download</a>'
)
assert _resolve(WELIB_PAGE, page) == f"https://welib.org/auto_download/{WANTED.upper()}/0/0"
@pytest.mark.parametrize(
("link", "expected"),
[
(f"https://welib.org/md5/{WANTED}", WANTED),
(f"https://welib.org/md5/{WANTED.upper()}/", WANTED),
(f"https://welib.org/md5/{WANTED}0", None),
(f"https://welib.org/search?q=md5:{WANTED}", None),
("https://example.org/book/42", None),
],
)
def test_md5_is_read_only_from_an_md5_page_path(link, expected):
assert annas_archive._md5_from_page_url(link) == expected
def test_a_page_that_is_not_md5_addressed_keeps_taking_the_first_download():
page = f'<a href="/files/{OTHER}.epub">Download</a>'
assert (
_resolve("https://example.org/book/42", page) == f"https://example.org/files/{OTHER}.epub"
)