Fix 404 on issue downloads and make the BookOrbit link admin-only

Download hrefs were built with form encoding, which turns the spaces in
"The Daily EPUB - <date>.epub" into "+"; in a URL path that is a literal
plus, so /files/epub/<name> never matched the file. They now use the
same path-segment percent-encoder the OPDS feed already uses, and the
download-menu test follows the rendered link and expects 200.

The Read in BookOrbit button renders only for admins, and the
/issues/{date}/read redirect answers 403 to other signed-in users.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QVPagF6jfDv78CC5Jv2wp4
This commit is contained in:
2026-09-07 02:39:06 +00:00
co-authored by Claude Fable 5.1
parent 78d9ef5959
commit e69940fac8
4 changed files with 94 additions and 14 deletions
+1 -1
View File
@@ -460,7 +460,7 @@ fn xml_escape(s: &str) -> String {
}
/// Percent-encode one URL path segment (filenames contain spaces and parens).
fn percent_encode(s: &str) -> String {
pub fn percent_encode(s: &str) -> String {
let mut out = String::with_capacity(s.len());
for byte in s.as_bytes() {
match byte {
+90 -10
View File
@@ -643,7 +643,8 @@ fn download(
let name = path.file_name()?.to_str()?;
Some(Download {
label: label.to_string(),
href: format!("/files/{kind}/{}", crate::web::encode_component(name)),
// A path segment, not a query string: spaces must be %20, never `+`.
href: format!("/files/{kind}/{}", crate::publish::percent_encode(name)),
size_bytes: metadata.len(),
size: format_file_size(metadata.len()),
})
@@ -1224,13 +1225,16 @@ pub async fn read(
Path(date): Path<Date>,
Query(query): Query<ReadQuery>,
) -> Result<Response, WebError> {
let _viewer = auth
let viewer = auth
.user()
.await
.map(Viewer::from)
.ok_or_else(|| WebError::Unauthenticated {
next: format!("/issues/{date}/read"),
})?;
if viewer.role != crate::web::users::Role::Admin {
return Err(WebError::Forbidden);
}
let config = state.config();
if !config.bookorbit.is_active() {
return Err(WebError::NotFound);
@@ -2253,13 +2257,21 @@ mod tests {
["X4 EPUB", "XTC"]
);
let primary_href = downloads.primary.href.clone();
assert!(
primary_href.starts_with("/files/epub/The%20Daily%20EPUB%20-%20"),
"{primary_href}"
);
assert!(!primary_href.contains('+'), "{primary_href}");
let app = crate::server::router(crate::server::AppState::new(db, config, None));
let cookie = login_cookie(&app, "reader", "correct horse battery").await;
let issue = app
.clone()
.oneshot(
Request::builder()
.uri(format!("/issues/{}", source.meta.date))
.header(header::COOKIE, cookie)
.header(header::COOKIE, &cookie)
.body(Body::empty())
.unwrap(),
)
@@ -2268,6 +2280,18 @@ mod tests {
let issue = response_text(issue).await;
assert!(issue.contains(">Download EPUB</a>"));
assert!(issue.contains("Choose download format"));
// The link the page renders must resolve to the file it names.
let file = app
.oneshot(
Request::builder()
.uri(primary_href)
.header(header::COOKIE, cookie)
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
assert_eq!(file.status(), StatusCode::OK);
assert!(issue.contains("Standard EPUB"));
assert!(issue.contains("X4 EPUB"));
assert!(issue.contains("XTC"));
@@ -2279,7 +2303,7 @@ mod tests {
#[tokio::test]
async fn bookorbit_read_is_hidden_and_not_found_when_inactive() {
let (_dir, db, source) = seeded_issue(false).await;
crate::web::users::add(&db, "reader", "correct horse battery", false)
crate::web::users::add(&db, "admin", "correct horse battery", true)
.await
.unwrap();
let app = crate::server::router(crate::server::AppState::new(
@@ -2287,7 +2311,7 @@ mod tests {
crate::config::Config::default(),
None,
));
let cookie = login_cookie(&app, "reader", "correct horse battery").await;
let cookie = login_cookie(&app, "admin", "correct horse battery").await;
let issue = app
.clone()
@@ -2320,7 +2344,7 @@ mod tests {
#[tokio::test]
async fn bookorbit_read_button_and_cached_redirect_use_the_public_url() {
let (dir, db, source) = seeded_issue(true).await;
crate::web::users::add(&db, "reader", "correct horse battery", false)
crate::web::users::add(&db, "admin", "correct horse battery", true)
.await
.unwrap();
let epub_dir = dir.path().join("epubs");
@@ -2349,7 +2373,7 @@ mod tests {
config.clone(),
None,
));
let cookie = login_cookie(&app, "reader", "correct horse battery").await;
let cookie = login_cookie(&app, "admin", "correct horse battery").await;
let issue = app
.clone()
@@ -2384,7 +2408,7 @@ mod tests {
config.bookorbit.public_url = "https://example.test/".into();
let app = crate::server::router(crate::server::AppState::new(db, config, None));
let cookie = login_cookie(&app, "reader", "correct horse battery").await;
let cookie = login_cookie(&app, "admin", "correct horse battery").await;
let read = app
.oneshot(
Request::builder()
@@ -2402,6 +2426,62 @@ mod tests {
);
}
#[tokio::test]
async fn bookorbit_read_is_admin_only() {
let (dir, db, source) = seeded_issue(true).await;
crate::web::users::add(&db, "reader", "correct horse battery", false)
.await
.unwrap();
let epub_dir = dir.path().join("epubs");
std::fs::create_dir(&epub_dir).unwrap();
std::fs::write(
epub_dir.join(crate::publish::issue_filename(
source.meta.date,
Edition::Standard,
"epub",
)),
b"standard epub",
)
.unwrap();
db.set_bookorbit_ids(source.meta.date, Some((12, 34)))
.await
.unwrap();
let mut config = crate::config::Config::default();
config.publish.epub_dir = epub_dir;
config.bookorbit.enabled = true;
config.bookorbit.opds_user = Some("reader".into());
config.bookorbit.opds_pass = Some("secret".into());
let app = crate::server::router(crate::server::AppState::new(db, config, None));
let cookie = login_cookie(&app, "reader", "correct horse battery").await;
let issue = app
.clone()
.oneshot(
Request::builder()
.uri(format!("/issues/{}", source.meta.date))
.header(header::COOKIE, &cookie)
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
let issue = response_text(issue).await;
assert!(!issue.contains("Read in BookOrbit"));
assert!(issue.contains(">Download EPUB</a>"));
let read = app
.oneshot(
Request::builder()
.uri(format!("/issues/{}/read", source.meta.date))
.header(header::COOKIE, cookie)
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
assert_eq!(read.status(), StatusCode::FORBIDDEN);
}
#[tokio::test]
async fn bookorbit_read_button_is_hidden_without_the_standard_epub() {
let (dir, db, source) = seeded_issue(true).await;
@@ -2434,7 +2514,7 @@ mod tests {
#[tokio::test]
async fn bookorbit_read_reports_upstream_connection_errors() {
let (_dir, db, source) = seeded_issue(true).await;
crate::web::users::add(&db, "reader", "correct horse battery", false)
crate::web::users::add(&db, "admin", "correct horse battery", true)
.await
.unwrap();
let mut config = crate::config::Config::default();
@@ -2443,7 +2523,7 @@ mod tests {
config.bookorbit.opds_pass = Some("secret".into());
config.bookorbit.api_url = "http://127.0.0.1:9".into();
let app = crate::server::router(crate::server::AppState::new(db, config, None));
let cookie = login_cookie(&app, "reader", "correct horse battery").await;
let cookie = login_cookie(&app, "admin", "correct horse battery").await;
let read = app
.oneshot(
+1 -1
View File
@@ -3,7 +3,7 @@
<article class="reader-page mx-auto mt-10 w-full max-w-[68ch] px-4 sm:px-6 lg:mt-12 lg:px-0">
<header class="mb-8 text-center"><p class="font-sans text-[0.72rem] uppercase tracking-[0.12em] text-muted">{{ display_date }} · No. {{ issue_number }}</p><p class="mt-2 font-sans text-sm text-muted">{{ stats_line }}</p></header>
<section aria-labelledby="brief-heading" data-toc-entry="/issues/{{ date }}"><h1 id="brief-heading" class="reader-section-heading">The Brief</h1><div class="editorial prose-body mt-5 [&>p:first-child]:first-letter:float-left [&>p:first-child]:first-letter:mr-2 [&>p:first-child]:first-letter:mt-1 [&>p:first-child]:first-letter:font-serif [&>p:first-child]:first-letter:text-[4.6rem] [&>p:first-child]:first-letter:font-semibold [&>p:first-child]:first-letter:leading-[0.72]">{{ front_page_html|safe }}</div></section>
{% if read_href.is_some() || downloads.is_some() %}<div class="my-8 flex flex-wrap gap-2 border-y border-rule py-4">{% match read_href %}{% when Some with (href) %}<a class="btn" href="{{ href }}" target="_blank" rel="noopener">Read in BookOrbit</a>{% when None %}{% endmatch %}{% match downloads %}{% when Some with (downloads) %}{% if downloads.others.is_empty() %}<a class="btn" href="{{ downloads.primary.href }}">Download {{ downloads.primary.label }}</a>{% else %}<div class="relative inline-flex"><a class="btn rounded-r-none" href="{{ downloads.primary.href }}">Download {{ downloads.primary.label }}</a><details class="-ml-px"><summary class="btn list-none rounded-l-none px-2 [&::-webkit-details-marker]:hidden" aria-label="Choose download format"><span aria-hidden="true">▾</span></summary><div class="absolute left-0 top-full z-20 mt-1 min-w-52 border border-rule bg-paper p-1 shadow-[0_18px_32px_-26px_rgb(0_0_0_/_0.6)]"><a class="btn group w-full justify-between border-0" href="{{ downloads.primary.href }}"><span>{% if downloads.primary.label == "EPUB" %}Standard {% endif %}{{ downloads.primary.label }}</span><span class="ml-4 text-muted group-hover:text-paper">{{ downloads.primary.size }}</span></a>{% for download in downloads.others %}<a class="btn group w-full justify-between border-0" href="{{ download.href }}"><span>{{ download.label }}</span><span class="ml-4 text-muted group-hover:text-paper">{{ download.size }}</span></a>{% endfor %}</div></details></div>{% endif %}{% when None %}{% endmatch %}</div>{% endif %}
{% if (page.is_admin() && read_href.is_some()) || downloads.is_some() %}<div class="my-8 flex flex-wrap gap-2 border-y border-rule py-4">{% if page.is_admin() %}{% match read_href %}{% when Some with (href) %}<a class="btn" href="{{ href }}" target="_blank" rel="noopener">Read in BookOrbit</a>{% when None %}{% endmatch %}{% endif %}{% match downloads %}{% when Some with (downloads) %}{% if downloads.others.is_empty() %}<a class="btn" href="{{ downloads.primary.href }}">Download {{ downloads.primary.label }}</a>{% else %}<div class="relative inline-flex"><a class="btn rounded-r-none" href="{{ downloads.primary.href }}">Download {{ downloads.primary.label }}</a><details class="-ml-px"><summary class="btn list-none rounded-l-none px-2 [&::-webkit-details-marker]:hidden" aria-label="Choose download format"><span aria-hidden="true">▾</span></summary><div class="absolute left-0 top-full z-20 mt-1 min-w-52 border border-rule bg-paper p-1 shadow-[0_18px_32px_-26px_rgb(0_0_0_/_0.6)]"><a class="btn group w-full justify-between border-0" href="{{ downloads.primary.href }}"><span>{% if downloads.primary.label == "EPUB" %}Standard {% endif %}{{ downloads.primary.label }}</span><span class="ml-4 text-muted group-hover:text-paper">{{ downloads.primary.size }}</span></a>{% for download in downloads.others %}<a class="btn group w-full justify-between border-0" href="{{ download.href }}"><span>{{ download.label }}</span><span class="ml-4 text-muted group-hover:text-paper">{{ download.size }}</span></a>{% endfor %}</div></details></div>{% endif %}{% when None %}{% endmatch %}</div>{% endif %}
<section class="mt-12" aria-labelledby="index-heading"><h2 id="index-heading" class="text-center text-3xl font-semibold leading-[1.1] tracking-[-0.01em]">In This Issue</h2>
{% for section in sections %}<section class="mt-14"><h3 class="reader-section-heading">{{ section.name }}</h3><ul class="m-0 list-none p-0">{% for entry in section.entries %}<li class="border-b border-rule py-6" data-toc-entry="{{ entry.href }}"><h4 class="font-serif font-semibold leading-[1.1] tracking-[-0.01em] {% if entry.is_lead %}text-3xl{% else %}text-2xl{% endif %}"><a class="text-ink no-underline hover:text-accent" href="{{ entry.href }}">{{ entry.title }}</a></h4><p class="mt-2 font-sans text-sm text-muted">{{ entry.source }} · {{ entry.reading_minutes }} min read{% if page.is_admin() %} · <a class="text-muted hover:text-accent" href="{{ entry.dashboard_href }}">dashboard</a>{% endif %}</p>{% if !entry.summary.is_empty() %}<p class="index-summary mt-4">{{ entry.summary }}</p>{% endif %}{% match entry.why %}{% when Some with (why) %}<p class="mt-4 border-l-2 border-accent pl-3 italic text-ink-2">Why it's here: {{ why }}</p>{% when None %}{% endmatch %}{% match entry.rating %}{% when Some with (widget) %}{% include "_rating_widget.html" %}{% when None %}{% endmatch %}</li>{% endfor %}</ul></section>{% endfor %}
</section>