From 85f0d7d92f737fc57622e781a7b1b229bb45b2e1 Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Fri, 18 Sep 2026 01:49:50 +0200 Subject: [PATCH] put: paginate the Launchpad published-sources lookup The superseded check read only the first getPublishedSources page (Launchpad defaults to 75 entries per page), so a source with a long publication history could hide its true maximum published version and let a superseded upload through, only to be rejected by the queue hours later. Follow next_collection_link (ws.size=100, hard cap of 20 pages beyond which the check errors rather than risk a false 'not superseded'). --- src/launchpad.rs | 304 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 261 insertions(+), 43 deletions(-) diff --git a/src/launchpad.rs b/src/launchpad.rs index 126583d..07a9646 100644 --- a/src/launchpad.rs +++ b/src/launchpad.rs @@ -31,6 +31,20 @@ const LP_USER_KEY: &str = "lp.user"; /// Base URL of the Launchpad REST API const API_BASE: &str = "https://api.launchpad.net/1.0"; +/// Page size (`ws.size`) asked from Launchpad collections. Launchpad +/// truncates collection answers at 75 entries by default and rejects +/// `ws.size` above 300 (both verified against the live API); 100 sits +/// comfortably under the cap while keeping multi-page walks rare. +const WS_PAGE_SIZE: u32 = 100; + +/// Hard cap on the pages followed while walking a `getPublishedSources` +/// collection: 20 pages x 100 entries = 2000 currently published entries +/// for one source name. The query only counts `Published` entries of live +/// series/pockets, so real histories are a handful of entries; a walk +/// reaching the cap means the API is misbehaving (an endless next-link +/// chain), not that the history is genuinely huge. +const MAX_COLLECTION_PAGES: u32 = 20; + /// The Launchpad username configured in git: the repository-local /// configuration wins over the global one, like git's own precedence. /// `None` when no git repository is found or the key is unset. @@ -178,20 +192,21 @@ fn percent_encode(value: &str) -> String { encoded } -/// URL of the `getPublishedSources` API call listing the currently -/// `Published` source packages named `source_name` in the PPA `user/name`: -/// `exact_match` avoids Launchpad's default case-insensitive substring -/// matching, which would return unrelated sources (`data` matching -/// `datatables`). +/// URL of the first page of the `getPublishedSources` API call listing the +/// currently `Published` source packages named `source_name` in the PPA +/// `user/name`: `exact_match` avoids Launchpad's default case-insensitive +/// substring matching, which would return unrelated sources (`data` matching +/// `datatables`). Further pages are reached through the answer's +/// `next_collection_link`, not by hand-building URLs. fn published_sources_url(user: &str, ppa: &str, source_name: &str) -> String { format!( - "{}?ws.op=getPublishedSources&source_name={}&exact_match=true&status=Published", + "{}?ws.op=getPublishedSources&source_name={}&exact_match=true&status=Published&ws.size={WS_PAGE_SIZE}", archive_url(user, ppa), percent_encode(source_name) ) } -/// One entry of a `getPublishedSources` answer: the subset of the source +/// One page of a `getPublishedSources` answer: the subset of the source /// package publishing history the superseded-upload check needs (the live /// answer carries many more fields, ignored by serde) #[derive(Debug, Deserialize)] @@ -200,42 +215,52 @@ struct PublishedSource { source_package_version: String, } -/// The `getPublishedSources` collection answer +/// One page of the `getPublishedSources` collection answer #[derive(Debug, Deserialize)] struct PublishedSources { - /// The currently published source packages matching the query + /// The currently published source packages matching the query, on this + /// page only #[serde(default)] entries: Vec, + /// URL of the next page, present only when the collection was + /// truncated (Launchpad answers carry it as a plain JSON string) + next_collection_link: Option, } -/// Every version of `source_name` currently `Published` in the PPA -/// `user/name` (same `user/ppa_name` format as `pkh put --ppa`), in API -/// order. Empty when the source was never published there — a 200 answer -/// with zero entries, the normal first-upload case. -pub async fn published_versions( - ppa: &str, - source_name: &str, -) -> Result, Box> { - let (user, name) = split_ppa(ppa)?; - let client = crate::distro_info::http_client(); +/// Parse one page of a `getPublishedSources` collection into the versions +/// it carries plus the link to the next page (`None` on the last one): the +/// pagination decision, factored out of the HTTP walk so it can be tested +/// without a server. +fn parse_collection_page(body: &str) -> Result<(Vec, Option), serde_json::Error> { + let sources: PublishedSources = serde_json::from_str(body)?; + Ok(( + sources + .entries + .into_iter() + .map(|entry| entry.source_package_version) + .collect(), + sources.next_collection_link, + )) +} +/// GET one page of a collection, mapping the API statuses to the same +/// errors as the other Launchpad calls (404 means the PPA does not exist). +/// Returns the response body for [`parse_collection_page`]. +async fn fetch_collection_page( + client: &reqwest::Client, + url: &str, + ppa: &str, +) -> Result> { let response = client - .get(published_sources_url(&user, &name, source_name)) + .get(url) .send() .await .map_err(|e| format!("cannot reach the Launchpad API: {e}"))?; match response.status() { - reqwest::StatusCode::OK => { - let sources: PublishedSources = response - .json() - .await - .map_err(|e| format!("cannot parse the Launchpad API response for '{ppa}': {e}"))?; - Ok(sources - .entries - .into_iter() - .map(|entry| entry.source_package_version) - .collect()) - } + reqwest::StatusCode::OK => response + .text() + .await + .map_err(|e| format!("cannot read the Launchpad API response for '{ppa}': {e}").into()), reqwest::StatusCode::NOT_FOUND => { Err(format!("PPA '{ppa}' does not exist: create it on launchpad.net first").into()) } @@ -243,6 +268,66 @@ pub async fn published_versions( } } +/// Walk a `getPublishedSources` collection page by page: fetch the first +/// page, then follow `next_collection_link` (the canonical Launchpad +/// pagination) until a page comes without one, accumulating the versions of +/// every page in order. +/// +/// Exceeding [`MAX_COLLECTION_PAGES`] errors rather than returning the +/// partial list: the result feeds `put`'s superseded-upload check, where a +/// silently truncated list is exactly the bug pagination fixes — a +/// superseded upload wrongly allowed through, to be rejected (or to +/// silently supersede) in Launchpad's queue hours later. Every other +/// failure mode of this check (network, HTTP status, parsing) aborts the +/// upload too, and `put` fails before anything is written, so erring costs +/// only a clear message. +async fn walk_collection( + client: &reqwest::Client, + first_url: &str, + ppa: &str, +) -> Result, Box> { + let mut versions = Vec::new(); + let mut url = first_url.to_string(); + for _page in 1..=MAX_COLLECTION_PAGES { + let body = fetch_collection_page(client, &url, ppa).await?; + let (mut page_versions, next) = parse_collection_page(&body) + .map_err(|e| format!("cannot parse the Launchpad API response for '{ppa}': {e}"))?; + versions.append(&mut page_versions); + match next { + Some(next) => url = next, + None => return Ok(versions), + } + } + + Err(format!( + "the Launchpad API keeps paginating the published sources of '{ppa}' \ + after {MAX_COLLECTION_PAGES} pages: cannot run the superseded check \ + on a partial list" + ) + .into()) +} + +/// Every version of `source_name` currently `Published` in the PPA +/// `user/name` (same `user/ppa_name` format as `pkh put --ppa`), in API +/// order. Empty when the source was never published there — a 200 answer +/// with zero entries, the normal first-upload case. Launchpad truncates +/// collections per page, so the walk follows the API's `next_collection_link` +/// until the collection is exhausted: a single page would miss the highest +/// version of a source published in many series/pockets over time, and the +/// superseded check would wrongly pass. +pub async fn published_versions( + ppa: &str, + source_name: &str, +) -> Result, Box> { + let (user, name) = split_ppa(ppa)?; + walk_collection( + crate::distro_info::http_client(), + &published_sources_url(&user, &name, source_name), + ppa, + ) + .await +} + /// PPA uploads only target Ubuntu series: fail before uploading when the /// changes' distribution is not a known series (typo) or a non-Ubuntu one — /// both are only rejected during queue processing otherwise @@ -372,16 +457,17 @@ mod tests { } /// The query matches the verified live `getPublishedSources` call, with - /// the source name percent-encoded + /// the source name percent-encoded and an explicit page size (the API + /// default of 75 entries would hide part of long publishing histories) #[test] fn published_sources_url_matches_launchpad_call() { assert_eq!( published_sources_url("vhaudiquet", "noctalia", "noctalia"), - "https://api.launchpad.net/1.0/~vhaudiquet/+archive/ubuntu/noctalia?ws.op=getPublishedSources&source_name=noctalia&exact_match=true&status=Published" + "https://api.launchpad.net/1.0/~vhaudiquet/+archive/ubuntu/noctalia?ws.op=getPublishedSources&source_name=noctalia&exact_match=true&status=Published&ws.size=100" ); assert_eq!( published_sources_url("vhaudiquet", "noctalia", "g++"), - "https://api.launchpad.net/1.0/~vhaudiquet/+archive/ubuntu/noctalia?ws.op=getPublishedSources&source_name=g%2B%2B&exact_match=true&status=Published" + "https://api.launchpad.net/1.0/~vhaudiquet/+archive/ubuntu/noctalia?ws.op=getPublishedSources&source_name=g%2B%2B&exact_match=true&status=Published&ws.size=100" ); } @@ -411,21 +497,153 @@ mod tests { ] }"#; - let sources: PublishedSources = serde_json::from_str(json).unwrap(); - let versions: Vec<&str> = sources - .entries - .iter() - .map(|entry| entry.source_package_version.as_str()) - .collect(); + let (versions, next) = parse_collection_page(json).unwrap(); assert_eq!(versions, vec!["5.1.0-1ubuntu2", "2:1.0-1"]); + // A page without a next link is the end of the collection + assert_eq!(next, None); } /// A 200 answer with zero entries is the normal "nothing published /// there" case, and must deserialize to an empty list #[test] fn published_sources_parses_empty_collection() { - let sources: PublishedSources = - serde_json::from_str(r#"{"start": 0, "total_size": 0, "entries": []}"#).unwrap(); - assert!(sources.entries.is_empty()); + let (versions, next) = + parse_collection_page(r#"{"start": 0, "total_size": 0, "entries": []}"#).unwrap(); + assert!(versions.is_empty()); + assert_eq!(next, None); + } + + /// A truncated page announces the next one through + /// `next_collection_link`, carried as a plain JSON string (shape + /// verified against the live API) + #[test] + fn parse_collection_page_reads_next_link() { + let json = r#"{ + "start": 0, + "total_size": 150, + "entries": [{"source_package_version": "1.0-1"}], + "next_collection_link": "https://api.launchpad.net/1.0/~u/+archive/ubuntu/p?ws.op=getPublishedSources&ws.size=100&memo=100&ws.start=100" + }"#; + + let (versions, next) = parse_collection_page(json).unwrap(); + assert_eq!(versions, vec!["1.0-1"]); + assert_eq!( + next.as_deref(), + Some( + "https://api.launchpad.net/1.0/~u/+archive/ubuntu/p?ws.op=getPublishedSources&ws.size=100&memo=100&ws.start=100" + ) + ); + } + + /// Serve canned byte responses on a local port, one per connection (the + /// last response repeats), and return the listener for URL building + /// + /// The canned responses must use 'Connection: close' so the client opens + /// a fresh connection (and receives a fresh response) per request. + fn serve_responses(listener: std::net::TcpListener, responses: Vec) { + use std::io::{Read, Write}; + std::thread::spawn(move || { + for (served, mut stream) in listener.incoming().flatten().enumerate() { + let index = served.min(responses.len() - 1); + // Drain the request first: closing with unread inbound data + // would send a TCP RST and destroy the response in flight + let mut buf = [0u8; 4096]; + loop { + match stream.read(&mut buf) { + Ok(0) => break, + Ok(n) if buf[..n].windows(4).any(|w| w == b"\r\n\r\n") => break, + Ok(_) => continue, + Err(_) => break, + } + } + let body = &responses[index]; + let _ = stream.write_all( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ) + .as_bytes(), + ); + let _ = stream.flush(); + } + }); + } + + /// One `getPublishedSources` page carrying `versions`, with the + /// `next_collection_link` of a truncated page when `next` is given + fn collection_body(versions: &[&str], next: Option<&str>) -> String { + let entries: Vec = versions + .iter() + .map(|v| format!(r#"{{"source_package_version": "{v}"}}"#)) + .collect(); + let next_field = next + .map(|link| format!(r#", "next_collection_link": "{link}""#)) + .unwrap_or_default(); + format!( + r#"{{"start": 0, "total_size": {}, "entries": [{}]{next_field}}}"#, + versions.len(), + entries.join(", ") + ) + } + + /// Bind a fresh mock server ready to serve `responses` (the caller + /// needs the address to build self-referential `next_collection_link`s + /// before serving starts) + fn bound_collection_server() -> (std::net::TcpListener, String) { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + (listener, base) + } + + /// The collection walk follows `next_collection_link`: the versions of + /// every page are collected in order, and the walk stops on the page + /// without a next link (the mock repeats its last response forever, so + /// an extra fetch would still pass — but a missing next-link handling + /// would drop page two's versions from the result) + #[tokio::test] + async fn walk_collection_collects_every_page() { + let (listener, base) = bound_collection_server(); + serve_responses( + listener, + vec![ + collection_body(&["1.0-1", "1.6-1"], Some(&format!("{base}/next"))), + collection_body(&["0.9-1"], None), + ], + ); + + let versions = walk_collection( + crate::distro_info::http_client(), + &format!("{base}/~u/+archive/ubuntu/p?ws.op=getPublishedSources"), + "u/p", + ) + .await + .unwrap(); + assert_eq!(versions, vec!["1.0-1", "1.6-1", "0.9-1"]); + } + + /// A next-link chain that never ends must error, not loop forever: the + /// partial list would feed the superseded check a false "not superseded" + #[tokio::test] + async fn walk_collection_errors_when_pagination_never_ends() { + // The last (only) response repeats forever, each page linking back + // to the server: the walk must stop at the page cap by itself + let (listener, base) = bound_collection_server(); + serve_responses( + listener, + vec![collection_body(&["1.0-1"], Some(&format!("{base}/loop")))], + ); + + let err = walk_collection( + crate::distro_info::http_client(), + &format!("{base}/~u/+archive/ubuntu/p?ws.op=getPublishedSources"), + "u/p", + ) + .await + .unwrap_err() + .to_string(); + assert!( + err.contains("keeps paginating the published sources of 'u/p'"), + "unexpected: {err}" + ); } }