From 13f6a4f4b40d84bde57a4ff16ccc78cd427babf0 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 20:28:50 -0300 Subject: [PATCH 01/22] fix(nodejs): honour cookie scope and stop truncating token values The session jar stored cookies in a flat name/value object and replayed all of them on every request, so a cookie set by www.flightradar24.com was also sent to cdn./api./data-live./data-cloud., and Path, Secure and expiry were ignored entirely. `Set-Cookie` was also split on every `=`, which truncated any value containing one: `_frPl` arrived as `sess.token.with` instead of `sess.token.with==padding`, and that truncated value is what getAirportDetails sends as `token` and getFlights as `enc`. Parse the attributes properly and key the jar by name/domain/path. `Max-Age=0` now deletes rather than storing an empty value, and a Map keeps a cookie named `__proto__` off Object.prototype. --- nodejs/FlightRadarAPI/index.d.ts | 4 +- nodejs/FlightRadarAPI/request.js | 198 ++++++++++++++++++++++++--- nodejs/tests/testFeedRetry.js | 5 +- nodejs/tests/testRequestTransport.js | 99 ++++++++++++++ 4 files changed, 284 insertions(+), 22 deletions(-) diff --git a/nodejs/FlightRadarAPI/index.d.ts b/nodejs/FlightRadarAPI/index.d.ts index 0ed2c1a..7382f44 100644 --- a/nodejs/FlightRadarAPI/index.d.ts +++ b/nodejs/FlightRadarAPI/index.d.ts @@ -25,12 +25,12 @@ export interface ImpersonateOptions { */ export class APIClient { constructor(options?: { impersonate?: ImpersonateOptions; retry?: RetryPolicy }); - request(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record}>; + request(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record; rawCookies: string[]}>; /** * Make a stateless request that bypasses the shared cookie jar. Safe to * call from concurrent fan-outs. */ - requestStandalone(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record}>; + requestStandalone(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record; rawCookies: string[]}>; getCookie(name: string): string | undefined; clearCookies(): void; /** Drop a single cookie, leaving the rest of the jar intact. */ diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index 94a07d9..d0ce206 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -138,6 +138,97 @@ async function runWithRetry(fn, retry) { const DEFAULT_TIMEOUT_MS = 30_000; +/** + * Parse one `Set-Cookie` header into a cookie record. + * + * The name/value pair is split on the FIRST `=` only: session tokens are + * routinely base64 and end in `=` padding, which a greedy split truncates. + * + * @param {string} header - a single Set-Cookie value + * @param {URL} url - the URL the header arrived from, for the default scope + * @return {object|null} `{name, value, domain, path, secure, hostOnly, expires}`, or null if unparsable + */ +function parseSetCookie(header, url) { + const [pair, ...attributeParts] = String(header).split(";"); + const separator = pair.indexOf("="); + + if (separator < 1) return null; + + const name = pair.slice(0, separator).trim(); + const value = pair.slice(separator + 1).trim(); + + if (!name) return null; + + const cookie = { + name, + value, + domain: url.hostname, + path: defaultPath(url.pathname), + secure: false, + hostOnly: true, + expires: null, + }; + + for (const part of attributeParts) { + const index = part.indexOf("="); + const key = (index < 0 ? part : part.slice(0, index)).trim().toLowerCase(); + const attributeValue = index < 0 ? "" : part.slice(index + 1).trim(); + + if (key === "secure") cookie.secure = true; + else if (key === "path" && attributeValue.startsWith("/")) cookie.path = attributeValue; + else if (key === "expires" && cookie.expires === null) { + const parsed = Date.parse(attributeValue); + if (!Number.isNaN(parsed)) cookie.expires = parsed; + } + else if (key === "max-age") { + const seconds = Number(attributeValue); + // Max-Age wins over Expires, and <= 0 means "delete now". + if (Number.isFinite(seconds)) cookie.expires = Date.now() + seconds * 1000; + } + else if (key === "domain" && attributeValue) { + const domain = attributeValue.replace(/^\./, "").toLowerCase(); + if (domainMatches(url.hostname, domain)) { + cookie.domain = domain; + cookie.hostOnly = false; + } + } + } + + return cookie; +} + +/** + * RFC 6265 default-path: the request path up to, but not including, the rightmost `/`. + * + * @param {string} pathname + * @return {string} + */ +function defaultPath(pathname) { + if (!pathname.startsWith("/")) return "/"; + const lastSlash = pathname.lastIndexOf("/"); + return lastSlash < 1 ? "/" : pathname.slice(0, lastSlash); +} + +/** + * @param {string} host - request hostname + * @param {string} domain - cookie domain, without a leading dot + * @return {boolean} whether the cookie's domain covers this host + */ +function domainMatches(host, domain) { + return host === domain || host.endsWith("." + domain); +} + +/** + * @param {string} requestPath + * @param {string} cookiePath + * @return {boolean} + */ +function pathMatches(requestPath, cookiePath) { + if (requestPath === cookiePath) return true; + if (!requestPath.startsWith(cookiePath)) return false; + return cookiePath.endsWith("/") || requestPath[cookiePath.length] === "/"; +} + /** * Detect Cloudflare-level blocks. * @@ -248,21 +339,28 @@ async function request(url, { content = await response.arrayBuffer(); } - const rawCookies = response.headers.getSetCookie(); + const rawCookies = response.headers.getSetCookie() ?? []; const responseCookies = {}; - if (rawCookies?.length > 0) { - rawCookies.forEach((string) => { - const keyAndValue = string.split(";")[0].split("="); - responseCookies[keyAndValue[0]] = keyAndValue[1]; - }); + for (const header of rawCookies) { + const pair = String(header).split(";")[0]; + const separator = pair.indexOf("="); + + // Split on the first `=` only, so base64 padding survives. + if (separator > 0) responseCookies[pair.slice(0, separator).trim()] = pair.slice(separator + 1).trim(); } - return { content, statusCode, cookies: responseCookies }; + return { content, statusCode, cookies: responseCookies, rawCookies }; } /** * HTTP session that automatically manages cookies across requests. + * + * The jar honours the scope FR24 sets on each cookie: a cookie stored by + * `www.flightradar24.com` is not replayed to `cdn.`/`api.`/`data-live.`, and + * `Path`, `Secure` and expiry are respected. A Map keyed by name/domain/path + * keeps same-named cookies from different hosts apart, and keeps a cookie + * called `__proto__` from reaching Object.prototype. */ class Session { /** @@ -270,29 +368,32 @@ class Session { * @param {object} [options.dispatcher] - undici Agent to use for every request. */ constructor({ dispatcher = null } = {}) { - this.__cookies = {}; + this.__jar = new Map(); this.__dispatcher = dispatcher; } /** - * Return the value of a stored cookie by name. + * Return the value of a stored cookie by name, ignoring scope. * * @param {string} name * @return {string|undefined} */ getCookie(name) { - return this.__cookies[name]; + for (const cookie of this.__jar.values()) { + if (cookie.name === name && !this.__isExpired(cookie)) return cookie.value; + } + return undefined; } /** * Clear all stored cookies. */ clearCookies() { - this.__cookies = {}; + this.__jar.clear(); } /** - * Drop a single stored cookie, leaving the rest of the jar intact. + * Drop every stored cookie with this name, leaving the rest of the jar intact. * * Sheds load-balancer stickiness without discarding the login session, * which lives in the same jar. @@ -300,12 +401,73 @@ class Session { * @param {string} name */ deleteCookie(name) { - delete this.__cookies[name]; + for (const [key, cookie] of this.__jar) { + if (cookie.name === name) this.__jar.delete(key); + } + } + + /** + * @param {object} cookie + * @return {boolean} + */ + __isExpired(cookie) { + return cookie.expires !== null && cookie.expires <= Date.now(); } /** - * Make an HTTP request, automatically sending stored cookies and storing - * any cookies returned by the response. + * Store the `Set-Cookie` headers a response arrived with. + * + * @param {string} url - the URL that produced the response + * @param {Array} rawCookies + */ + __storeCookies(url, rawCookies) { + const target = new URL(url); + + for (const header of rawCookies ?? []) { + const cookie = parseSetCookie(header, target); + + if (cookie === null) continue; + + const key = `${cookie.name};${cookie.domain};${cookie.path}`; + + // An expiry in the past is a deletion instruction, not a value. + if (this.__isExpired(cookie)) this.__jar.delete(key); + else this.__jar.set(key, cookie); + } + } + + /** + * Select the stored cookies that are in scope for a URL. + * + * @param {string} url + * @return {object} name/value pairs to send + */ + __cookiesFor(url) { + const target = new URL(url); + const isSecure = target.protocol === "https:"; + const selected = {}; + + for (const [key, cookie] of this.__jar) { + if (this.__isExpired(cookie)) { + this.__jar.delete(key); + continue; + } + if (cookie.secure && !isSecure) continue; + if (!pathMatches(target.pathname, cookie.path)) continue; + + const hostInScope = cookie.hostOnly ? + target.hostname === cookie.domain : + domainMatches(target.hostname, cookie.domain); + + if (hostInScope) selected[cookie.name] = cookie.value; + } + + return selected; + } + + /** + * Make an HTTP request, automatically sending the cookies that are in + * scope for the URL and storing any cookies the response returns. * * Accepts the same parameters as the module-level {@link request} function. * @@ -315,7 +477,7 @@ class Session { */ async request(url, options = {}) { const { cookies: extraCookies, ...rest } = options; - const merged = { ...this.__cookies, ...(extraCookies ?? {}) }; + const merged = { ...this.__cookiesFor(url), ...(extraCookies ?? {}) }; const cookies = Object.keys(merged).length > 0 ? merged : null; const result = await request(url, { @@ -324,9 +486,7 @@ class Session { cookies, }); - if (result.cookies && Object.keys(result.cookies).length > 0) { - Object.assign(this.__cookies, result.cookies); - } + this.__storeCookies(url, result.rawCookies); return result; } diff --git a/nodejs/tests/testFeedRetry.js b/nodejs/tests/testFeedRetry.js index 4e56e71..15989f7 100644 --- a/nodejs/tests/testFeedRetry.js +++ b/nodejs/tests/testFeedRetry.js @@ -119,7 +119,10 @@ describe("Session.deleteCookie (offline)", function() { const { Session } = require("../FlightRadarAPI/request"); const session = new Session(); - session.__cookies = { AWSALB: "sticky", _frPl: "login-token" }; + session.__storeCookies("https://data-cloud.flightradar24.com/zones/fcgi/feed.js", [ + "AWSALB=sticky; Path=/", + "_frPl=login-token; Path=/", + ]); session.deleteCookie("AWSALB"); expect(session.getCookie("AWSALB")).to.equal(undefined); diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index bda12c4..202d686 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -201,3 +201,102 @@ describe("Timeout rewrap (transport)", function() { } }); }); + + +describe("Session cookie jar scope (offline)", function() { + const { Session } = require("../FlightRadarAPI/request"); + + let mockAgent; + let sitePool; + let cdnPool; + let session; + + /** + * @param {object|Array} headers - headers as the stub received them + * @return {string|undefined} the Cookie header, if any + */ + const cookieHeaderOf = (headers) => (Array.isArray(headers) ? + headers.find((h) => h.toLowerCase().startsWith("cookie:")) : + (headers?.cookie || headers?.Cookie)); + + beforeEach(function() { + mockAgent = new MockAgent(); + mockAgent.disableNetConnect(); + sitePool = mockAgent.get("https://www.flightradar24.com"); + cdnPool = mockAgent.get("https://cdn.flightradar24.com"); + session = new Session({ dispatcher: mockAgent }); + }); + + afterEach(async function() { + await mockAgent.close(); + }); + + /** + * @param {Array} setCookie - Set-Cookie headers the login stub replies with + * @return {Promise} + */ + async function login(setCookie) { + sitePool.intercept({ path: "/user/login" }).reply(200, { success: true }, { + headers: { "content-type": "application/json", "set-cookie": setCookie }, + }); + await session.request("https://www.flightradar24.com/user/login"); + } + + it("keeps a token whose value contains '='", async function() { + await login(["_frPl=sess.token.with==padding; Path=/; Secure"]); + + expect(session.getCookie("_frPl")).to.equal("sess.token.with==padding"); + }); + + it("does not replay a www cookie to the cdn host", async function() { + await login(["_frPl=login-token; Path=/; Secure"]); + + let received = null; + cdnPool.intercept({ path: "/assets/airlines/logotypes/AA_AAL.png" }).reply((opts) => { + received = opts.headers; + return { statusCode: 200, data: "" }; + }); + await session.request("https://cdn.flightradar24.com/assets/airlines/logotypes/AA_AAL.png"); + + expect(cookieHeaderOf(received)).to.equal(undefined); + }); + + it("sends the cookie back to the host that set it", async function() { + await login(["_frPl=login-token; Path=/; Secure"]); + + let received = null; + sitePool.intercept({ path: "/webapi/v1/bookmarks" }).reply((opts) => { + received = opts.headers; + return { statusCode: 200, data: "{}" }; + }); + await session.request("https://www.flightradar24.com/webapi/v1/bookmarks"); + + expect(String(cookieHeaderOf(received))).to.include("_frPl=login-token"); + }); + + it("treats Max-Age=0 as a deletion", async function() { + await login(["_frPl=login-token; Path=/", "AWSALB=sticky; Path=/"]); + expect(session.getCookie("AWSALB")).to.equal("sticky"); + + sitePool.intercept({ path: "/logout" }).reply(200, {}, { + headers: { "content-type": "application/json", "set-cookie": ["AWSALB=; Path=/; Max-Age=0"] }, + }); + await session.request("https://www.flightradar24.com/logout"); + + expect(session.getCookie("AWSALB")).to.equal(undefined); + expect(session.getCookie("_frPl")).to.equal("login-token"); + }); + + it("respects the Path attribute", async function() { + await login(["scoped=yes; Path=/data"]); + + let received = null; + sitePool.intercept({ path: "/flights/most-tracked" }).reply((opts) => { + received = opts.headers; + return { statusCode: 200, data: "{}" }; + }); + await session.request("https://www.flightradar24.com/flights/most-tracked"); + + expect(cookieHeaderOf(received)).to.equal(undefined); + }); +}); From 77870fbcfd5f40b123f56b8ee61893e6b801de07 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 20:28:50 -0300 Subject: [PATCH 02/22] build(deps): raise dependency floors past known advisories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lockfile already resolved a clean undici, but consumers of a published library resolve the declared range instead, and `^6.13.0` admits 6.13.0-6.27.0 — 15 advisories including request smuggling and an unbounded decompression chain. The Python dependencies had no bounds at all, which also left them as the only three packages in the dependency graph with no resolved version, so Dependabot could not match advisories against them. --- nodejs/package-lock.json | 2 +- nodejs/package.json | 2 +- python/pyproject.toml | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index a28f5c9..0af00d5 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -10,7 +10,7 @@ "license": "MIT", "dependencies": { "node-html-parser": "^6.1.13", - "undici": "^6.13.0" + "undici": "^6.28.0" }, "devDependencies": { "chai": "^4.3.10", diff --git a/nodejs/package.json b/nodejs/package.json index 9ff4cac..c65f564 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -36,7 +36,7 @@ }, "dependencies": { "node-html-parser": "^6.1.13", - "undici": "^6.13.0" + "undici": "^6.28.0" }, "devDependencies": { "chai": "^4.3.10", diff --git a/python/pyproject.toml b/python/pyproject.toml index 68458ec..a0a9c0a 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -18,9 +18,12 @@ classifiers = [ ] requires-python = ">=3.10" dependencies = [ - "Brotli", - "beautifulsoup4", - "curl_cffi", + # Floors are security minimums, not feature minimums: Brotli < 1.2.0 has no + # decompression-bomb mitigation (GHSA-2qfp-q593-8484) and curl_cffi < 0.15.0 + # follows redirects into internal networks (GHSA-qw2m-4pqf-rmpp). + "Brotli>=1.2.0", + "beautifulsoup4>=4.12.0", + "curl_cffi>=0.15.0", ] [tool.hatch.build] From 894be48492ae80616da8e17814abcafd7a939d2e Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 21:02:33 -0300 Subject: [PATCH 03/22] fix(nodejs): correct cookie edge cases found in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five defects in the new jar, all reproduced before fixing: - A malformed `Max-Age` (bare or empty) read as 0 and deleted the cookie, because Number("") is 0. RFC 6265 says ignore a non-integer value. A bare `Max-Age` from FR24 would have dropped the login token silently. - `Domain=com` passed the suffix check, so one response could plant a cookie replayed to every .com host the caller later requested — the cross-host leak this change exists to close. Reject dotless domains. - getCookie() returned the first jar entry by insertion order, so a re-issued token under a different scope never superseded the old one and authenticated calls kept sending a dead `token`/`enc`. Take the newest instead. - Cookies were attributed to the requested URL, but fetch follows redirects, so `Set-Cookie` belongs to the final URL. Key off that. - Two in-scope cookies of the same name resolved by insertion order rather than specificity. Sort so the longer path wins. The negative scoping assertions passed vacuously if an interceptor never fired; they now assert it did. The pyproject comment claimed the Brotli floor mitigated decompression bombs. It does not: 1.2.0 only adds the bounded API, and request.py still calls the unbounded brotli.decompress. Claim removed rather than left overstating what the bump buys. --- nodejs/FlightRadarAPI/request.js | 43 +++++++++++++++++++++------- nodejs/tests/testRequestTransport.js | 36 +++++++++++++++++++++++ python/pyproject.toml | 4 +-- 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index d0ce206..83891c0 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -180,14 +180,16 @@ function parseSetCookie(header, url) { const parsed = Date.parse(attributeValue); if (!Number.isNaN(parsed)) cookie.expires = parsed; } - else if (key === "max-age") { - const seconds = Number(attributeValue); - // Max-Age wins over Expires, and <= 0 means "delete now". - if (Number.isFinite(seconds)) cookie.expires = Date.now() + seconds * 1000; + else if (key === "max-age" && /^-?\d+$/.test(attributeValue)) { + // Max-Age wins over Expires, and <= 0 means "delete now". A malformed + // value must be ignored, not read as 0 — that would delete the cookie. + cookie.expires = Date.now() + Number(attributeValue) * 1000; } else if (key === "domain" && attributeValue) { const domain = attributeValue.replace(/^\./, "").toLowerCase(); - if (domainMatches(url.hostname, domain)) { + // A dotless domain is a TLD: `Domain=com` would scope the cookie to + // every .com host the caller later requests. + if (domain.includes(".") && domainMatches(url.hostname, domain)) { cookie.domain = domain; cookie.hostOnly = false; } @@ -350,7 +352,8 @@ async function request(url, { if (separator > 0) responseCookies[pair.slice(0, separator).trim()] = pair.slice(separator + 1).trim(); } - return { content, statusCode, cookies: responseCookies, rawCookies }; + // `response.url` is the URL after redirects — the host that actually set the cookies. + return { content, statusCode, cookies: responseCookies, rawCookies, url: response.url || url }; } /** @@ -369,6 +372,7 @@ class Session { */ constructor({ dispatcher = null } = {}) { this.__jar = new Map(); + this.__sequence = 0; this.__dispatcher = dispatcher; } @@ -379,10 +383,17 @@ class Session { * @return {string|undefined} */ getCookie(name) { + let match = null; + for (const cookie of this.__jar.values()) { - if (cookie.name === name && !this.__isExpired(cookie)) return cookie.value; + if (cookie.name !== name || this.__isExpired(cookie)) continue; + // The same name can live under several scopes at once. Take the + // newest: a re-issued token supersedes the one it replaces, even + // when the old one sat at a more specific path. + if (match === null || cookie.storedAt > match.storedAt) match = cookie; } - return undefined; + + return match === null ? undefined : match.value; } /** @@ -430,6 +441,8 @@ class Session { const key = `${cookie.name};${cookie.domain};${cookie.path}`; + cookie.storedAt = ++this.__sequence; + // An expiry in the past is a deletion instruction, not a value. if (this.__isExpired(cookie)) this.__jar.delete(key); else this.__jar.set(key, cookie); @@ -445,7 +458,7 @@ class Session { __cookiesFor(url) { const target = new URL(url); const isSecure = target.protocol === "https:"; - const selected = {}; + const matches = []; for (const [key, cookie] of this.__jar) { if (this.__isExpired(cookie)) { @@ -459,9 +472,17 @@ class Session { target.hostname === cookie.domain : domainMatches(target.hostname, cookie.domain); - if (hostInScope) selected[cookie.name] = cookie.value; + if (hostInScope) matches.push(cookie); } + // Shortest path first, so a more specific cookie overwrites a broader + // one of the same name rather than losing to insertion order. + matches.sort((a, b) => a.path.length - b.path.length); + + const selected = {}; + + for (const cookie of matches) selected[cookie.name] = cookie.value; + return selected; } @@ -486,7 +507,7 @@ class Session { cookies, }); - this.__storeCookies(url, result.rawCookies); + this.__storeCookies(result.url || url, result.rawCookies); return result; } diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index 202d686..e1ceb70 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -258,6 +258,7 @@ describe("Session cookie jar scope (offline)", function() { }); await session.request("https://cdn.flightradar24.com/assets/airlines/logotypes/AA_AAL.png"); + expect(received, "the cdn interceptor never fired").to.not.equal(null); expect(cookieHeaderOf(received)).to.equal(undefined); }); @@ -297,6 +298,41 @@ describe("Session cookie jar scope (offline)", function() { }); await session.request("https://www.flightradar24.com/flights/most-tracked"); + expect(received, "the interceptor never fired").to.not.equal(null); expect(cookieHeaderOf(received)).to.equal(undefined); }); + + it("ignores a malformed Max-Age instead of reading it as a deletion", async function() { + await login(["_frPl=login-token; Path=/; Max-Age"]); + + expect(session.getCookie("_frPl")).to.equal("login-token"); + }); + + it("refuses a Domain attribute naming a bare TLD", async function() { + await login(["evil=1; Domain=com; Path=/"]); + + expect(session.__cookiesFor("https://example.com/")).to.deep.equal({}); + }); + + it("prefers the re-issued cookie over the one it supersedes", async function() { + await login(["_frPl=old-token"]); + sitePool.intercept({ path: "/user/login" }).reply(200, { success: true }, { + headers: { + "content-type": "application/json", + "set-cookie": ["_frPl=new-token; Domain=.flightradar24.com; Path=/"], + }, + }); + await session.request("https://www.flightradar24.com/user/login"); + + expect(session.getCookie("_frPl")).to.equal("new-token"); + }); + + it("scopes a Path-less cookie to the directory that set it", async function() { + // FR24 sends `path=/` on every cookie observed, so this documents the + // RFC 6265 default rather than a path the SDK relies on. + await login(["scoped=yes"]); + + expect(session.__cookiesFor("https://www.flightradar24.com/user/settings")).to.deep.equal({ scoped: "yes" }); + expect(session.__cookiesFor("https://www.flightradar24.com/webapi/v1/bookmarks")).to.deep.equal({}); + }); }); diff --git a/python/pyproject.toml b/python/pyproject.toml index a0a9c0a..19cadd7 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -18,9 +18,7 @@ classifiers = [ ] requires-python = ">=3.10" dependencies = [ - # Floors are security minimums, not feature minimums: Brotli < 1.2.0 has no - # decompression-bomb mitigation (GHSA-2qfp-q593-8484) and curl_cffi < 0.15.0 - # follows redirects into internal networks (GHSA-qw2m-4pqf-rmpp). + # Security floors: curl_cffi 0.15.0 fixes redirect-based SSRF (GHSA-qw2m-4pqf-rmpp). "Brotli>=1.2.0", "beautifulsoup4>=4.12.0", "curl_cffi>=0.15.0", From d9216f6c6a7933135d67edcc846ed320308095ee Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 21:21:17 -0300 Subject: [PATCH 04/22] fix(nodejs): sync request() types and cover the redirect path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups: - `index.d.ts` still described the pre-`url` return shape, so TypeScript consumers could not see the field the jar now keys off. tsd passed because nothing exercised it. - The redirect attribution had no test at all — the subtlest change in the branch. MockAgent does not follow redirects, so this needs a real local server. Verified the test fails when the fix is reverted. - A `Secure` cookie arriving over plaintext is no longer stored; we already refused to send one, so accepting it was incoherent. - `storedAt` is initialised with the record, leaving the jar as the only thing that assigns it. Kept the hand-rolled Set-Cookie parser rather than delegating to undici's `getSetCookies`, which was the tempting simplification: it drops a negative `Max-Age` instead of deleting, accepts an empty cookie name, and widens a relative `Path` to `/`. Three regressions to remove two bugs is a bad trade, so those three cases now have tests pinning the behaviour. --- nodejs/FlightRadarAPI/index.d.ts | 4 +- nodejs/FlightRadarAPI/request.js | 6 +++ nodejs/tests/testRequestTransport.js | 80 ++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/nodejs/FlightRadarAPI/index.d.ts b/nodejs/FlightRadarAPI/index.d.ts index 7382f44..7df78a4 100644 --- a/nodejs/FlightRadarAPI/index.d.ts +++ b/nodejs/FlightRadarAPI/index.d.ts @@ -25,12 +25,12 @@ export interface ImpersonateOptions { */ export class APIClient { constructor(options?: { impersonate?: ImpersonateOptions; retry?: RetryPolicy }); - request(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record; rawCookies: string[]}>; + request(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record; rawCookies: string[]; url: string}>; /** * Make a stateless request that bypasses the shared cookie jar. Safe to * call from concurrent fan-outs. */ - requestStandalone(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record; rawCookies: string[]}>; + requestStandalone(url: string, options?: object): Promise<{content: any; statusCode: number; cookies: Record; rawCookies: string[]; url: string}>; getCookie(name: string): string | undefined; clearCookies(): void; /** Drop a single cookie, leaving the rest of the jar intact. */ diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index 83891c0..05f7c54 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -144,6 +144,10 @@ const DEFAULT_TIMEOUT_MS = 30_000; * The name/value pair is split on the FIRST `=` only: session tokens are * routinely base64 and end in `=` padding, which a greedy split truncates. * + * Not delegated to undici's `getSetCookies`: it drops a negative `Max-Age` + * instead of treating it as a deletion, accepts an empty cookie name, and + * widens a relative `Path` to `/`. See the parser tests. + * * @param {string} header - a single Set-Cookie value * @param {URL} url - the URL the header arrived from, for the default scope * @return {object|null} `{name, value, domain, path, secure, hostOnly, expires}`, or null if unparsable @@ -167,6 +171,7 @@ function parseSetCookie(header, url) { secure: false, hostOnly: true, expires: null, + storedAt: 0, }; for (const part of attributeParts) { @@ -438,6 +443,7 @@ class Session { const cookie = parseSetCookie(header, target); if (cookie === null) continue; + if (cookie.secure && target.protocol !== "https:") continue; const key = `${cookie.name};${cookie.domain};${cookie.path}`; diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index e1ceb70..c6a46ca 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -336,3 +336,83 @@ describe("Session cookie jar scope (offline)", function() { expect(session.__cookiesFor("https://www.flightradar24.com/webapi/v1/bookmarks")).to.deep.equal({}); }); }); + + +describe("Set-Cookie parsing edge cases (offline)", function() { + const { Session } = require("../FlightRadarAPI/request"); + + const jarAfter = (header, url = "https://www.flightradar24.com/user/login") => { + const session = new Session(); + session.__storeCookies(url, [header]); + return session; + }; + + // These three are why the parser is hand-rolled rather than delegated to + // undici's getSetCookies, which gets each of them wrong. + it("treats a negative Max-Age as a deletion", function() { + expect(jarAfter("a=1; Path=/; Max-Age=-5").getCookie("a")).to.equal(undefined); + }); + + it("rejects a cookie with an empty name", function() { + const session = jarAfter("=9; Path=/"); + expect(session.__cookiesFor("https://www.flightradar24.com/")).to.deep.equal({}); + }); + + it("ignores a relative Path instead of widening the cookie to /", function() { + const session = jarAfter("a=1; Path=relative"); + expect(session.__cookiesFor("https://www.flightradar24.com/user/settings")).to.deep.equal({ a: "1" }); + expect(session.__cookiesFor("https://www.flightradar24.com/webapi/v1/bookmarks")).to.deep.equal({}); + }); + + it("does not store a Secure cookie arriving over a plaintext connection", function() { + const session = jarAfter("a=1; Path=/; Secure", "http://insecure.example.com/"); + expect(session.getCookie("a")).to.equal(undefined); + }); + + it("keeps an Expires date in the past as a deletion", function() { + expect(jarAfter("a=1; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT").getCookie("a")).to.equal(undefined); + }); +}); + + +describe("Cookie attribution across redirects (offline)", function() { + const http = require("http"); + const { Session } = require("../FlightRadarAPI/request"); + + let server; + let port; + + beforeEach(function(done) { + // A real server, because MockAgent does not follow redirects. + // 127.0.0.1 and localhost are distinct origins to fetch. + server = http.createServer((req, res) => { + if (req.url === "/start") { + res.writeHead(302, { Location: `http://localhost:${port}/landed` }); + res.end(); + return; + } + if (req.url === "/landed") { + res.setHeader("Set-Cookie", "planted=yes; Path=/"); + } + res.setHeader("Content-Type", "application/json"); + res.end("{}"); + }); + server.listen(0, "127.0.0.1", () => { + port = server.address().port; + done(); + }); + }); + + afterEach(function(done) { + server.close(done); + }); + + it("credits a cookie to the host that ended the redirect chain", async function() { + const session = new Session(); + await session.request(`http://127.0.0.1:${port}/start`); + + // Set by localhost after the hop, so it must not be replayed to 127.0.0.1. + expect(session.__cookiesFor(`http://localhost:${port}/`)).to.deep.equal({ planted: "yes" }); + expect(session.__cookiesFor(`http://127.0.0.1:${port}/`)).to.deep.equal({}); + }); +}); From d2103e149253ff68d7afb8e40085d821066d1d43 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 21:25:09 -0300 Subject: [PATCH 05/22] test(nodejs): make the redirect test work on node 18 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server bound only 127.0.0.1 while the redirect targeted `localhost`. Node 18 resolves that to ::1 first, so the hop was refused. Node 22 happened to pick the v4 address, which is why this passed locally and failed in CI. Bind dual-stack instead, and drop keep-alive sockets in the teardown — server.close() waits for the undici agent's connections, so the failure was followed by an afterEach timeout that obscured it. Verified on node 18.14.2, 20.20.2 and 22.12.0: 76 passing on each, and the test still fails on 18 when the fix under test is reverted. --- nodejs/tests/testRequestTransport.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index c6a46ca..c39f4ac 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -397,13 +397,17 @@ describe("Cookie attribution across redirects (offline)", function() { res.setHeader("Content-Type", "application/json"); res.end("{}"); }); - server.listen(0, "127.0.0.1", () => { + // Bound dual-stack, not to 127.0.0.1: Node 18 resolves `localhost` to + // ::1 first, and a v4-only listener refuses that connection. + server.listen(0, () => { port = server.address().port; done(); }); }); afterEach(function(done) { + // The undici agent keeps sockets alive, and close() waits for them. + server.closeAllConnections?.(); server.close(done); }); From 1a4e5d7239f5e6b516fb3efff57f531f3227eb60 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 22:19:46 -0300 Subject: [PATCH 06/22] fix: bound response decompression in both ports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A compressed body was trusted to expand to any size. brotli reaches ratios high enough that ~500 bytes on the wire expands to 300 MB, and ~1.7 KB to 1 GB — enough to kill the process on a small host. Python now decompresses through `brotli.Decompressor.process` with a max-output argument and `zlib.decompressobj` with `max_length`, so the cap is enforced by the decompressor instead of being checked afterwards. That distinction is the whole fix: a first attempt checked the size between input chunks, which still materialised a 1 GB body (peak RSS 1936 MB) before reporting a breach it had already suffered. With the output cap, a 1 GB and a 4 GB bomb both peak at 253 MB — cost tracks the limit, not the payload. A test pins that with tracemalloc and fails against the chunk-checking version. Node reads `response.body` as a stream with a byte budget, since undici decompresses in the transport. Both ports raise DecompressionLimitError and default to 64 MiB, far above FR24's largest payload. Note the Brotli>=1.2.0 floor from an earlier commit does not do this on its own: 1.2.0 only makes the bounded API available. --- nodejs/FlightRadarAPI/errors.js | 7 +- nodejs/FlightRadarAPI/index.d.ts | 4 + nodejs/FlightRadarAPI/index.js | 6 +- nodejs/FlightRadarAPI/request.js | 60 ++++++++++++-- nodejs/tests/testRequestTransport.js | 63 +++++++++++++++ python/FlightRadarAPI/__init__.py | 11 ++- python/FlightRadarAPI/errors.py | 5 ++ python/FlightRadarAPI/request.py | 64 ++++++++++++++- python/tests/test_request_transport.py | 103 ++++++++++++++++++++++++- 9 files changed, 305 insertions(+), 18 deletions(-) diff --git a/nodejs/FlightRadarAPI/errors.js b/nodejs/FlightRadarAPI/errors.js index 6d1c237..3b1c986 100644 --- a/nodejs/FlightRadarAPI/errors.js +++ b/nodejs/FlightRadarAPI/errors.js @@ -23,7 +23,12 @@ class CloudflareError extends FlightRadarError { } } +/** Thrown when a response body exceeds the size budget. */ +class DecompressionLimitError extends FlightRadarError {} + /** Thrown when login fails or an authenticated endpoint is accessed without login. */ class LoginError extends FlightRadarError {} -module.exports = { FlightRadarError, AirportNotFoundError, CloudflareError, LoginError }; +module.exports = { + FlightRadarError, AirportNotFoundError, CloudflareError, DecompressionLimitError, LoginError, +}; diff --git a/nodejs/FlightRadarAPI/index.d.ts b/nodejs/FlightRadarAPI/index.d.ts index 7df78a4..536e7e0 100644 --- a/nodejs/FlightRadarAPI/index.d.ts +++ b/nodejs/FlightRadarAPI/index.d.ts @@ -520,6 +520,10 @@ export class CloudflareError extends FlightRadarError { constructor(message?: string, response?: any); } +export class DecompressionLimitError extends FlightRadarError { + constructor(message?: string); +} + export class LoginError extends FlightRadarError { constructor(message?: string); } diff --git a/nodejs/FlightRadarAPI/index.js b/nodejs/FlightRadarAPI/index.js index 9bd40ad..afe5f47 100644 --- a/nodejs/FlightRadarAPI/index.js +++ b/nodejs/FlightRadarAPI/index.js @@ -9,7 +9,9 @@ * https://www.flightradar24.com/terms-and-conditions */ -const { FlightRadarError, AirportNotFoundError, CloudflareError, LoginError } = require("./errors"); +const { + FlightRadarError, AirportNotFoundError, CloudflareError, DecompressionLimitError, LoginError, +} = require("./errors"); const FlightRadar24API = require("./api"); const FlightTrackerConfig = require("./flightTrackerConfig"); const Airport = require("./entities/airport"); @@ -25,7 +27,7 @@ module.exports = { FlightTrackerConfig, Countries, Airport, Entity, Flight, - FlightRadarError, AirportNotFoundError, CloudflareError, LoginError, + FlightRadarError, AirportNotFoundError, CloudflareError, DecompressionLimitError, LoginError, RetryPolicy, APIClient, author, version, }; diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index 05f7c54..807be53 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -1,4 +1,4 @@ -const { CloudflareError } = require("./errors"); +const { CloudflareError, DecompressionLimitError } = require("./errors"); const { fetch, Agent } = require("undici"); /** Thrown when a request exceeds its timeout. Surfaced as a distinct class @@ -138,6 +138,50 @@ async function runWithRetry(fn, retry) { const DEFAULT_TIMEOUT_MS = 30_000; +// A compressed body is trusted only as far as its expanded size: brotli reaches +// ratios high enough to exhaust memory from a few kilobytes on the wire. undici +// decompresses in the transport, so the budget lands on the decoded body. +// FR24's largest payload (the airports feed) is orders of magnitude under this. +const MAX_RESPONSE_BYTES = 64 * 1024 * 1024; + +/** + * Read a response body, refusing one that grows past `limit`. + * + * Streamed rather than buffered whole so the cap bounds the work: reading + * stops and the socket is released at the first chunk over budget, instead of + * discovering the size after paying for it. + * + * @param {Response} response + * @param {number} limit - maximum bytes to accept + * @param {string} url - for the error message + * @return {Promise} + */ +async function readBoundedBody(response, limit, url) { + if (response.body === null) return Buffer.alloc(0); + + const reader = response.body.getReader(); + const chunks = []; + let total = 0; + + for (;;) { + const { done, value } = await reader.read(); + + if (done) break; + + total += value.byteLength; + + if (total > limit) { + await reader.cancel(); + throw new DecompressionLimitError( + `Response body from ${url} exceeds the ${limit} byte limit.`, + ); + } + chunks.push(value); + } + + return Buffer.concat(chunks); +} + /** * Parse one `Set-Cookie` header into a cookie record. * @@ -270,6 +314,7 @@ function isCloudflareBlock(statusCode, headers) { * @param {object} [options.cookies] - Cookies to include in the request * @param {Array} [options.allowedErrorCodes=[]] - Status codes that should not throw * @param {number} [options.timeout=30000] - Request timeout in milliseconds + * @param {number} [options.maxResponseBytes] - Maximum accepted response body size * @return {Promise<{content: *, statusCode: number, cookies: object}>} */ async function request(url, { @@ -280,6 +325,7 @@ async function request(url, { allowedErrorCodes = [], timeout = DEFAULT_TIMEOUT_MS, dispatcher = null, + maxResponseBytes = MAX_RESPONSE_BYTES, } = {}) { if (params !== null && Object.keys(params).length > 0) { url += "?" + new URLSearchParams(params).toString(); @@ -334,16 +380,17 @@ async function request(url, { } const contentType = response.headers.get("content-type") ?? ""; + const body = await readBoundedBody(response, maxResponseBytes, url); let content; if (contentType.includes("application/json")) { - content = await response.json(); + content = JSON.parse(body.toString("utf-8")); } else if (contentType.includes("text")) { - content = await response.text(); + content = body.toString("utf-8"); } else { - content = await response.arrayBuffer(); + content = body.buffer.slice(body.byteOffset, body.byteOffset + body.byteLength); } const rawCookies = response.headers.getSetCookie() ?? []; @@ -593,4 +640,7 @@ class APIClient { } } -module.exports = { request, Session, APIClient, RetryPolicy, buildImpersonateAgent, CHROME136_PROFILE }; +module.exports = { + request, Session, APIClient, RetryPolicy, buildImpersonateAgent, CHROME136_PROFILE, + MAX_RESPONSE_BYTES, +}; diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index c39f4ac..770c02d 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -420,3 +420,66 @@ describe("Cookie attribution across redirects (offline)", function() { expect(session.__cookiesFor(`http://127.0.0.1:${port}/`)).to.deep.equal({}); }); }); + + +describe("Response size budget (offline)", function() { + const { request, MAX_RESPONSE_BYTES } = require("../FlightRadarAPI/request"); + const { DecompressionLimitError } = require("../FlightRadarAPI/errors"); + + let mockAgent; + let mockPool; + + beforeEach(function() { + mockAgent = new MockAgent(); + mockAgent.disableNetConnect(); + mockPool = mockAgent.get("https://example.com"); + }); + + afterEach(async function() { + await mockAgent.close(); + }); + + it("refuses a body past the budget", async function() { + mockPool.intercept({ path: "/huge" }) + .reply(200, "x".repeat(4096), { headers: { "content-type": "text/plain" } }); + + try { + await request("https://example.com/huge", { dispatcher: mockAgent, maxResponseBytes: 1024 }); + expect.fail("should have refused the body"); + } + catch (err) { + expect(err).to.be.instanceOf(DecompressionLimitError); + expect(err.message).to.include("1024"); + } + }); + + it("accepts a body at the budget", async function() { + mockPool.intercept({ path: "/exact" }) + .reply(200, "x".repeat(1024), { headers: { "content-type": "text/plain" } }); + + const { content } = await request("https://example.com/exact", { + dispatcher: mockAgent, maxResponseBytes: 1024, + }); + expect(content).to.have.lengthOf(1024); + }); + + it("still dispatches on content-type after the change to streamed reads", async function() { + mockPool.intercept({ path: "/j" }) + .reply(200, { a: 1 }, { headers: { "content-type": "application/json" } }); + mockPool.intercept({ path: "/t" }) + .reply(200, "hi", { headers: { "content-type": "text/plain" } }); + mockPool.intercept({ path: "/b" }) + .reply(200, Buffer.from([1, 2, 3]), { headers: { "content-type": "image/png" } }); + + expect((await request("https://example.com/j", { dispatcher: mockAgent })).content).to.deep.equal({ a: 1 }); + expect((await request("https://example.com/t", { dispatcher: mockAgent })).content).to.equal("hi"); + + const binary = (await request("https://example.com/b", { dispatcher: mockAgent })).content; + expect(binary).to.be.instanceOf(ArrayBuffer); + expect([...Buffer.from(binary)]).to.deep.equal([1, 2, 3]); + }); + + it("defaults to a 64 MiB budget", function() { + expect(MAX_RESPONSE_BYTES).to.equal(64 * 1024 * 1024); + }); +}); diff --git a/python/FlightRadarAPI/__init__.py b/python/FlightRadarAPI/__init__.py index 52e7635..e48a9fe 100644 --- a/python/FlightRadarAPI/__init__.py +++ b/python/FlightRadarAPI/__init__.py @@ -12,12 +12,18 @@ """ __author__ = "Jean Loui Bernard Silva de Jesus" -__version__ = "1.5.3" +__version__ = "1.5.4" from .api import FlightRadar24API from .core import Countries from .entities import Airport, Entity, Flight -from .errors import AirportNotFoundError, CloudflareError, FlightRadarError, LoginError +from .errors import ( + AirportNotFoundError, + CloudflareError, + DecompressionLimitError, + FlightRadarError, + LoginError, +) from .flight_tracker_config import FlightTrackerConfig from .request import RetryPolicy @@ -29,6 +35,7 @@ "Flight", "AirportNotFoundError", "CloudflareError", + "DecompressionLimitError", "FlightRadarError", "LoginError", "FlightTrackerConfig", diff --git a/python/FlightRadarAPI/errors.py b/python/FlightRadarAPI/errors.py index 1dd414d..dd7a53c 100644 --- a/python/FlightRadarAPI/errors.py +++ b/python/FlightRadarAPI/errors.py @@ -16,5 +16,10 @@ def __init__(self, message: str, response): self.response = response +class DecompressionLimitError(FlightRadarError): + """Raised when a response body expands past the decompression budget.""" + pass + + class LoginError(FlightRadarError): pass diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index 991b113..f656c86 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -1,10 +1,10 @@ # -*- coding: utf-8 -*- -import gzip import json import logging import random import time +import zlib from typing import Any, Dict, List, Optional, Union from urllib.parse import urlencode @@ -12,12 +12,66 @@ from curl_cffi import requests from curl_cffi.requests import Session -from .errors import CloudflareError +from .errors import CloudflareError, DecompressionLimitError _logger = logging.getLogger(__name__) DEFAULT_IMPERSONATE = "chrome136" +# A compressed body is trusted only as far as its expanded size: brotli reaches +# ratios high enough to exhaust memory from a few kilobytes on the wire. FR24's +# largest payload (the airports feed) is orders of magnitude under this. +MAX_DECOMPRESSED_BYTES = 64 * 1024 * 1024 +_GZIP_WBITS = 31 # 16 + MAX_WBITS: gzip wrapper rather than raw deflate + + +def _decompress_gzip(data: bytes, limit: int = MAX_DECOMPRESSED_BYTES) -> bytes: + """Inflate gzip bytes, refusing a body that expands past ``limit``.""" + decompressor = zlib.decompressobj(_GZIP_WBITS) + output = decompressor.decompress(data, limit + 1) + + # Output stops at max_length, so anything left over means the body did not fit. + if len(output) > limit or decompressor.unconsumed_tail: + raise DecompressionLimitError( + f"gzip body expands past the {limit} byte decompression limit." + ) + return output + + +def _decompress_brotli(data: bytes, limit: int = MAX_DECOMPRESSED_BYTES) -> bytes: + """Decompress brotli bytes, refusing a body that expands past ``limit``. + + ``process`` takes a max-output argument, so the cap is enforced by the + decompressor rather than checked after the fact: peak memory stays near + ``limit`` no matter how far the body would have expanded. + """ + decompressor = brotli.Decompressor() + output = bytearray() + fed = False + + while not decompressor.is_finished(): + room = limit + 1 - len(output) + + if room <= 0: + raise DecompressionLimitError( + f"brotli body expands past the {limit} byte decompression limit." + ) + + piece = decompressor.process(data if not fed else b"", room) + fed = True + output += piece + + if len(output) > limit: + raise DecompressionLimitError( + f"brotli body expands past the {limit} byte decompression limit." + ) + + # No output left and still hungry: the stream ended mid-message. + if not piece and decompressor.can_accept_more_data(): + break + + return bytes(output) + class RetryPolicy: """ @@ -140,8 +194,8 @@ class APIRequest: """ __content_encodings = { "": lambda x: x, - "br": brotli.decompress, - "gzip": gzip.decompress + "br": _decompress_brotli, + "gzip": _decompress_gzip } def __init__( @@ -232,6 +286,8 @@ def get_content(self) -> Union[Dict, bytes]: decode = self.__content_encodings.get(content_encoding, self.__content_encodings[""]) try: content = decode(content) + except DecompressionLimitError: + raise except Exception as err: # Decided by the body, not the header: undecodable text is genuinely # broken and must warn, while binary bodies carry no such tell. Nothing diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index 03f0187..fae1849 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -14,10 +14,9 @@ Deliberately **not** here: -- gzip / brotli decompression. Whether decompression happens in the SDK or - in the transport layer depends entirely on the library; testing it tells - us little beyond "the standard library still works". Removed because the - signal-to-maintenance ratio was poor. +- Whether decompression happens in the SDK or in the transport layer, which + depends entirely on the library. The decompression *budget* is tested below, + because that logic is ours and a bomb slipping past it is a real failure. """ from typing import Any, Dict @@ -102,3 +101,99 @@ def get(self, url, **kwargs): # type: ignore[override] ) assert "code=ATL" in captured["url"] assert "limit=1" in captured["url"] + + +class TestDecompressionLimit: + """A compressed body must not be trusted to expand to any size. + + An explicit small ``limit`` keeps these fast: a 1 MB expansion against a + 1 KiB cap exercises the same code path as a 4 GB one against 64 MB. + """ + + ONE_MB = b"\x00" * (1024 * 1024) + + def test_brotli_bomb_is_refused(self): + import brotli + + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import _decompress_brotli + + with pytest.raises(DecompressionLimitError): + _decompress_brotli(brotli.compress(self.ONE_MB), limit=1024) + + def test_gzip_bomb_is_refused(self): + import gzip + + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import _decompress_gzip + + with pytest.raises(DecompressionLimitError): + _decompress_gzip(gzip.compress(self.ONE_MB), limit=1024) + + def test_the_body_is_never_materialised_past_the_cap(self): + """The cap must bound the work, not just the verdict. + + Regression test: an earlier attempt checked the size only after each + input chunk, so a small bomb was fully expanded before the check ran + and the limit reported a breach it had already suffered. + """ + import brotli + import tracemalloc + + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import _decompress_brotli + + bomb = brotli.compress(self.ONE_MB * 32) # 32 MB expanded + + tracemalloc.start() + try: + with pytest.raises(DecompressionLimitError): + _decompress_brotli(bomb, limit=64 * 1024) + _, peak = tracemalloc.get_traced_memory() + finally: + tracemalloc.stop() + + # Generous headroom for the decompressor's own buffers, but nowhere + # near the 32 MB the body would have expanded to. + assert peak < 4 * 1024 * 1024, f"peaked at {peak} bytes" + + @pytest.mark.parametrize("encoding", ["br", "gzip"]) + def test_a_body_under_the_limit_round_trips_exactly(self, encoding): + import gzip + + import brotli + + from FlightRadarAPI.request import _decompress_brotli, _decompress_gzip + + body = b'{"rows": [{"name": "Guarulhos", "iata": "GRU"}]}' * 100 + compress, decompress = { + "br": (brotli.compress, _decompress_brotli), + "gzip": (gzip.compress, _decompress_gzip), + }[encoding] + + assert decompress(compress(body)) == body + + def test_an_empty_body_is_not_mistaken_for_a_bomb(self): + import gzip + + import brotli + + from FlightRadarAPI.request import _decompress_brotli, _decompress_gzip + + assert _decompress_brotli(brotli.compress(b"")) == b"" + assert _decompress_gzip(gzip.compress(b"")) == b"" + + def test_the_encoding_table_uses_the_bounded_helpers(self): + """A default-limit bomb is too slow to build, so pin the wiring instead.""" + from FlightRadarAPI.request import ( + MAX_DECOMPRESSED_BYTES, + APIRequest, + _decompress_brotli, + _decompress_gzip, + ) + + table = getattr(APIRequest, "_APIRequest__content_encodings") + + assert table["br"] is _decompress_brotli + assert table["gzip"] is _decompress_gzip + assert MAX_DECOMPRESSED_BYTES == 64 * 1024 * 1024 From 12ea74dc67b4fda41b7a98f39800a19d7f790ae2 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 22:19:46 -0300 Subject: [PATCH 07/22] ci: pin actions to SHAs, pin docs deps, and enforce the audits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Every action is pinned to a commit SHA. Tags are mutable, so a compromised maintainer account can repoint one and the next run executes it. `pypa/gh-action-pypi-publish@release/v1` was the worst case: a *branch*, in the job holding `id-token: write` for PyPI. - The docs job installed mkdocs unpinned while holding `contents: write`, so a malicious release would run with a token that can push to the repository. Versions now come from a pinned file. - The npm audit never ran. The job defaults into ./nodejs and the step also did `cd nodejs`, so it failed with "No such file or directory" on every run while continue-on-error reported success — which is how the vulnerable undici floor sat unnoticed. Fixed and enforced. - The Python audit now blocks on the dependencies users actually install, resolved in a clean venv, and keeps the dev toolchain informational so a vulnerable linter cannot block every PR. --- .github/workflows/delete-pr-branch.yml | 2 +- .github/workflows/deploy-docs.yml | 10 ++++------ .github/workflows/labeler.yml | 2 +- .github/workflows/lint-pr-title.yml | 2 +- .github/workflows/node-package.yml | 17 ++++++++--------- .github/workflows/publish.yml | 12 ++++++------ .github/workflows/python-package.yml | 26 ++++++++++++++++++-------- docs/requirements.txt | 5 +++++ 8 files changed, 44 insertions(+), 32 deletions(-) create mode 100644 docs/requirements.txt diff --git a/.github/workflows/delete-pr-branch.yml b/.github/workflows/delete-pr-branch.yml index 4d9f0b1..f0d91d5 100644 --- a/.github/workflows/delete-pr-branch.yml +++ b/.github/workflows/delete-pr-branch.yml @@ -13,7 +13,7 @@ jobs: if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest steps: - - uses: actions/github-script@v7 + - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 with: script: | const pr = context.payload.pull_request; diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index b6d1a67..e96f493 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -13,22 +13,20 @@ jobs: deploy: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Configure Git Credentials run: | git config user.name github-actions[bot] git config user.email 41898282+github-actions[bot]@users.noreply.github.com - - uses: actions/setup-python@v5 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: 3.x - run: echo "cache_id=$(date --utc '+%V')" >> $GITHUB_ENV - - uses: actions/cache@v4 + - uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 with: key: mkdocs-material-${{ env.cache_id }} path: .cache restore-keys: | mkdocs-material- - - run: pip install \ - mkdocs-material \ - mkdocs-git-committers-plugin-2 + - run: pip install -r docs/requirements.txt - run: mkdocs gh-deploy --force \ No newline at end of file diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml index b8fdd6e..14719ee 100644 --- a/.github/workflows/labeler.yml +++ b/.github/workflows/labeler.yml @@ -13,7 +13,7 @@ jobs: name: Apply PR labels runs-on: ubuntu-latest steps: - - uses: actions/github-script@v7 + - uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 with: script: | const pr = context.payload.pull_request; diff --git a/.github/workflows/lint-pr-title.yml b/.github/workflows/lint-pr-title.yml index 3735fd2..bef0cee 100644 --- a/.github/workflows/lint-pr-title.yml +++ b/.github/workflows/lint-pr-title.yml @@ -21,7 +21,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Lint PR title - uses: amannn/action-semantic-pull-request@v5 + uses: amannn/action-semantic-pull-request@e32d7e603df1aa1ba07e981f2a23455dee596825 # v5 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: diff --git a/.github/workflows/node-package.yml b/.github/workflows/node-package.yml index 905a59b..c53c804 100644 --- a/.github/workflows/node-package.yml +++ b/.github/workflows/node-package.yml @@ -27,9 +27,9 @@ jobs: matrix: node-version: ['18.x', '20.x', '22.x'] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Set up NodeJS ${{ matrix.node-version }} - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: ${{ matrix.node-version }} - name: Install dependencies @@ -41,16 +41,15 @@ jobs: - name: Offline tests (PR gate) run: npm run test:offline - name: Integration tests (live FR24) - uses: nick-fields/retry@v3 + uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 # v3 with: timeout_minutes: 10 max_attempts: 3 command: cd nodejs && npm run test:integration continue-on-error: ${{ github.event_name == 'push' }} - - name: Dependency audit (informational) - # Informational only: surfaces high-severity advisories as a soft-fail - # without blocking PRs. Re-enable enforcement (drop continue-on-error) - # once the audit baseline is clean. + - name: Dependency audit (shipped dependencies) + # Enforced, and no `cd`: this job already defaults into ./nodejs, so the + # old `cd nodejs` failed every run and continue-on-error hid it, which + # is how the undici floor stayed vulnerable unnoticed. if: matrix.node-version == '22.x' - continue-on-error: true - run: cd nodejs && npm audit --omit=dev --audit-level=high + run: npm audit --omit=dev --audit-level=high diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 681e5b7..ba1732c 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -25,7 +25,7 @@ jobs: verify-versions: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Read declared versions id: versions @@ -61,10 +61,10 @@ jobs: permissions: id-token: write # Required for trusted publishing (OIDC) steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Set up Python - uses: actions/setup-python@v5 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: '3.13' @@ -84,7 +84,7 @@ jobs: - name: Publish to PyPI if: github.event_name == 'release' || inputs.dry_run == false - uses: pypa/gh-action-pypi-publish@release/v1 + uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2 with: packages-dir: python/dist/ @@ -99,10 +99,10 @@ jobs: run: working-directory: ./nodejs steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Set up NodeJS - uses: actions/setup-node@v4 + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: '22.x' registry-url: 'https://registry.npmjs.org' diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 54a6167..dd79eb3 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -23,9 +23,9 @@ jobs: matrix: python-version: ['3.10', '3.11', '3.12', '3.13'] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: ${{ matrix.python-version }} - name: Install dependencies @@ -40,19 +40,29 @@ jobs: - name: Offline tests (PR gate) run: cd python && pytest -m "not integration" --cov=FlightRadarAPI --cov-report=term --cov-report=xml -v - name: Integration tests (live FR24) - uses: nick-fields/retry@v3 + uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 # v3 with: timeout_minutes: 10 max_attempts: 3 command: cd python && pytest -m integration -vv -s continue-on-error: ${{ github.event_name == 'push' }} - - name: Dependency audit (informational) - # Informational only: surfaces vulnerable transitive deps as a soft-fail - # without blocking PRs. Re-enable enforcement (drop continue-on-error) - # once the audit baseline is clean. + - name: Dependency audit (shipped dependencies) + # Enforced, and scoped to what users actually install: a vulnerable dev + # tool must not block every PR, but a vulnerable runtime dep must. + if: matrix.python-version == '3.13' + run: | + python -m venv /tmp/shipenv + /tmp/shipenv/bin/pip install --quiet --upgrade pip + /tmp/shipenv/bin/pip install --quiet ./python + /tmp/shipenv/bin/pip freeze | grep -v "^FlightRadarAPI" > /tmp/shipped.txt + cat /tmp/shipped.txt + pip-audit -r /tmp/shipped.txt + - name: Dependency audit (dev toolchain, informational) if: matrix.python-version == '3.13' continue-on-error: true - run: pip-audit --skip-editable -r <(pip freeze | grep -v "^FlightRadarAPI") + run: | + pip freeze | grep -v "^FlightRadarAPI" > /tmp/devenv.txt + pip-audit -r /tmp/devenv.txt - name: Build and verify install run: | python -m build ./python diff --git a/docs/requirements.txt b/docs/requirements.txt new file mode 100644 index 0000000..1e580f2 --- /dev/null +++ b/docs/requirements.txt @@ -0,0 +1,5 @@ +# Pinned because this workflow runs with `contents: write`: an automatic +# upgrade to a compromised release would execute with a token that can push +# to the repository. Bump deliberately, not implicitly. +mkdocs-material==9.7.7 +mkdocs-git-committers-plugin-2==2.5.0 From 5ea775bcdd97258dab376f10da4ed07c169f5716 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 22:19:46 -0300 Subject: [PATCH 08/22] chore(release): 1.5.4 Both packages, since publish.yml verifies they match. --- nodejs/package-lock.json | 4 ++-- nodejs/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 0af00d5..09c6e86 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,12 +1,12 @@ { "name": "flightradarapi", - "version": "1.5.3", + "version": "1.5.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "flightradarapi", - "version": "1.5.3", + "version": "1.5.4", "license": "MIT", "dependencies": { "node-html-parser": "^6.1.13", diff --git a/nodejs/package.json b/nodejs/package.json index c65f564..f72cc7c 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "flightradarapi", - "version": "1.5.3", + "version": "1.5.4", "description": "SDK for FlightRadar24", "main": "./FlightRadarAPI/index.js", "types": "./FlightRadarAPI/index.d.ts", From 1e158a3eaff1f9f3fe424307cab9f7510462295f Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 22:39:11 -0300 Subject: [PATCH 09/22] ci: deploy docs from the Pages artifact instead of gh-pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pages was switched to build_type=workflow, which stops it rebuilding on a push to the gh-pages branch. The job still ran `mkdocs gh-deploy`, so the next docs change would have pushed the branch, reported success and never appeared on the site — the same silent-success shape as the audit step this branch already fixed. Build the site and hand it to actions/deploy-pages instead. That drops `contents: write` to `contents: read`, so a compromised docs dependency no longer runs with a token that can push to the repository, which was the point of pinning docs/requirements.txt in the first place. Verified `mkdocs build` against the pinned requirements: exits 0 and writes to site/, which is what the upload step expects. Ignored that directory, since the build now runs locally too. --- .github/workflows/deploy-docs.yml | 38 +++++++++++++++++++++++-------- .gitignore | 1 + 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index e96f493..a9f6a64 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -1,4 +1,4 @@ -name: Deploy MkDocs +name: Deploy MkDocs on: push: branches: @@ -7,21 +7,28 @@ on: - 'mkdocs.yml' - 'docs/**' - '.github/workflows/deploy-docs.yml' + +# Pages is served from the Actions artifact, so nothing here needs to push to +# the repository: a compromised docs dependency gets no write access. permissions: - contents: write + contents: read + pages: write + id-token: write + +# One deploy at a time, and never cancel one midway. +concurrency: + group: pages + cancel-in-progress: false + jobs: - deploy: + build: runs-on: ubuntu-latest steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - - name: Configure Git Credentials - run: | - git config user.name github-actions[bot] - git config user.email 41898282+github-actions[bot]@users.noreply.github.com - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: 3.x - - run: echo "cache_id=$(date --utc '+%V')" >> $GITHUB_ENV + - run: echo "cache_id=$(date --utc '+%V')" >> $GITHUB_ENV - uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 with: key: mkdocs-material-${{ env.cache_id }} @@ -29,4 +36,17 @@ jobs: restore-keys: | mkdocs-material- - run: pip install -r docs/requirements.txt - - run: mkdocs gh-deploy --force \ No newline at end of file + - run: mkdocs build + - uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5.0.0 + with: + path: site + + deploy: + needs: build + runs-on: ubuntu-latest + environment: + name: github-pages + url: ${{ steps.deployment.outputs.page_url }} + steps: + - id: deployment + uses: actions/deploy-pages@cd2ce8fcbc39b97be8ca5fce6e763baed58fa128 # v5.0.0 diff --git a/.gitignore b/.gitignore index c581bf5..c1736d8 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ venv/ .mypy_cache/ .pytest_cache/ htmlcov/ +site/ .coverage coverage.xml From 66bd319ee122daeb47907fd03d715b241a5d040b Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 23:06:28 -0300 Subject: [PATCH 10/22] fix: reject truncated bodies and handle BOM after the budget change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the budget work found that guarding size had quietly traded away integrity. `_decompress_gzip` returned whatever it had inflated: a body cut to 3/4 of its compressed bytes came back as 76 KB of partial JSON with no error, where gzip.decompress raises EOFError. `_decompress_brotli` did the same, returning 0 bytes. Both now raise when the stream ends mid-message. That also restores a fallback this had broken. get_content() reads a raised error as "curl_cffi already decoded this" and returns the raw bytes; since _decompress_brotli(b'[]') returned b"" instead of raising, an already-decoded body became empty and json.loads blew up on it. `_decompress_gzip` also stopped at the first member, so a concatenated gzip body — legal Content-Encoding, emitted by some proxies — silently lost everything after it. It now drains unused_data. On the Node side, Response.json()/text() ran the spec UTF-8 decode, which strips a byte-order mark; reading the body as a Buffer does not, so a BOM-prefixed JSON body started failing to parse. Stripped by hand. Also moved the docs pin file out of docs_dir: mkdocs copies non-page files into the built site, so it was being published at /requirements.txt. --- .../docs-requirements.txt | 0 .github/workflows/deploy-docs.yml | 3 +- nodejs/FlightRadarAPI/request.js | 18 ++++++- nodejs/tests/testRequestTransport.js | 47 ++++++++++++++++ python/FlightRadarAPI/request.py | 40 ++++++++++---- python/tests/test_request_transport.py | 53 +++++++++++++++++++ 6 files changed, 148 insertions(+), 13 deletions(-) rename docs/requirements.txt => .github/docs-requirements.txt (100%) diff --git a/docs/requirements.txt b/.github/docs-requirements.txt similarity index 100% rename from docs/requirements.txt rename to .github/docs-requirements.txt diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index a9f6a64..aa241c5 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -6,6 +6,7 @@ on: paths: - 'mkdocs.yml' - 'docs/**' + - '.github/docs-requirements.txt' - '.github/workflows/deploy-docs.yml' # Pages is served from the Actions artifact, so nothing here needs to push to @@ -35,7 +36,7 @@ jobs: path: .cache restore-keys: | mkdocs-material- - - run: pip install -r docs/requirements.txt + - run: pip install -r .github/docs-requirements.txt - run: mkdocs build - uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9 # v5.0.0 with: diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index 807be53..7346a0f 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -144,6 +144,20 @@ const DEFAULT_TIMEOUT_MS = 30_000; // FR24's largest payload (the airports feed) is orders of magnitude under this. const MAX_RESPONSE_BYTES = 64 * 1024 * 1024; +/** + * Decode a body as UTF-8 text, dropping a leading byte-order mark. + * + * `Response.text()` ran the spec's UTF-8 decode, which strips the BOM; + * `Buffer.toString` does not, and a BOM left in place breaks `JSON.parse`. + * + * @param {Buffer} body + * @return {string} + */ +function decodeText(body) { + const text = body.toString("utf-8"); + return text.charCodeAt(0) === 0xFEFF ? text.slice(1) : text; +} + /** * Read a response body, refusing one that grows past `limit`. * @@ -384,10 +398,10 @@ async function request(url, { let content; if (contentType.includes("application/json")) { - content = JSON.parse(body.toString("utf-8")); + content = JSON.parse(decodeText(body)); } else if (contentType.includes("text")) { - content = body.toString("utf-8"); + content = decodeText(body); } else { content = body.buffer.slice(body.byteOffset, body.byteOffset + body.byteLength); diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index 770c02d..5e2365d 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -483,3 +483,50 @@ describe("Response size budget (offline)", function() { expect(MAX_RESPONSE_BYTES).to.equal(64 * 1024 * 1024); }); }); + + +describe("Byte-order mark handling (offline)", function() { + // Response.json()/text() ran the spec UTF-8 decode, which strips a BOM. + // Reading the body as a Buffer does not, so this has to be done by hand. + const BOM = Buffer.from([0xEF, 0xBB, 0xBF]); + + let mockAgent; + let mockPool; + + beforeEach(function() { + mockAgent = new MockAgent(); + mockAgent.disableNetConnect(); + mockPool = mockAgent.get("https://example.com"); + }); + + afterEach(async function() { + await mockAgent.close(); + }); + + it("parses JSON that starts with a BOM", async function() { + mockPool.intercept({ path: "/bom.json" }) + .reply(200, Buffer.concat([BOM, Buffer.from(JSON.stringify({ ok: true }))]), + { headers: { "content-type": "application/json" } }); + + const { content } = await request("https://example.com/bom.json", { dispatcher: mockAgent }); + expect(content).to.deep.equal({ ok: true }); + }); + + it("strips a BOM from text responses", async function() { + mockPool.intercept({ path: "/bom.txt" }) + .reply(200, Buffer.concat([BOM, Buffer.from("hello")]), + { headers: { "content-type": "text/plain" } }); + + const { content } = await request("https://example.com/bom.txt", { dispatcher: mockAgent }); + expect(content).to.equal("hello"); + }); + + it("leaves binary bodies byte-for-byte intact", async function() { + mockPool.intercept({ path: "/bom.bin" }) + .reply(200, Buffer.concat([BOM, Buffer.from([1, 2])]), + { headers: { "content-type": "image/png" } }); + + const { content } = await request("https://example.com/bom.bin", { dispatcher: mockAgent }); + expect([...Buffer.from(content)]).to.deep.equal([0xEF, 0xBB, 0xBF, 1, 2]); + }); +}); diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index f656c86..be6349a 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -26,15 +26,31 @@ def _decompress_gzip(data: bytes, limit: int = MAX_DECOMPRESSED_BYTES) -> bytes: - """Inflate gzip bytes, refusing a body that expands past ``limit``.""" - decompressor = zlib.decompressobj(_GZIP_WBITS) - output = decompressor.decompress(data, limit + 1) - - # Output stops at max_length, so anything left over means the body did not fit. - if len(output) > limit or decompressor.unconsumed_tail: - raise DecompressionLimitError( - f"gzip body expands past the {limit} byte decompression limit." - ) + """Inflate gzip bytes, refusing a body that expands past ``limit``. + + A short read must raise rather than return what arrived: the caller cannot + tell truncated JSON from a malformed feed, and `get_content` treats a raised + error as "the transport already decoded this" and hands back the raw bytes. + """ + output = b"" + remaining = data + + while remaining: + # A new object per member: `Content-Encoding: gzip` may carry several, + # and one decompressor stops at the first trailer. + decompressor = zlib.decompressobj(_GZIP_WBITS) + output += decompressor.decompress(remaining, limit + 1 - len(output)) + + if len(output) > limit or decompressor.unconsumed_tail: + raise DecompressionLimitError( + f"gzip body expands past the {limit} byte decompression limit." + ) + + if not decompressor.eof: + raise zlib.error("gzip stream ended mid-member") + + remaining = decompressor.unused_data + return output @@ -66,10 +82,14 @@ def _decompress_brotli(data: bytes, limit: int = MAX_DECOMPRESSED_BYTES) -> byte f"brotli body expands past the {limit} byte decompression limit." ) - # No output left and still hungry: the stream ended mid-message. + # No output left and still hungry: nothing more will come. if not piece and decompressor.can_accept_more_data(): break + # Same contract as the gzip helper: a partial body must not pass as whole. + if not decompressor.is_finished(): + raise brotli.error("brotli stream ended mid-message") + return bytes(output) diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index fae1849..ec80861 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -197,3 +197,56 @@ def test_the_encoding_table_uses_the_bounded_helpers(self): assert table["br"] is _decompress_brotli assert table["gzip"] is _decompress_gzip assert MAX_DECOMPRESSED_BYTES == 64 * 1024 * 1024 + + +class TestDecompressionIntegrity: + """A partial body must not pass as a whole one.""" + + BODY = b'{"rows": [' + b'{"name": "Guarulhos"},' * 500 + b'{}]}' + + def test_a_truncated_gzip_body_raises(self): + import gzip + + from FlightRadarAPI.request import _decompress_gzip + + blob = gzip.compress(self.BODY) + + with pytest.raises(Exception) as excinfo: + _decompress_gzip(blob[: len(blob) * 3 // 4]) + + # Not the budget error: this body was short, not oversized. + assert "limit" not in str(excinfo.value) + + def test_a_truncated_brotli_body_raises(self): + import brotli + + from FlightRadarAPI.request import _decompress_brotli + + blob = brotli.compress(self.BODY) + + with pytest.raises(Exception) as excinfo: + _decompress_brotli(blob[: len(blob) * 3 // 4]) + + assert "limit" not in str(excinfo.value) + + def test_gzip_reads_every_member(self): + """Concatenated members are legal Content-Encoding: gzip.""" + import gzip + + from FlightRadarAPI.request import _decompress_gzip + + assert _decompress_gzip(gzip.compress(b"first") + gzip.compress(b"second")) == b"firstsecond" + + @pytest.mark.parametrize("encoding", ["br", "gzip"]) + def test_an_already_decoded_body_still_reaches_the_transport_fallback(self, encoding): + """curl_cffi may decompress for us, leaving plain bytes under a br/gzip header. + + get_content() reads a raised error as "already decoded" and returns the + raw bytes, so these helpers must raise rather than quietly return b"". + """ + from FlightRadarAPI.request import _decompress_brotli, _decompress_gzip + + decompress = {"br": _decompress_brotli, "gzip": _decompress_gzip}[encoding] + + with pytest.raises(Exception): + decompress(b'{"already": "json"}') From ff4850dccbe324e428b8d98988f2745a7803c1a5 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 23:36:53 -0300 Subject: [PATCH 11/22] fix(python): stream the body so the size budget actually applies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The budget added earlier never ran in production. curl_cffi decompresses in the transport, so by the time `.content` exists the body has already expanded: `_decompress_brotli` was handed already-decoded bytes, raised, and fell through to the "transport already decoded" path. Measured against a local server — 27 compressed bytes arrived as 200 008 — and against FR24 itself, where every real request logs that fallback at DEBUG. The tests passed only because they called the helpers directly with compressed input, which is a state production never reaches. Read the response with `stream=True` and `iter_content()` instead, and apply the budget to the chunks as they arrive. A 512 MB bomb from 841 bytes on the wire is now refused with no growth in RSS, which is what the Node port already did by streaming `response.body`. Also closes the two smaller gaps from the same review: - `max_response_bytes` is now a per-request argument, matching Node's `maxResponseBytes`; it was a module constant with no way to override. - `get_content()` handling an already-decoded body under a live `Content-Encoding` header — the path every FR24 response takes — had no test. It does now, along with one asserting the response is closed when the body is refused. Renamed to MAX_RESPONSE_BYTES for parity with the Node constant, since the budget covers the response, not just what this code decompresses. --- python/FlightRadarAPI/request.py | 72 ++++++++++++++++++------- python/tests/_request_doubles.py | 13 ++++- python/tests/test_request_transport.py | 73 ++++++++++++++++++++++---- 3 files changed, 125 insertions(+), 33 deletions(-) diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index be6349a..0f22381 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -19,13 +19,15 @@ DEFAULT_IMPERSONATE = "chrome136" # A compressed body is trusted only as far as its expanded size: brotli reaches -# ratios high enough to exhaust memory from a few kilobytes on the wire. FR24's -# largest payload (the airports feed) is orders of magnitude under this. -MAX_DECOMPRESSED_BYTES = 64 * 1024 * 1024 +# ratios high enough to exhaust memory from a few kilobytes on the wire. The +# body is streamed against this budget, because curl_cffi decompresses in the +# transport — by the time a buffered `.content` exists, the bomb has already +# gone off. FR24's largest payload (the airports feed) is far under this. +MAX_RESPONSE_BYTES = 64 * 1024 * 1024 _GZIP_WBITS = 31 # 16 + MAX_WBITS: gzip wrapper rather than raw deflate -def _decompress_gzip(data: bytes, limit: int = MAX_DECOMPRESSED_BYTES) -> bytes: +def _decompress_gzip(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: """Inflate gzip bytes, refusing a body that expands past ``limit``. A short read must raise rather than return what arrived: the caller cannot @@ -54,7 +56,7 @@ def _decompress_gzip(data: bytes, limit: int = MAX_DECOMPRESSED_BYTES) -> bytes: return output -def _decompress_brotli(data: bytes, limit: int = MAX_DECOMPRESSED_BYTES) -> bytes: +def _decompress_brotli(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: """Decompress brotli bytes, refusing a body that expands past ``limit``. ``process`` takes a max-output argument, so the cap is enforced by the @@ -229,6 +231,7 @@ def __init__( data: Optional[Dict] = None, allowed_error_codes: Optional[List[int]] = None, impersonate: str = DEFAULT_IMPERSONATE, + max_response_bytes: int = MAX_RESPONSE_BYTES, ): """ Constructor of the APIRequest class. @@ -240,33 +243,62 @@ def __init__( :param data: data for the request. If "data" is None, request will be a GET. Otherwise, it will be a POST :param allowed_error_codes: status codes that should not raise an error :param impersonate: curl_cffi browser profile (only used when no session is provided) + :param max_response_bytes: reject a body that streams past this many bytes """ self.url = url + self.__max_response_bytes = max_response_bytes if params: url += "?" + urlencode(params) + # Streamed, so the budget below can stop a decompression bomb while it + # is still arriving. A buffered read would expand it in full first. if session is not None: request_method = session.get if data is None else session.post - self.__response = request_method(url, headers=headers, data=data, timeout=timeout) + self.__response = request_method(url, headers=headers, data=data, timeout=timeout, stream=True) else: request_method = requests.get if data is None else requests.post self.__response = request_method( - url, headers=headers, data=data, timeout=timeout, + url, headers=headers, data=data, timeout=timeout, stream=True, impersonate=impersonate # type: ignore[arg-type] ) - # Cloudflare detection only when the caller did not opt-in to this status code. - # `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. - if (self.get_status_code() not in (allowed_error_codes or []) - and self.__is_cloudflare_block()): - raise CloudflareError( - message="Blocked by Cloudflare. Perhaps you are making too many calls, " - "or the TLS impersonation needs to be updated.", - response=self.__response - ) - - if self.get_status_code() not in (allowed_error_codes or []): - self.__response.raise_for_status() + try: + # Cloudflare detection only when the caller did not opt-in to this status code. + # `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. + if (self.get_status_code() not in (allowed_error_codes or []) + and self.__is_cloudflare_block()): + raise CloudflareError( + message="Blocked by Cloudflare. Perhaps you are making too many calls, " + "or the TLS impersonation needs to be updated.", + response=self.__response + ) + + if self.get_status_code() not in (allowed_error_codes or []): + self.__response.raise_for_status() + + self.__content = self.__read_bounded_body() + finally: + close = getattr(self.__response, "close", None) + + if close is not None: + close() + + def __read_bounded_body(self) -> bytes: + """Collect the response body, refusing one that streams past the budget.""" + limit = self.__max_response_bytes + chunks: List[bytes] = [] + total = 0 + + for chunk in self.__response.iter_content(): + total += len(chunk) + + if total > limit: + raise DecompressionLimitError( + f"Response body from {self.url} exceeds the {limit} byte limit." + ) + chunks.append(chunk) + + return b"".join(chunks) def __is_cloudflare_block(self) -> bool: """ @@ -294,7 +326,7 @@ def get_content(self) -> Union[Dict, bytes]: """ Return the received content from the request. """ - content = self.__response.content + content = self.__content content_encoding = self.__response.headers.get("Content-Encoding", "") content_type = self.__response.headers.get("Content-Type", "") diff --git a/python/tests/_request_doubles.py b/python/tests/_request_doubles.py index 8d79f51..19f494c 100644 --- a/python/tests/_request_doubles.py +++ b/python/tests/_request_doubles.py @@ -47,8 +47,9 @@ def __contains__(self, key: object) -> bool: class FakeResponse: """Minimal stand-in for a curl_cffi response object. - ``APIRequest`` only touches ``.status_code``, ``.headers``, ``.content``, - and ``.raise_for_status()`` — that's all we have to implement. + ``APIRequest`` only touches ``.status_code``, ``.headers``, + ``.iter_content()``, ``.close()`` and ``.raise_for_status()`` — that's all + we have to implement. """ def __init__( @@ -66,6 +67,14 @@ def raise_for_status(self) -> None: if 400 <= self.status_code < 600: raise RuntimeError(f"HTTP {self.status_code}") + def iter_content(self, chunk_size: int = 8192): + """Yield the body in chunks, as the streaming transport does.""" + for start in range(0, len(self.content), chunk_size): + yield self.content[start:start + chunk_size] + + def close(self) -> None: + self.closed = True + class StubSession: """Session double whose ``.get`` / ``.post`` return a pre-baked response. diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index ec80861..5434443 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -183,20 +183,71 @@ def test_an_empty_body_is_not_mistaken_for_a_bomb(self): assert _decompress_brotli(brotli.compress(b"")) == b"" assert _decompress_gzip(gzip.compress(b"")) == b"" - def test_the_encoding_table_uses_the_bounded_helpers(self): - """A default-limit bomb is too slow to build, so pin the wiring instead.""" - from FlightRadarAPI.request import ( - MAX_DECOMPRESSED_BYTES, - APIRequest, - _decompress_brotli, - _decompress_gzip, + def test_the_budget_is_enforced_on_the_streamed_body(self): + """The budget that matters is the one on the wire. + + curl_cffi decompresses in the transport, so a helper that inspects + `.content` never sees a bomb before it has already expanded. Only the + streaming read can refuse one, so that is what this pins. + """ + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import MAX_RESPONSE_BYTES, APIRequest + + session = StubSession(FakeResponse( + status_code=200, + headers={"Content-Type": "application/json"}, + content=b"x" * 5000, + )) + + with pytest.raises(DecompressionLimitError): + APIRequest("https://x.test/", session=session, max_response_bytes=1024) # type: ignore[arg-type] + + assert MAX_RESPONSE_BYTES == 64 * 1024 * 1024 + + def test_a_body_at_the_budget_is_accepted(self): + from FlightRadarAPI.request import APIRequest + + session = StubSession(FakeResponse( + status_code=200, + headers={"Content-Type": "application/octet-stream"}, + content=b"x" * 1024, + )) + request = APIRequest("https://x.test/", session=session, max_response_bytes=1024) # type: ignore[arg-type] + + assert request.get_bytes_content() == b"x" * 1024 + + def test_an_already_decoded_body_survives_the_encoding_header(self): + """curl_cffi decompresses for us, so `Content-Encoding` outlives the encoding. + + get_content() must hand back the decoded bytes rather than failing on + them — this is the path every real FR24 response takes. + """ + from FlightRadarAPI.request import APIRequest + + session = StubSession(FakeResponse( + status_code=200, + headers={"Content-Type": "application/json", "Content-Encoding": "gzip"}, + content=b'{"rows": []}', + )) + request = APIRequest("https://x.test/", session=session) # type: ignore[arg-type] + + assert request.get_json_content() == {"rows": []} + + def test_the_response_is_closed_even_when_the_body_is_refused(self): + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import APIRequest + + response = FakeResponse( + status_code=200, + headers={"Content-Type": "application/json"}, + content=b"x" * 5000, ) + session = StubSession(response) - table = getattr(APIRequest, "_APIRequest__content_encodings") + with pytest.raises(DecompressionLimitError): + APIRequest("https://x.test/", session=session, max_response_bytes=10) # type: ignore[arg-type] - assert table["br"] is _decompress_brotli - assert table["gzip"] is _decompress_gzip - assert MAX_DECOMPRESSED_BYTES == 64 * 1024 * 1024 + assert getattr(response, "closed", False) is True class TestDecompressionIntegrity: From 8046fb1f3cd20bac5737ce6db8c243e25bc005a0 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Wed, 19 Aug 2026 23:57:31 -0300 Subject: [PATCH 12/22] revert(python): stop streaming the body; the cure cost more than the bomb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Streaming was the wrong call, and the measurements say so plainly. In curl_cffi 0.16, `stream=True`: - Turns `timeout` from a wall-clock cap into a ">=1 byte/sec" liveness check. A tarpit trickling one byte every 400ms held a `timeout=2` request for 23.8s and would hold it indefinitely — including every worker in the get_flight_details fan-out. That is the same denial-of-service the byte budget exists to prevent. - Stops the session reusing connections: 5 requests opened 5 TCP+TLS connections instead of 1. A fresh handshake per request is precisely the fingerprint Cloudflare bot management scores against, which undermines the TLS impersonation this library is built around. - Leaves `Response.content` empty, so `get_response_object()` and `CloudflareError.response` — both public — silently return nothing. The challenge body is the first thing anyone inspects when debugging a block. Reverted to a buffered read. Confirmed restored: timeout raises at 2.0s, 5 requests share 1 connection, and the response body is readable again. The budget now bounds what reaches the parser rather than what the transport allocates. It cannot undo the peak — libcurl expanded the body before we saw it — but it does stop the larger second cost of parsing a body that size into Python objects. The module note states that limit honestly instead of implying the Node port's guarantee, and a test pins the no-streaming decision so the trade-off cannot be undone by accident. Also validates max_response_bytes, which turned a typo into a blanket failure on every response, and restores the encoding-table wiring test. --- python/FlightRadarAPI/request.py | 87 ++++++++++++-------------- python/tests/test_request_transport.py | 68 +++++++++++--------- 2 files changed, 79 insertions(+), 76 deletions(-) diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index 0f22381..86039c9 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -19,10 +19,18 @@ DEFAULT_IMPERSONATE = "chrome136" # A compressed body is trusted only as far as its expanded size: brotli reaches -# ratios high enough to exhaust memory from a few kilobytes on the wire. The -# body is streamed against this budget, because curl_cffi decompresses in the -# transport — by the time a buffered `.content` exists, the bomb has already -# gone off. FR24's largest payload (the airports feed) is far under this. +# ratios high enough to exhaust memory from a few kilobytes on the wire. +# +# The reach of this budget differs from the Node port's, and not by choice. +# curl_cffi decompresses inside libcurl, so a buffered read hands us a body +# that has already expanded. Intercepting earlier means `stream=True`, which in +# curl_cffi 0.16 costs two things measured to be worse than the bomb it would +# stop: `timeout` degrades from a wall-clock cap to a >=1 byte/sec liveness +# check (a tarpit held a `timeout=2` request for 23.8s), and the session stops +# reusing connections (5 handshakes across 5 requests instead of 1), which is +# the very fingerprint the TLS impersonation exists to avoid. So here the +# budget bounds what reaches the parser, and the helpers below bound the cases +# where the transport hands back bytes it did not decode. MAX_RESPONSE_BYTES = 64 * 1024 * 1024 _GZIP_WBITS = 31 # 16 + MAX_WBITS: gzip wrapper rather than raw deflate @@ -245,60 +253,47 @@ def __init__( :param impersonate: curl_cffi browser profile (only used when no session is provided) :param max_response_bytes: reject a body that streams past this many bytes """ + if max_response_bytes < 1: + raise ValueError("max_response_bytes must be >= 1") + self.url = url - self.__max_response_bytes = max_response_bytes if params: url += "?" + urlencode(params) - # Streamed, so the budget below can stop a decompression bomb while it - # is still arriving. A buffered read would expand it in full first. if session is not None: request_method = session.get if data is None else session.post - self.__response = request_method(url, headers=headers, data=data, timeout=timeout, stream=True) + self.__response = request_method(url, headers=headers, data=data, timeout=timeout) else: request_method = requests.get if data is None else requests.post self.__response = request_method( - url, headers=headers, data=data, timeout=timeout, stream=True, + url, headers=headers, data=data, timeout=timeout, impersonate=impersonate # type: ignore[arg-type] ) - try: - # Cloudflare detection only when the caller did not opt-in to this status code. - # `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. - if (self.get_status_code() not in (allowed_error_codes or []) - and self.__is_cloudflare_block()): - raise CloudflareError( - message="Blocked by Cloudflare. Perhaps you are making too many calls, " - "or the TLS impersonation needs to be updated.", - response=self.__response - ) - - if self.get_status_code() not in (allowed_error_codes or []): - self.__response.raise_for_status() - - self.__content = self.__read_bounded_body() - finally: - close = getattr(self.__response, "close", None) - - if close is not None: - close() - - def __read_bounded_body(self) -> bytes: - """Collect the response body, refusing one that streams past the budget.""" - limit = self.__max_response_bytes - chunks: List[bytes] = [] - total = 0 - - for chunk in self.__response.iter_content(): - total += len(chunk) - - if total > limit: - raise DecompressionLimitError( - f"Response body from {self.url} exceeds the {limit} byte limit." - ) - chunks.append(chunk) - - return b"".join(chunks) + # Cloudflare detection only when the caller did not opt-in to this status code. + # `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. + if (self.get_status_code() not in (allowed_error_codes or []) + and self.__is_cloudflare_block()): + raise CloudflareError( + message="Blocked by Cloudflare. Perhaps you are making too many calls, " + "or the TLS impersonation needs to be updated.", + response=self.__response + ) + + if self.get_status_code() not in (allowed_error_codes or []): + self.__response.raise_for_status() + + self.__content = self.__response.content + + # The transport already expanded this, so the check cannot undo the peak. + # What it does stop is the far larger second cost: parsing a body of that + # size into Python objects. See the module note on why streaming — the + # only way to intervene earlier — is not used here. + if len(self.__content) > max_response_bytes: + raise DecompressionLimitError( + f"Response body from {self.url} is {len(self.__content)} bytes, " + f"past the {max_response_bytes} byte limit." + ) def __is_cloudflare_block(self) -> bool: """ diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index 5434443..ad0c883 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -183,15 +183,9 @@ def test_an_empty_body_is_not_mistaken_for_a_bomb(self): assert _decompress_brotli(brotli.compress(b"")) == b"" assert _decompress_gzip(gzip.compress(b"")) == b"" - def test_the_budget_is_enforced_on_the_streamed_body(self): - """The budget that matters is the one on the wire. - - curl_cffi decompresses in the transport, so a helper that inspects - `.content` never sees a bomb before it has already expanded. Only the - streaming read can refuse one, so that is what this pins. - """ + def test_a_body_past_the_budget_is_refused(self): from FlightRadarAPI.errors import DecompressionLimitError - from FlightRadarAPI.request import MAX_RESPONSE_BYTES, APIRequest + from FlightRadarAPI.request import MAX_RESPONSE_BYTES session = StubSession(FakeResponse( status_code=200, @@ -205,8 +199,6 @@ def test_the_budget_is_enforced_on_the_streamed_body(self): assert MAX_RESPONSE_BYTES == 64 * 1024 * 1024 def test_a_body_at_the_budget_is_accepted(self): - from FlightRadarAPI.request import APIRequest - session = StubSession(FakeResponse( status_code=200, headers={"Content-Type": "application/octet-stream"}, @@ -216,38 +208,54 @@ def test_a_body_at_the_budget_is_accepted(self): assert request.get_bytes_content() == b"x" * 1024 - def test_an_already_decoded_body_survives_the_encoding_header(self): - """curl_cffi decompresses for us, so `Content-Encoding` outlives the encoding. + def test_a_nonsensical_budget_is_rejected_at_the_call(self): + """Better than turning every response into a confusing limit error.""" + session = StubSession(FakeResponse(status_code=200, content=b"ok")) - get_content() must hand back the decoded bytes rather than failing on - them — this is the path every real FR24 response takes. - """ - from FlightRadarAPI.request import APIRequest + for bad in (0, -1): + with pytest.raises(ValueError): + APIRequest("https://x.test/", session=session, max_response_bytes=bad) # type: ignore[arg-type] + + def test_the_body_is_not_requested_as_a_stream(self): + """Pins a deliberate trade-off, not an oversight. + `stream=True` would let the budget act before the body expands, but in + curl_cffi 0.16 it degrades `timeout` from a wall-clock cap to a + >=1 byte/sec liveness check, and stops the session reusing connections + — a fresh TLS handshake per request, which is the fingerprint the + impersonation exists to avoid. Measured both before choosing. + """ session = StubSession(FakeResponse( status_code=200, - headers={"Content-Type": "application/json", "Content-Encoding": "gzip"}, - content=b'{"rows": []}', + headers={"Content-Type": "application/json"}, + content=b"{}", )) - request = APIRequest("https://x.test/", session=session) # type: ignore[arg-type] + APIRequest("https://x.test/", session=session) # type: ignore[arg-type] - assert request.get_json_content() == {"rows": []} + assert session.calls[0].get("stream") is None - def test_the_response_is_closed_even_when_the_body_is_refused(self): - from FlightRadarAPI.errors import DecompressionLimitError - from FlightRadarAPI.request import APIRequest + def test_the_response_body_stays_readable_for_callers(self): + """get_response_object() and CloudflareError.response are public. - response = FakeResponse( + A streamed response leaves `.content` empty, so anyone inspecting a + Cloudflare challenge body would get nothing back. + """ + session = StubSession(FakeResponse( status_code=200, headers={"Content-Type": "application/json"}, - content=b"x" * 5000, - ) - session = StubSession(response) + content=b'{"a": 1}', + )) + request = APIRequest("https://x.test/", session=session) # type: ignore[arg-type] - with pytest.raises(DecompressionLimitError): - APIRequest("https://x.test/", session=session, max_response_bytes=10) # type: ignore[arg-type] + assert request.get_response_object().content == b'{"a": 1}' + + def test_the_encoding_table_uses_the_bounded_helpers(self): + from FlightRadarAPI.request import _decompress_brotli, _decompress_gzip + + table = getattr(APIRequest, "_APIRequest__content_encodings") - assert getattr(response, "closed", False) is True + assert table["br"] is _decompress_brotli + assert table["gzip"] is _decompress_gzip class TestDecompressionIntegrity: From e6f374748370a1c1b42746b2ab232faee214784d Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 00:13:16 -0300 Subject: [PATCH 13/22] fix(python): take content decoding from libcurl so the budget is real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The budget only ever bounded what reached the parser: libcurl decoded the body first, so the peak had already happened. Disabling CURLOPT_HTTP_CONTENT_DECODING hands this module the compressed bytes instead, which puts the cap back inside the decompressor — the guarantee the Node port has — while keeping the buffered read that a previous attempt at this had to give up. Measured end to end: a 512 MB bomb from 841 bytes on the wire is refused with RSS growing by the budget, and a 2 GB bomb costs 6 MB more than the 512 MB one, so the cost tracks the limit rather than the payload. `timeout` stays a wall-clock cap (2.0s) and the session still reuses connections (1 for 5 requests) — the two things streaming cost. The option is applied per request, not once at construction. Set once it works exactly once: `Session.request` resets the handle, so requests 2..n arrive pre-expanded with no budget in reach. Every double-based test passed through that because each built a fresh client; there is now one asserting the option is set on all three of three requests, and the local-server tests reuse a client so the lapse cannot come back quietly. Owning the decoding means decoding everything `Accept-Encoding` advertises, so `deflate` is implemented rather than left to a transport that no longer does it — `Core.html_headers` asks for it, and get_airlines() would have been served raw bytes. Both shapes are accepted: RFC 9110 says zlib-wrapped, plenty of servers send raw. Real FR24 traffic now decodes here instead of falling through the "transport already decoded" path: 0 fallbacks across 4 calls, where before every single request took it. That also makes the truncation and budget checks in those helpers live code rather than a dead branch with passing tests. --- python/FlightRadarAPI/request.py | 87 +++++++++++---- python/tests/_request_doubles.py | 15 +++ python/tests/test_request_transport.py | 145 +++++++++++++++++++++++++ 3 files changed, 224 insertions(+), 23 deletions(-) diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index 86039c9..06cd273 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -9,7 +9,7 @@ from urllib.parse import urlencode import brotli -from curl_cffi import requests +from curl_cffi import CurlOpt, requests from curl_cffi.requests import Session from .errors import CloudflareError, DecompressionLimitError @@ -21,20 +21,60 @@ # A compressed body is trusted only as far as its expanded size: brotli reaches # ratios high enough to exhaust memory from a few kilobytes on the wire. # -# The reach of this budget differs from the Node port's, and not by choice. -# curl_cffi decompresses inside libcurl, so a buffered read hands us a body -# that has already expanded. Intercepting earlier means `stream=True`, which in -# curl_cffi 0.16 costs two things measured to be worse than the bomb it would -# stop: `timeout` degrades from a wall-clock cap to a >=1 byte/sec liveness -# check (a tarpit held a `timeout=2` request for 23.8s), and the session stops -# reusing connections (5 handshakes across 5 requests instead of 1), which is -# the very fingerprint the TLS impersonation exists to avoid. So here the -# budget bounds what reaches the parser, and the helpers below bound the cases -# where the transport hands back bytes it did not decode. +# Enforcing that needs the compressed bytes, which means taking content decoding +# away from libcurl (see `_new_curl_handle`): left to itself it expands the body +# before any of this code runs, and the only other way to intervene — +# `stream=True` — was measured to cost more than the bomb it stops (`timeout` +# degrades to a >=1 byte/sec liveness check, and connection reuse disappears). +# Owning the decoding is why `deflate` is implemented below rather than left to +# the transport: whatever `accept-encoding` advertises, this module must decode. MAX_RESPONSE_BYTES = 64 * 1024 * 1024 +_ZLIB_WBITS = 15 # zlib-wrapped deflate, as RFC 9110 specifies +_RAW_DEFLATE_WBITS = -15 # raw deflate, as many servers actually send _GZIP_WBITS = 31 # 16 + MAX_WBITS: gzip wrapper rather than raw deflate +def _keep_body_encoded(session: Session) -> None: + """Stop libcurl decompressing this session's next response. + + libcurl decompresses transparently, which would expand a bomb before this + module ever sees it. With decoding off the compressed bytes arrive intact + and the helpers below can enforce a real budget. `Accept-Encoding` is still + sent, so responses stay compressed on the wire. + + Applied per request, not once at construction: `Session.request` resets the + handle, so an option set in the constructor survives the first call and + silently lapses on every one after it. Setting it here also leaves + curl_cffi's thread-local handles in place. + """ + session.curl.setopt(CurlOpt.HTTP_CONTENT_DECODING, 0) + + +def _decompress_deflate(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: + """Inflate a deflate body in either shape it arrives in. + + RFC 9110 says zlib-wrapped; plenty of servers send raw. libcurl accepted + both, so taking decoding over means accepting both too. + """ + for wbits in (_ZLIB_WBITS, _RAW_DEFLATE_WBITS): + decompressor = zlib.decompressobj(wbits) + + try: + output = decompressor.decompress(data, limit + 1) + except zlib.error: + continue + + if len(output) > limit or decompressor.unconsumed_tail: + raise DecompressionLimitError( + f"deflate body expands past the {limit} byte decompression limit." + ) + + if decompressor.eof: + return output + + raise zlib.error("body is not a complete deflate stream") + + def _decompress_gzip(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: """Inflate gzip bytes, refusing a body that expands past ``limit``. @@ -223,9 +263,10 @@ class APIRequest: Class to make requests to the FlightRadar24. """ __content_encodings = { - "": lambda x: x, + "": lambda data, limit: data, "br": _decompress_brotli, - "gzip": _decompress_gzip + "gzip": _decompress_gzip, + "deflate": _decompress_deflate } def __init__( @@ -257,18 +298,21 @@ def __init__( raise ValueError("max_response_bytes must be >= 1") self.url = url + self.__max_response_bytes = max_response_bytes if params: url += "?" + urlencode(params) if session is not None: + _keep_body_encoded(session) request_method = session.get if data is None else session.post self.__response = request_method(url, headers=headers, data=data, timeout=timeout) else: - request_method = requests.get if data is None else requests.post - self.__response = request_method( - url, headers=headers, data=data, timeout=timeout, - impersonate=impersonate # type: ignore[arg-type] - ) + # A throwaway session rather than the module-level helpers, whose + # internal handle this cannot reach. + with Session(impersonate=impersonate) as standalone: # type: ignore[arg-type] + _keep_body_encoded(standalone) + request_method = standalone.get if data is None else standalone.post + self.__response = request_method(url, headers=headers, data=data, timeout=timeout) # Cloudflare detection only when the caller did not opt-in to this status code. # `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. @@ -285,10 +329,7 @@ def __init__( self.__content = self.__response.content - # The transport already expanded this, so the check cannot undo the peak. - # What it does stop is the far larger second cost: parsing a body of that - # size into Python objects. See the module note on why streaming — the - # only way to intervene earlier — is not used here. + # Bounds the body as received; the decompressors bound its expansion. if len(self.__content) > max_response_bytes: raise DecompressionLimitError( f"Response body from {self.url} is {len(self.__content)} bytes, " @@ -332,7 +373,7 @@ def get_content(self) -> Union[Dict, bytes]: # surfacing an error the caller cannot act on. decode = self.__content_encodings.get(content_encoding, self.__content_encodings[""]) try: - content = decode(content) + content = decode(content, self.__max_response_bytes) except DecompressionLimitError: raise except Exception as err: diff --git a/python/tests/_request_doubles.py b/python/tests/_request_doubles.py index 19f494c..1681f23 100644 --- a/python/tests/_request_doubles.py +++ b/python/tests/_request_doubles.py @@ -76,6 +76,20 @@ def close(self) -> None: self.closed = True +class FakeCurl: + """Curl handle double that records the options set on it. + + ``APIRequest`` disables content decoding per request; recording the calls is + what lets a test prove it happens on every one, not just the first. + """ + + def __init__(self) -> None: + self.options: List[Any] = [] + + def setopt(self, option: Any, value: Any) -> None: + self.options.append((option, value)) + + class StubSession: """Session double whose ``.get`` / ``.post`` return a pre-baked response. @@ -85,6 +99,7 @@ class StubSession: def __init__(self, response: FakeResponse) -> None: self._response = response self.calls: List[Dict[str, Any]] = [] + self.curl = FakeCurl() def _record(self, url: str, **kwargs: Any) -> FakeResponse: self.calls.append({"url": url, **kwargs}) diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index ad0c883..0d35f3b 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -216,6 +216,27 @@ def test_a_nonsensical_budget_is_rejected_at_the_call(self): with pytest.raises(ValueError): APIRequest("https://x.test/", session=session, max_response_bytes=bad) # type: ignore[arg-type] + def test_content_decoding_is_disabled_on_every_request(self): + """Regression: the option lapsed after the first request. + + Setting it once at construction looked right and worked once — + `Session.request` resets the handle, so requests 2..n arrived + pre-expanded with no budget in reach, and no double-based test noticed + because each one built a fresh client. + """ + from curl_cffi import CurlOpt + + session = StubSession(FakeResponse( + status_code=200, + headers={"Content-Type": "application/json"}, + content=b"{}", + )) + + for _ in range(3): + APIRequest("https://x.test/", session=session) # type: ignore[arg-type] + + assert session.curl.options == [(CurlOpt.HTTP_CONTENT_DECODING, 0)] * 3 + def test_the_body_is_not_requested_as_a_stream(self): """Pins a deliberate trade-off, not an oversight. @@ -309,3 +330,127 @@ def test_an_already_decoded_body_still_reaches_the_transport_fallback(self, enco with pytest.raises(Exception): decompress(b'{"already": "json"}') + + +class TestBudgetAgainstARealTransport: + """Exercises the budget over a socket, not over a double. + + A previous attempt at this budget was inert in production and every + double-based test still passed, because the doubles fed the helpers + compressed input that the real transport never produced. These tests talk + to a local server so the decoding path is the production one. + """ + + @staticmethod + def _serve(blob: bytes, encoding: str): + import threading + from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + + class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def do_GET(self) -> None: + self.send_response(200) + self.send_header("Content-Encoding", encoding) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(blob))) + self.end_headers() + self.wfile.write(blob) + + def log_message(self, *args: object) -> None: + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + server.daemon_threads = True + threading.Thread(target=server.serve_forever, daemon=True).start() + return server + + def test_a_bomb_is_refused_before_it_expands(self): + import brotli + + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import APIClient + + # A few hundred bytes on the wire, 32 MB expanded. + server = self._serve(brotli.compress(b"\x00" * (32 * 1024 * 1024)), "br") + + try: + client = APIClient() + request = client.request( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, deflate, br"}, + max_response_bytes=1024 * 1024, + ) + + # Decoding is lazy, which is what keeps an unread body from ever + # expanding; the budget therefore lands on the read. + with pytest.raises(DecompressionLimitError): + request.get_content() + finally: + server.shutdown() + + @pytest.mark.parametrize("encoding", ["br", "gzip", "deflate"]) + def test_every_advertised_encoding_round_trips(self, encoding): + """Taking decoding from libcurl means decoding everything we advertise. + + `Core.html_headers` asks for deflate, so dropping it here would corrupt + get_airlines() rather than fail loudly. + """ + import gzip as gzip_module + import zlib as zlib_module + + import brotli + + from FlightRadarAPI.request import APIClient + + body = b'{"rows": [{"name": "Guarulhos"}]}' + compressor = zlib_module.compressobj(wbits=-15) + blob = { + "br": brotli.compress(body), + "gzip": gzip_module.compress(body), + "deflate": compressor.compress(body) + compressor.flush(), + }[encoding] + + server = self._serve(blob, encoding) + + try: + response = APIClient().request( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, deflate, br"}, + ) + assert response.get_json_content() == {"rows": [{"name": "Guarulhos"}]} + finally: + server.shutdown() + + def test_the_standalone_path_decodes_and_bounds_too(self): + """`get_flight_details` uses it from a thread pool, bypassing the session.""" + import brotli + + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import APIClient + + body = b'{"ok": true}' + server = self._serve(brotli.compress(body), "br") + + try: + client = APIClient() + url = f"http://127.0.0.1:{server.server_port}/" + headers = {"accept-encoding": "gzip, br"} + + assert client.request_standalone(url, headers=headers).get_json_content() == {"ok": True} + finally: + server.shutdown() + + server = self._serve(brotli.compress(b"\x00" * (32 * 1024 * 1024)), "br") + + try: + request = client.request_standalone( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, br"}, + max_response_bytes=1024 * 1024, + ) + + with pytest.raises(DecompressionLimitError): + request.get_content() + finally: + server.shutdown() From 8763937ace1e4cc0e6bf3696a8c9dd0a10b1ef37 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 00:38:37 -0300 Subject: [PATCH 14/22] fix: harden the encodings this code is now responsible for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Taking decoding from libcurl means owning every shape the header arrives in, and the first pass did not. - The lookup was exact and case-sensitive, so `GZIP` or `gzip, br` fell through to identity and returned compressed bytes as content; callers got a UnicodeDecodeError out of json.loads with nothing logged. The token is normalised now, and an encoding with no decoder warns instead of passing bytes along as if they were readable. - Worse, `zstd` was reachable: with no explicit header, curl_cffi's chrome136 profile asks for "gzip, deflate, br, zstd" (verified), and nothing here decodes zstd. Every request now advertises exactly what `__content_encodings` implements. - `get_response_object().content` and `CloudflareError.response` were handing back the compressed body — both public, and a challenge page is the first thing anyone reads when debugging a block. The body is decoded during the request now and written back, so they read as they did before this branch. - `MAXFILESIZE_LARGE` bounds the download itself, so the received size is limited rather than merely checked once libcurl has buffered everything. Surfaced as DecompressionLimitError, because the retry policy treats raw curl errors as transient and retrying an oversized body is futile. On the Node side, `getCookie` and `__cookiesFor` disagreed about which same-named cookie wins — newest vs longest path — so a re-issued token could go out in the query string while the Cookie header on any /user/... request carried the stale one, and `Core.userLogoutUrl` is exactly that path. Both take the newest now. The cookie maps are also null-prototype: a cookie named `toString` read as an inherited function rather than as absent. --- nodejs/FlightRadarAPI/request.js | 13 +- nodejs/tests/testRequestTransport.js | 27 ++++ python/FlightRadarAPI/request.py | 147 +++++++++++++++------ python/tests/_request_doubles.py | 13 +- python/tests/test_request_transport.py | 176 ++++++++++++++++++++++--- 5 files changed, 299 insertions(+), 77 deletions(-) diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index 7346a0f..2012f54 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -408,7 +408,9 @@ async function request(url, { } const rawCookies = response.headers.getSetCookie() ?? []; - const responseCookies = {}; + // Null-prototype: a cookie named `toString` must read as absent, not as an + // inherited function, and one named `__proto__` must not vanish. + const responseCookies = Object.create(null); for (const header of rawCookies) { const pair = String(header).split(";")[0]; @@ -542,11 +544,12 @@ class Session { if (hostInScope) matches.push(cookie); } - // Shortest path first, so a more specific cookie overwrites a broader - // one of the same name rather than losing to insertion order. - matches.sort((a, b) => a.path.length - b.path.length); + // Oldest first, so the newest of a same-named pair wins — the rule + // getCookie() uses, because a re-issued token supersedes the one it + // replaces. Path length only breaks a tie within one response. + matches.sort((a, b) => (a.storedAt - b.storedAt) || (a.path.length - b.path.length)); - const selected = {}; + const selected = Object.create(null); for (const cookie of matches) selected[cookie.name] = cookie.value; diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index 5e2365d..eb3c41c 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -530,3 +530,30 @@ describe("Byte-order mark handling (offline)", function() { expect([...Buffer.from(content)]).to.deep.equal([0xEF, 0xBB, 0xBF, 1, 2]); }); }); + + +describe("Cookie read paths agree (offline)", function() { + const { Session } = require("../FlightRadarAPI/request"); + + it("resolves a re-issued cookie the same way for reads and for sending", function() { + // getCookie feeds the `token`/`enc` query params while the jar builds + // the Cookie header; if they disagree, one request carries two + // different values for the same token. /user/logout is a real URL. + const session = new Session(); + session.__storeCookies("https://www.flightradar24.com/user/login", ["_frPl=old-token"]); + session.__storeCookies("https://www.flightradar24.com/", ["_frPl=new-token; Path=/"]); + + expect(session.getCookie("_frPl")).to.equal("new-token"); + expect(session.__cookiesFor("https://www.flightradar24.com/user/logout")) + .to.deep.equal({ _frPl: "new-token" }); + }); + + it("does not expose inherited Object properties as cookies", function() { + const session = new Session(); + session.__storeCookies("https://www.flightradar24.com/", ["a=1; Path=/"]); + + const selected = session.__cookiesFor("https://www.flightradar24.com/"); + expect(selected.toString).to.equal(undefined); + expect(selected.constructor).to.equal(undefined); + }); +}); diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index 06cd273..a32b196 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -9,7 +9,7 @@ from urllib.parse import urlencode import brotli -from curl_cffi import CurlOpt, requests +from curl_cffi import CurlECode, CurlOpt, requests from curl_cffi.requests import Session from .errors import CloudflareError, DecompressionLimitError @@ -34,6 +34,16 @@ _GZIP_WBITS = 31 # 16 + MAX_WBITS: gzip wrapper rather than raw deflate +def _bound_download(session: Session, limit: int) -> None: + """Have libcurl abort a response body larger than ``limit``. + + Bounds the bytes as received, which the post-hoc length check cannot: by + the time it runs, libcurl has buffered the whole body. Aborts both when + `Content-Length` announces the size and mid-transfer when it does not. + """ + session.curl.setopt(CurlOpt.MAXFILESIZE_LARGE, limit) + + def _keep_body_encoded(session: Session) -> None: """Stop libcurl decompressing this session's next response. @@ -264,11 +274,18 @@ class APIRequest: """ __content_encodings = { "": lambda data, limit: data, + "identity": lambda data, limit: data, "br": _decompress_brotli, "gzip": _decompress_gzip, - "deflate": _decompress_deflate + "deflate": _decompress_deflate, } + #: Advertised on every request, because taking decoding from libcurl means + #: only asking for what `__content_encodings` can decode. curl_cffi's + #: impersonation otherwise defaults to "gzip, deflate, br, zstd", and a zstd + #: reply would arrive as bytes nothing here can read. + supported_encodings = "gzip, deflate, br" + def __init__( self, url: str, @@ -299,20 +316,49 @@ def __init__( self.url = url self.__max_response_bytes = max_response_bytes + headers = self.__with_supported_encodings(headers) if params: url += "?" + urlencode(params) - if session is not None: - _keep_body_encoded(session) - request_method = session.get if data is None else session.post - self.__response = request_method(url, headers=headers, data=data, timeout=timeout) - else: - # A throwaway session rather than the module-level helpers, whose - # internal handle this cannot reach. - with Session(impersonate=impersonate) as standalone: # type: ignore[arg-type] - _keep_body_encoded(standalone) - request_method = standalone.get if data is None else standalone.post + try: + if session is not None: + _keep_body_encoded(session) + _bound_download(session, max_response_bytes) + request_method = session.get if data is None else session.post self.__response = request_method(url, headers=headers, data=data, timeout=timeout) + else: + # A throwaway session rather than the module-level helpers, whose + # internal handle this cannot reach. + with Session(impersonate=impersonate) as standalone: # type: ignore[arg-type] + _keep_body_encoded(standalone) + _bound_download(standalone, max_response_bytes) + request_method = standalone.get if data is None else standalone.post + self.__response = request_method(url, headers=headers, data=data, timeout=timeout) + except requests.errors.RequestsError as err: # type: ignore[attr-defined] + # Not a transient failure, so it must not reach the retry policy as one. + if getattr(err, "code", None) == CurlECode.FILESIZE_EXCEEDED: + raise DecompressionLimitError( + f"Response body from {self.url} is larger than the " + f"{max_response_bytes} byte limit." + ) from err + raise + + received = self.__response.content + + # Bounds the body as libcurl buffered it; the decompressors bound its + # expansion. This one cannot undo the read, only the work after it. + if len(received) > max_response_bytes: + raise DecompressionLimitError( + f"Response body from {self.url} is {len(received)} bytes, " + f"past the {max_response_bytes} byte limit." + ) + + self.__content = self.__decode_body(received) + + # `get_response_object()` and `CloudflareError.response` are public, and + # a challenge page is the first thing anyone reads when debugging a + # block. Since libcurl no longer decodes, hand them the decoded body. + self.__response.content = self.__content # Cloudflare detection only when the caller did not opt-in to this status code. # `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. @@ -327,14 +373,15 @@ def __init__( if self.get_status_code() not in (allowed_error_codes or []): self.__response.raise_for_status() - self.__content = self.__response.content + @classmethod + def __with_supported_encodings(cls, headers: Optional[Dict]) -> Optional[Dict]: + """Ask only for encodings this class can decode.""" + if headers and any(name.lower() == "accept-encoding" for name in headers): + return headers - # Bounds the body as received; the decompressors bound its expansion. - if len(self.__content) > max_response_bytes: - raise DecompressionLimitError( - f"Response body from {self.url} is {len(self.__content)} bytes, " - f"past the {max_response_bytes} byte limit." - ) + merged = dict(headers or {}) + merged["accept-encoding"] = cls.supported_encodings + return merged def __is_cloudflare_block(self) -> bool: """ @@ -358,28 +405,39 @@ def __is_cloudflare_block(self) -> bool: return False return bool(self.__response.headers.get("cf-mitigated")) - def get_content(self) -> Union[Dict, bytes]: - """ - Return the received content from the request. - """ - content = self.__content + def __decode_body(self, content: bytes) -> bytes: + """Undo the `Content-Encoding` this response arrived with. - content_encoding = self.__response.headers.get("Content-Encoding", "") - content_type = self.__response.headers.get("Content-Type", "") + Header values are case-insensitive and may carry whitespace, so the + token is normalised before lookup: an exact match would send `GZIP` + down the identity path and return compressed bytes as if they were + content. An encoding this class does not implement warns rather than + passing silently, since nothing downstream can act on the result. + """ + content_encoding = self.__response.headers.get("Content-Encoding", "") or "" + encoding = content_encoding.strip().lower() + decode = self.__content_encodings.get(encoding) + + if decode is None: + _logger.warning( + "APIRequest: no decoder for Content-Encoding=%r on %s. Returning the body " + "as received, which callers will not be able to read.", + content_encoding, self.url, + ) + return content - # Decompress the content if a known encoding was used; fall back to raw bytes otherwise. - # curl_cffi may already decompress content automatically, so failures here usually mean - # the bytes were already decoded by the transport layer — log and continue rather than - # surfacing an error the caller cannot act on. - decode = self.__content_encodings.get(content_encoding, self.__content_encodings[""]) try: - content = decode(content, self.__max_response_bytes) + return decode(content, self.__max_response_bytes) except DecompressionLimitError: raise except Exception as err: - # Decided by the body, not the header: undecodable text is genuinely - # broken and must warn, while binary bodies carry no such tell. Nothing - # here may raise, or it would replace `err` with its own failure. + # Reached when the body is not encoded the way the header claims — + # most often a transport that decoded it after all. Decided by the + # body, not the header: undecodable text is genuinely broken and + # must warn, while binary bodies carry no such tell. Nothing here + # may raise, or it would replace `err` with its own failure. + content_type = self.__response.headers.get("Content-Type", "") + if not isinstance(content, bytes): transport_decoded = True elif content_type.startswith(("application/json", "text/")): @@ -389,21 +447,28 @@ def get_content(self) -> Union[Dict, bytes]: except UnicodeDecodeError: transport_decoded = False else: - corrupt_gzip = content_encoding == "gzip" and content.startswith(b"\x1f\x8b") - transport_decoded = content_encoding in ("gzip", "br") and not corrupt_gzip + corrupt_gzip = encoding == "gzip" and content.startswith(b"\x1f\x8b") + transport_decoded = encoding in ("gzip", "br", "deflate") and not corrupt_gzip _logger.log( logging.DEBUG if transport_decoded else logging.WARNING, - "APIRequest.get_content: failed to decode Content-Encoding=%r for %s (%s). " - "Assuming the transport already decompressed and returning raw bytes.", + "APIRequest: failed to decode Content-Encoding=%r for %s (%s). " + "Assuming the body arrived already decoded and returning it as-is.", content_encoding, self.url, err, ) + return content + + def get_content(self) -> Union[Dict, bytes]: + """ + Return the received content from the request. + """ + content_type = self.__response.headers.get("Content-Type", "") # Return a dictionary if the content type is JSON. if "application/json" in content_type: - return json.loads(content) + return json.loads(self.__content) - return content + return self.__content def get_json_content(self) -> Dict[str, Any]: """ diff --git a/python/tests/_request_doubles.py b/python/tests/_request_doubles.py index 1681f23..2cdde15 100644 --- a/python/tests/_request_doubles.py +++ b/python/tests/_request_doubles.py @@ -47,9 +47,8 @@ def __contains__(self, key: object) -> bool: class FakeResponse: """Minimal stand-in for a curl_cffi response object. - ``APIRequest`` only touches ``.status_code``, ``.headers``, - ``.iter_content()``, ``.close()`` and ``.raise_for_status()`` — that's all - we have to implement. + ``APIRequest`` only touches ``.status_code``, ``.headers``, ``.content`` + and ``.raise_for_status()`` — that's all we have to implement. """ def __init__( @@ -67,14 +66,6 @@ def raise_for_status(self) -> None: if 400 <= self.status_code < 600: raise RuntimeError(f"HTTP {self.status_code}") - def iter_content(self, chunk_size: int = 8192): - """Yield the body in chunks, as the streaming transport does.""" - for start in range(0, len(self.content), chunk_size): - yield self.content[start:start + chunk_size] - - def close(self) -> None: - self.closed = True - class FakeCurl: """Curl handle double that records the options set on it. diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index 0d35f3b..6c7c0a5 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -216,10 +216,10 @@ def test_a_nonsensical_budget_is_rejected_at_the_call(self): with pytest.raises(ValueError): APIRequest("https://x.test/", session=session, max_response_bytes=bad) # type: ignore[arg-type] - def test_content_decoding_is_disabled_on_every_request(self): - """Regression: the option lapsed after the first request. + def test_the_curl_options_are_set_on_every_request(self): + """Regression: they lapsed after the first request. - Setting it once at construction looked right and worked once — + Setting them once at construction looked right and worked once — `Session.request` resets the handle, so requests 2..n arrived pre-expanded with no budget in reach, and no double-based test noticed because each one built a fresh client. @@ -233,9 +233,12 @@ def test_content_decoding_is_disabled_on_every_request(self): )) for _ in range(3): - APIRequest("https://x.test/", session=session) # type: ignore[arg-type] + APIRequest("https://x.test/", session=session, max_response_bytes=4096) # type: ignore[arg-type] - assert session.curl.options == [(CurlOpt.HTTP_CONTENT_DECODING, 0)] * 3 + assert session.curl.options == [ + (CurlOpt.HTTP_CONTENT_DECODING, 0), + (CurlOpt.MAXFILESIZE_LARGE, 4096), + ] * 3 def test_the_body_is_not_requested_as_a_stream(self): """Pins a deliberate trade-off, not an oversight. @@ -376,16 +379,14 @@ def test_a_bomb_is_refused_before_it_expands(self): try: client = APIClient() - request = client.request( - f"http://127.0.0.1:{server.server_port}/", - headers={"accept-encoding": "gzip, deflate, br"}, - max_response_bytes=1024 * 1024, - ) - - # Decoding is lazy, which is what keeps an unread body from ever - # expanding; the budget therefore lands on the read. + # Decoded during the request, so the budget lands there rather + # than on a later read — the same point the Node port enforces it. with pytest.raises(DecompressionLimitError): - request.get_content() + client.request( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, deflate, br"}, + max_response_bytes=1024 * 1024, + ) finally: server.shutdown() @@ -444,13 +445,148 @@ def test_the_standalone_path_decodes_and_bounds_too(self): server = self._serve(brotli.compress(b"\x00" * (32 * 1024 * 1024)), "br") try: - request = client.request_standalone( - f"http://127.0.0.1:{server.server_port}/", - headers={"accept-encoding": "gzip, br"}, - max_response_bytes=1024 * 1024, - ) + with pytest.raises(DecompressionLimitError): + client.request_standalone( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, br"}, + max_response_bytes=1024 * 1024, + ) + finally: + server.shutdown() + + +class TestEncodingRobustness: + """Owning the decoding means owning every shape the header arrives in.""" + + def _serve(self, blob: bytes, encoding: str): + return TestBudgetAgainstARealTransport._serve(blob, encoding) + + @pytest.mark.parametrize("header", ["gzip", "GZIP", " gzip ", "Gzip"]) + def test_the_encoding_token_is_matched_case_insensitively(self, header): + """RFC 9110 header values are case-insensitive; an exact match sent + `GZIP` down the identity path and returned compressed bytes.""" + import gzip as gzip_module + + from FlightRadarAPI.request import APIClient + + server = self._serve(gzip_module.compress(b'{"ok": true}'), header) + + try: + response = APIClient().request(f"http://127.0.0.1:{server.server_port}/") + assert response.get_json_content() == {"ok": True} + finally: + server.shutdown() + + def test_only_decodable_encodings_are_advertised(self): + """curl_cffi's impersonation asks for zstd, which nothing here decodes.""" + import threading + from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + from FlightRadarAPI.request import APIClient, APIRequest + + seen = [] + + class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def do_GET(self) -> None: + seen.append(self.headers.get("accept-encoding")) + body = b"{}" + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args: object) -> None: + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + server.daemon_threads = True + threading.Thread(target=server.serve_forever, daemon=True).start() + + try: + client = APIClient() + url = f"http://127.0.0.1:{server.server_port}/" + client.request(url) + client.request_standalone(url) + + assert seen == [APIRequest.supported_encodings] * 2 + assert "zstd" not in APIRequest.supported_encodings + finally: + server.shutdown() + + def test_an_undecodable_encoding_warns_instead_of_passing_bytes_along(self, caplog): + import gzip as gzip_module + + from FlightRadarAPI.request import APIClient + + server = self._serve(gzip_module.compress(b'{"ok": true}'), "zstd") + + try: + with caplog.at_level("WARNING", logger="FlightRadarAPI.request"): + APIClient().request( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, zstd"}, + ) + assert any("no decoder for Content-Encoding" in r.message for r in caplog.records) + finally: + server.shutdown() + + def test_the_public_response_object_carries_the_decoded_body(self): + """`CloudflareError.response` is how a user reads a challenge page.""" + import gzip as gzip_module + + from FlightRadarAPI.request import APIClient + + body = b'{"ok": true}' + server = self._serve(gzip_module.compress(body), "gzip") + + try: + response = APIClient().request(f"http://127.0.0.1:{server.server_port}/") + + assert response.get_response_object().content == body + assert response.get_response_object().text == body.decode() + finally: + server.shutdown() + + +class TestDownloadBound: + """The received bytes are bounded by libcurl, not only checked afterwards.""" + + def test_an_oversized_body_is_aborted_mid_download(self): + import threading + from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import APIClient + + big = b"x" * (5 * 1024 * 1024) + + class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def do_GET(self) -> None: + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(big))) + self.end_headers() + self.wfile.write(big) + + def log_message(self, *args: object) -> None: + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + server.daemon_threads = True + threading.Thread(target=server.serve_forever, daemon=True).start() + + try: + # Surfaced as our own error, not curl's: the retry policy treats + # curl errors as transient, and retrying an oversized body is futile. with pytest.raises(DecompressionLimitError): - request.get_content() + APIClient().request( + f"http://127.0.0.1:{server.server_port}/", + max_response_bytes=1024 * 1024, + ) finally: server.shutdown() From d76a911689de1a79648a36235414103f9840a672 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 10:08:48 -0300 Subject: [PATCH 15/22] refactor: separate the two size budgets and make the rest self-enforcing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review cleanups, no behaviour change on the happy path. `max_response_bytes` governed two different limits: the bytes on the wire and the bytes after expansion. Compression grows incompressible data, so a body that expands to just under the budget can arrive slightly over it and be rejected by a bound that was not meant for it. `max_download_bytes` is now separate, defaulting to the other. `supported_encodings` is derived from the decoder table instead of written out, so advertising an encoding with no decoder — the zstd bug from the previous commit — is no longer expressible. The post-hoc length check is labelled as the backstop it is: libcurl aborts first via MAXFILESIZE, so it is unreachable unless a transport ignores that option, and the stub-based test said so. Renamed to match, with a pointer to the socket-level test that proves the real bound. The module note claimed streaming cost connection reuse without qualifying it; the standalone path already opens a connection per call, so that cost lands on the session path only. The timeout degradation ruled streaming out on its own. On the Node side, the cookie map merged into the Cookie header was the last one built from a plain object literal, so it reintroduced the inherited prototype the other two had dropped. --- nodejs/FlightRadarAPI/request.js | 4 +- python/FlightRadarAPI/request.py | 48 +++++++++++++------- python/tests/test_request_transport.py | 61 +++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index 2012f54..ec7cc17 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -568,7 +568,9 @@ class Session { */ async request(url, options = {}) { const { cookies: extraCookies, ...rest } = options; - const merged = { ...this.__cookiesFor(url), ...(extraCookies ?? {}) }; + // Null-prototype like the maps it merges, so an inherited `toString` + // cannot reappear on the way into the Cookie header. + const merged = Object.assign(Object.create(null), this.__cookiesFor(url), extraCookies ?? {}); const cookies = Object.keys(merged).length > 0 ? merged : null; const result = await request(url, { diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index a32b196..5060dc5 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -24,8 +24,10 @@ # Enforcing that needs the compressed bytes, which means taking content decoding # away from libcurl (see `_new_curl_handle`): left to itself it expands the body # before any of this code runs, and the only other way to intervene — -# `stream=True` — was measured to cost more than the bomb it stops (`timeout` -# degrades to a >=1 byte/sec liveness check, and connection reuse disappears). +# `stream=True` — was measured to cost more than the bomb it stops: `timeout` +# degrades to a >=1 byte/sec liveness check, which alone rules it out, and the +# session stops reusing connections (the standalone path already opens one per +# call, so that second cost lands on the session path only). # Owning the decoding is why `deflate` is implemented below rather than left to # the transport: whatever `accept-encoding` advertises, this module must decode. MAX_RESPONSE_BYTES = 64 * 1024 * 1024 @@ -272,19 +274,24 @@ class APIRequest: """ Class to make requests to the FlightRadar24. """ + # Ordered as Chrome sends them, since this is what `Accept-Encoding` + # advertises. "" and "identity" mean "no encoding", not a decoder. __content_encodings = { "": lambda data, limit: data, "identity": lambda data, limit: data, - "br": _decompress_brotli, "gzip": _decompress_gzip, "deflate": _decompress_deflate, + "br": _decompress_brotli, } #: Advertised on every request, because taking decoding from libcurl means - #: only asking for what `__content_encodings` can decode. curl_cffi's - #: impersonation otherwise defaults to "gzip, deflate, br, zstd", and a zstd - #: reply would arrive as bytes nothing here can read. - supported_encodings = "gzip, deflate, br" + #: only asking for what can be decoded here. curl_cffi's impersonation + #: otherwise defaults to "gzip, deflate, br, zstd", and a zstd reply would + #: arrive as bytes nothing here can read. Derived rather than written out, + #: so advertising an encoding without a decoder is not expressible. + supported_encodings = ", ".join( + name for name in __content_encodings if name not in ("", "identity") + ) def __init__( self, @@ -298,6 +305,7 @@ def __init__( allowed_error_codes: Optional[List[int]] = None, impersonate: str = DEFAULT_IMPERSONATE, max_response_bytes: int = MAX_RESPONSE_BYTES, + max_download_bytes: Optional[int] = None, ): """ Constructor of the APIRequest class. @@ -309,11 +317,20 @@ def __init__( :param data: data for the request. If "data" is None, request will be a GET. Otherwise, it will be a POST :param allowed_error_codes: status codes that should not raise an error :param impersonate: curl_cffi browser profile (only used when no session is provided) - :param max_response_bytes: reject a body that streams past this many bytes + :param max_response_bytes: reject a body that expands past this many bytes + :param max_download_bytes: reject a body larger than this on the wire. + Defaults to ``max_response_bytes``. Separate because compression can + grow incompressible data, so a body that expands to just under the + budget may still arrive slightly over it. """ if max_response_bytes < 1: raise ValueError("max_response_bytes must be >= 1") + if max_download_bytes is None: + max_download_bytes = max_response_bytes + elif max_download_bytes < 1: + raise ValueError("max_download_bytes must be >= 1") + self.url = url self.__max_response_bytes = max_response_bytes headers = self.__with_supported_encodings(headers) @@ -323,7 +340,7 @@ def __init__( try: if session is not None: _keep_body_encoded(session) - _bound_download(session, max_response_bytes) + _bound_download(session, max_download_bytes) request_method = session.get if data is None else session.post self.__response = request_method(url, headers=headers, data=data, timeout=timeout) else: @@ -331,7 +348,7 @@ def __init__( # internal handle this cannot reach. with Session(impersonate=impersonate) as standalone: # type: ignore[arg-type] _keep_body_encoded(standalone) - _bound_download(standalone, max_response_bytes) + _bound_download(standalone, max_download_bytes) request_method = standalone.get if data is None else standalone.post self.__response = request_method(url, headers=headers, data=data, timeout=timeout) except requests.errors.RequestsError as err: # type: ignore[attr-defined] @@ -339,18 +356,19 @@ def __init__( if getattr(err, "code", None) == CurlECode.FILESIZE_EXCEEDED: raise DecompressionLimitError( f"Response body from {self.url} is larger than the " - f"{max_response_bytes} byte limit." + f"{max_download_bytes} byte download limit." ) from err raise received = self.__response.content - # Bounds the body as libcurl buffered it; the decompressors bound its - # expansion. This one cannot undo the read, only the work after it. - if len(received) > max_response_bytes: + # Backstop only: `_bound_download` makes libcurl abort first, so this + # is unreachable unless a transport ignores MAXFILESIZE. Kept because + # it costs one comparison and the alternative is an unbounded read. + if len(received) > max_download_bytes: raise DecompressionLimitError( f"Response body from {self.url} is {len(received)} bytes, " - f"past the {max_response_bytes} byte limit." + f"past the {max_download_bytes} byte download limit." ) self.__content = self.__decode_body(received) diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index 6c7c0a5..24405a9 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -183,7 +183,14 @@ def test_an_empty_body_is_not_mistaken_for_a_bomb(self): assert _decompress_brotli(brotli.compress(b"")) == b"" assert _decompress_gzip(gzip.compress(b"")) == b"" - def test_a_body_past_the_budget_is_refused(self): + def test_the_post_hoc_size_check_still_refuses_an_oversized_body(self): + """Covers the backstop, not the mechanism. + + In production `_bound_download` makes libcurl abort first, so this + comparison is unreachable — a stub session is the only way to reach it, + because `setopt` on the double is inert. `TestDownloadBound` is what + proves the real bound. + """ from FlightRadarAPI.errors import DecompressionLimitError from FlightRadarAPI.request import MAX_RESPONSE_BYTES @@ -590,3 +597,55 @@ def log_message(self, *args: object) -> None: ) finally: server.shutdown() + + +class TestSeparateLimits: + """The wire size and the expanded size are different budgets.""" + + def test_the_download_bound_defaults_to_the_expansion_budget(self): + from curl_cffi import CurlOpt + + session = StubSession(FakeResponse( + status_code=200, + headers={"Content-Type": "application/json"}, + content=b"{}", + )) + APIRequest("https://x.test/", session=session, max_response_bytes=2048) # type: ignore[arg-type] + + assert (CurlOpt.MAXFILESIZE_LARGE, 2048) in session.curl.options + + def test_the_download_bound_can_be_set_apart(self): + """Compression grows incompressible data, so a body that expands to + just under the budget can still arrive slightly over it.""" + from curl_cffi import CurlOpt + + session = StubSession(FakeResponse( + status_code=200, + headers={"Content-Type": "application/json"}, + content=b"{}", + )) + APIRequest( # type: ignore[arg-type] + "https://x.test/", session=session, + max_response_bytes=2048, max_download_bytes=4096, + ) + + assert (CurlOpt.MAXFILESIZE_LARGE, 4096) in session.curl.options + + def test_a_nonsensical_download_bound_is_rejected(self): + session = StubSession(FakeResponse(status_code=200, content=b"ok")) + + with pytest.raises(ValueError): + APIRequest("https://x.test/", session=session, max_download_bytes=0) # type: ignore[arg-type] + + +class TestAdvertisedEncodings: + def test_every_advertised_encoding_has_a_decoder(self): + """Derived from the table, so this cannot drift — pinned anyway, + because advertising zstd without a decoder is the bug it prevents.""" + table = getattr(APIRequest, "_APIRequest__content_encodings") + + for name in APIRequest.supported_encodings.split(", "): + assert name in table + assert table[name] is not table[""] + + assert APIRequest.supported_encodings == "gzip, deflate, br" From 104c5ee0c50f6de3b81affa1f8dde5c9d03d1091 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 13:11:47 -0300 Subject: [PATCH 16/22] fix: close the holes the last two rounds of budget work opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All seven reproduced before fixing, and re-measured after. Python. The expansion budget stopped applying to bodies that reach no decoder: an identity 5 MiB body passed with max_response_bytes=1024, because the only check on the received bytes had moved to the download bound when the two knobs were split. Checked after decoding now, so the budget means the same thing whatever encoding arrived. Raw deflate carries neither header nor checksum, so a body that is not deflate can inflate to plausible bytes and be returned as content. Requiring the whole input to be consumed is the only tell available and takes 3000 fuzzed JSON/HTML bodies from 6 silent passes to 1. It applies to raw only: the zlib wrapper has an adler32 and validates itself, and demanding full consumption there rejected legitimate bodies that arrived with trailing padding. Node. Both error paths threw before the body was read, so undici had to destroy the socket: 10 sequential 500s opened 8 new connections against 2 for 200s. Every Cloudflare block and every 5xx therefore cost a fresh TLS handshake, compounding under RetryPolicy — the same handshake-per-request pattern this branch rejected stream=True over. The body is read before the checks now, and 10 errors open none. That read also ran outside the abort timer, so timeout only covered time-to-headers. A server dripping 20 bytes at 400ms resolved after 8032ms against timeout 500 — precisely the degradation cited for rejecting streaming on the Python side. The read is inside the timer now: TimeoutError at 504ms. Draining the body for that first fix left response.bodyUsed true, so the challenge page CloudflareError exposes was no longer readable. The error carries it directly. Also: the path-length tie-break in __cookiesFor was unreachable, since storedAt is unique per cookie; dropped, with the same-name collapse documented as the deliberate limit it is. engines.node moves to >=18.17, which is what undici 6.28 actually requires. And the three size checks now say which case each one covers, so the next reader does not remove the wrong "duplicate". --- nodejs/FlightRadarAPI/errors.js | 7 +- nodejs/FlightRadarAPI/index.d.ts | 4 +- nodejs/FlightRadarAPI/request.js | 23 +++++- nodejs/package.json | 2 +- nodejs/tests/testRequestTransport.js | 99 ++++++++++++++++++++++++++ python/FlightRadarAPI/request.py | 26 ++++++- python/tests/test_request_transport.py | 87 ++++++++++++++++++++++ 7 files changed, 239 insertions(+), 9 deletions(-) diff --git a/nodejs/FlightRadarAPI/errors.js b/nodejs/FlightRadarAPI/errors.js index 3b1c986..67a8a01 100644 --- a/nodejs/FlightRadarAPI/errors.js +++ b/nodejs/FlightRadarAPI/errors.js @@ -16,10 +16,15 @@ class CloudflareError extends FlightRadarError { /** * @param {string} message * @param {object} response + * @param {string} [body] - the challenge page, already read off `response` */ - constructor(message, response) { + constructor(message, response, body) { super(message); this.response = response; + // `response.bodyUsed` is true by the time this is thrown: the body has + // to be drained for the connection to survive. Carry it so the page is + // still readable, which is the point of exposing the response at all. + this.body = body; } } diff --git a/nodejs/FlightRadarAPI/index.d.ts b/nodejs/FlightRadarAPI/index.d.ts index 536e7e0..533d408 100644 --- a/nodejs/FlightRadarAPI/index.d.ts +++ b/nodejs/FlightRadarAPI/index.d.ts @@ -517,7 +517,9 @@ export class AirportNotFoundError extends FlightRadarError { export class CloudflareError extends FlightRadarError { response: any; - constructor(message?: string, response?: any); + /** The challenge page, read off `response` before it was consumed. */ + body?: string; + constructor(message?: string, response?: any, body?: string); } export class DecompressionLimitError extends FlightRadarError { diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index ec7cc17..bc6af53 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -365,8 +365,23 @@ async function request(url, { settings.signal = controller.signal; let response; + let body; + try { response = await fetch(url, settings); + + // Read before the status checks below, for two reasons: an abandoned + // body leaves undici no choice but to destroy the connection, so every + // error response would cost a fresh TLS handshake; and a Cloudflare + // challenge page is the one thing worth having when a block happens. + // Inside the try so the timeout still covers the read — the abort + // signal is what stops a trickled body holding the call open. + // + // The cost is that an error response is read before it is raised, so a + // large one is paid for in full (bounded by the budget), and a body + // over budget surfaces as DecompressionLimitError rather than as the + // status or Cloudflare error behind it. + body = await readBoundedBody(response, maxResponseBytes, url); } catch (err) { if (err.name === "AbortError") { @@ -386,6 +401,7 @@ async function request(url, { "Blocked by Cloudflare. Perhaps you are making too many calls, " + "or the TLS impersonation needs to be updated.", response, + body.toString("utf-8"), ); } @@ -394,7 +410,6 @@ async function request(url, { } const contentType = response.headers.get("content-type") ?? ""; - const body = await readBoundedBody(response, maxResponseBytes, url); let content; if (contentType.includes("application/json")) { @@ -546,8 +561,10 @@ class Session { // Oldest first, so the newest of a same-named pair wins — the rule // getCookie() uses, because a re-issued token supersedes the one it - // replaces. Path length only breaks a tie within one response. - matches.sort((a, b) => (a.storedAt - b.storedAt) || (a.path.length - b.path.length)); + // replaces. `storedAt` is unique per cookie, so no tie is possible. + // Collapsing to one is a deliberate limit of building the header from + // a flat map; RFC 6265 would send both, most-specific path first. + matches.sort((a, b) => a.storedAt - b.storedAt); const selected = Object.create(null); diff --git a/nodejs/package.json b/nodejs/package.json index f72cc7c..d0a0f5b 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -32,7 +32,7 @@ }, "homepage": "https://github.com/JeanExtreme002/FlightRadarAPI#readme", "engines": { - "node": ">=18" + "node": ">=18.17" }, "dependencies": { "node-html-parser": "^6.1.13", diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index eb3c41c..6b45eb3 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -557,3 +557,102 @@ describe("Cookie read paths agree (offline)", function() { expect(selected.constructor).to.equal(undefined); }); }); + + +describe("Error responses and slow bodies (offline)", function() { + const http = require("http"); + const { request } = require("../FlightRadarAPI/request"); + const { CloudflareError } = require("../FlightRadarAPI/errors"); + + // A real server: MockAgent neither pools connections nor trickles a body. + /** + * @param {Function} handler - node request handler + * @return {Promise} the listening server + */ + function serve(handler) { + return new Promise((resolve) => { + const server = http.createServer(handler); + server.listen(0, "127.0.0.1", () => resolve(server)); + }); + } + + it("keeps the connection alive across error responses", async function() { + // Regression: throwing before reading left the body unconsumed, so + // undici destroyed the socket and every error cost a new handshake. + const payload = "x".repeat(200_000); + const connections = new Set(); + const server = await serve((req, res) => { + res.writeHead(500, { + "content-type": "application/json", + "content-length": String(payload.length), + }); + res.end(payload); + }); + server.on("connection", (socket) => connections.add(socket.remotePort)); + + try { + for (let i = 0; i < 5; i++) { + await request(`http://127.0.0.1:${server.address().port}/`).catch(() => {}); + } + expect(connections.size).to.be.lessThan(3); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); + + it("times out a body that trickles in under the cap", async function() { + // The budget alone cannot stop this: the body never exceeds it. + const server = await serve((req, res) => { + res.writeHead(200, { "content-type": "text/plain", "content-length": "20" }); + let sent = 0; + const timer = setInterval(() => { + if (sent++ >= 20) { + clearInterval(timer); + res.end(); + return; + } + res.write("x"); + }, 200); + }); + + try { + await request(`http://127.0.0.1:${server.address().port}/`, { timeout: 300 }); + expect.fail("should have timed out"); + } + catch (err) { + expect(err.name).to.equal("TimeoutError"); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); + + it("carries the challenge page on CloudflareError", async function() { + const challenge = "Attention Required! Cloudflare"; + const server = await serve((req, res) => { + res.writeHead(403, { + "cf-mitigated": "challenge", + "content-type": "text/html", + "content-length": String(challenge.length), + }); + res.end(challenge); + }); + + try { + await request(`http://127.0.0.1:${server.address().port}/`); + expect.fail("should have raised CloudflareError"); + } + catch (err) { + // response.bodyUsed is true by then, so the error has to carry it. + expect(err).to.be.instanceOf(CloudflareError); + expect(err.body).to.equal(challenge); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); +}); diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index 5060dc5..6d00160 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -82,6 +82,15 @@ def _decompress_deflate(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: ) if decompressor.eof: + # Raw deflate carries neither a header nor a checksum, so a body + # that is not deflate at all can still inflate to plausible bytes; + # requiring the whole input to be consumed is the only tell there + # is. The zlib wrapper has an adler32 and validates itself, so + # applying the same rule there would only reject a legitimate body + # that arrived with trailing padding. + if wbits == _RAW_DEFLATE_WBITS and decompressor.unused_data: + continue + return output raise zlib.error("body is not a complete deflate stream") @@ -362,9 +371,11 @@ def __init__( received = self.__response.content - # Backstop only: `_bound_download` makes libcurl abort first, so this - # is unreachable unless a transport ignores MAXFILESIZE. Kept because - # it costs one comparison and the alternative is an unbounded read. + # Three checks guard the size, each covering what the others cannot: + # MAXFILESIZE_LARGE stops the download at the socket, the decoders stop + # an expansion as it happens, and this one is the backstop for a + # transport that honours neither. Unreachable today, kept because it + # costs one comparison. if len(received) > max_download_bytes: raise DecompressionLimitError( f"Response body from {self.url} is {len(received)} bytes, " @@ -373,6 +384,15 @@ def __init__( self.__content = self.__decode_body(received) + # The decoders enforce the budget as they expand, but identity bodies + # and encodings with no decoder never reach one. Checked here so the + # budget means the same thing whatever arrived. + if len(self.__content) > max_response_bytes: + raise DecompressionLimitError( + f"Response body from {self.url} is {len(self.__content)} bytes, " + f"past the {max_response_bytes} byte limit." + ) + # `get_response_object()` and `CloudflareError.response` are public, and # a challenge page is the first thing anyone reads when debugging a # block. Since libcurl no longer decodes, hand them the decoded body. diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index 24405a9..ad3058b 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -649,3 +649,90 @@ def test_every_advertised_encoding_has_a_decoder(self): assert table[name] is not table[""] assert APIRequest.supported_encodings == "gzip, deflate, br" + + +class TestBudgetCoversEveryEncoding: + """The expansion budget must not depend on which decoder ran.""" + + def test_an_uncompressed_body_is_still_bounded(self): + """Regression: identity bodies reached no decoder, so nothing checked + them once the download bound was raised separately.""" + import threading + from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + + from FlightRadarAPI.errors import DecompressionLimitError + from FlightRadarAPI.request import APIClient + + big = b'{"x": "' + b"A" * (2 * 1024 * 1024) + b'"}' + + class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def do_GET(self) -> None: + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(big))) + self.end_headers() + self.wfile.write(big) + + def log_message(self, *args: object) -> None: + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + server.daemon_threads = True + threading.Thread(target=server.serve_forever, daemon=True).start() + + try: + with pytest.raises(DecompressionLimitError): + APIClient().request( + f"http://127.0.0.1:{server.server_port}/", + max_response_bytes=1024, + max_download_bytes=10 * 1024 * 1024, + ) + finally: + server.shutdown() + + +class TestDeflateIntegrity: + """Raw deflate has no header and no checksum to fail on.""" + + def test_a_body_that_is_not_deflate_raises_rather_than_inflating_to_garbage(self): + from FlightRadarAPI.request import _decompress_deflate + + # Requiring the whole input to be consumed is the only tell available, + # so this cannot be airtight — but a plain JSON body must not pass. + for body in (b'{"rows": []}', b"hello", b"plain text"): + with pytest.raises(Exception): + _decompress_deflate(body) + + def test_trailing_bytes_do_not_break_a_zlib_wrapped_body(self): + """The integrity guard belongs only where there is no checksum. + + Requiring the whole input to be consumed gives raw deflate the tell it + lacks, but the zlib wrapper validates itself with an adler32 — applying + the rule there rejected legitimate bodies that arrived with padding. + """ + import zlib as zlib_module + + from FlightRadarAPI.request import _decompress_deflate + + body = b'{"rows": [{"name": "Guarulhos"}]}' * 20 + + for trailer in (b"\n", b"\x00\x00"): + assert _decompress_deflate(zlib_module.compress(body) + trailer) == body + + @pytest.mark.parametrize("shape", ["zlib-wrapped", "raw"]) + def test_both_deflate_shapes_still_round_trip(self, shape): + import zlib as zlib_module + + from FlightRadarAPI.request import _decompress_deflate + + body = b'{"rows": [{"name": "Guarulhos"}]}' * 50 + + if shape == "zlib-wrapped": + blob = zlib_module.compress(body) + else: + compressor = zlib_module.compressobj(wbits=-15) + blob = compressor.compress(body) + compressor.flush() + + assert _decompress_deflate(blob) == body From 8b83af4af01fc4f6d5379530effe5897f4f45a7e Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 13:51:03 -0300 Subject: [PATCH 17/22] fix: restore two decoding behaviours libcurl used to provide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were capabilities lost when this module took content decoding over, and both surfaced as an unreadable body rather than an error. Padding after a gzip trailer restarted a member and raised, which `__decode_body` then swallowed as "the transport already decoded this" and returned the still-compressed bytes — so gzip'd JSON plus three NUL bytes died in json.loads. libcurl stopped at the stream end and ignored the tail, and the deflate helper already tolerated exactly this for the zlib shape, so the gzip helper was the inconsistency. Another member is only started when the tail actually looks like one. Stacked encodings missed the table entirely: `Content-Encoding: gzip, br` found no single-token decoder, warned, and handed back compressed bytes. The header is split now and the decoders applied in reverse, so gzip, "gzip, br" and "gzip, deflate, br" all round-trip, whitespace and case included. Also, a Set-Cookie whose Domain does not cover the request host was kept as host-only instead of discarded, so a cookie scoped to some other domain was replayed on every later request to that host. RFC 6265 5.3.6 says ignore it. And a module comment still pointed at `_new_curl_handle`, which the per-request rework replaced with `_keep_body_encoded`. --- nodejs/FlightRadarAPI/request.js | 10 ++++- nodejs/tests/testRequestTransport.js | 19 +++++++++ python/FlightRadarAPI/request.py | 57 +++++++++++++++++--------- python/tests/test_request_transport.py | 55 +++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 20 deletions(-) diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index bc6af53..b7f35d0 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -232,6 +232,8 @@ function parseSetCookie(header, url) { storedAt: 0, }; + let rejected = false; + for (const part of attributeParts) { const index = part.indexOf("="); const key = (index < 0 ? part : part.slice(0, index)).trim().toLowerCase(); @@ -250,16 +252,22 @@ function parseSetCookie(header, url) { } else if (key === "domain" && attributeValue) { const domain = attributeValue.replace(/^\./, "").toLowerCase(); + // A dotless domain is a TLD: `Domain=com` would scope the cookie to // every .com host the caller later requests. if (domain.includes(".") && domainMatches(url.hostname, domain)) { cookie.domain = domain; cookie.hostOnly = false; } + else { + // RFC 6265 5.3.6: a Domain that does not cover the host means + // the cookie is discarded, not narrowed back to the host. + rejected = true; + } } } - return cookie; + return rejected ? null : cookie; } /** diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index 6b45eb3..0b1c29d 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -656,3 +656,22 @@ describe("Error responses and slow bodies (offline)", function() { } }); }); + + +describe("Rejected Set-Cookie attributes (offline)", function() { + const { Session } = require("../FlightRadarAPI/request"); + + it("discards a cookie whose Domain does not cover the host", function() { + // RFC 6265 5.3.6 says ignore it, not narrow it back to the host — + // narrowing left the cookie replayed on every later request. + const session = new Session(); + session.__storeCookies("https://cdn.flightradar24.com/x", [ + "evil=1; Domain=evil.example.com; Path=/", + "tld=1; Domain=com; Path=/", + "kept=1; Domain=flightradar24.com; Path=/", + ]); + + expect(session.__cookiesFor("https://cdn.flightradar24.com/")) + .to.deep.equal({ kept: "1" }); + }); +}); diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index 6d00160..133cc35 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -22,7 +22,7 @@ # ratios high enough to exhaust memory from a few kilobytes on the wire. # # Enforcing that needs the compressed bytes, which means taking content decoding -# away from libcurl (see `_new_curl_handle`): left to itself it expands the body +# away from libcurl (see `_keep_body_encoded`): left to itself it expands the body # before any of this code runs, and the only other way to intervene — # `stream=True` — was measured to cost more than the bomb it stops: `timeout` # degrades to a >=1 byte/sec liveness check, which alone rules it out, and the @@ -34,6 +34,7 @@ _ZLIB_WBITS = 15 # zlib-wrapped deflate, as RFC 9110 specifies _RAW_DEFLATE_WBITS = -15 # raw deflate, as many servers actually send _GZIP_WBITS = 31 # 16 + MAX_WBITS: gzip wrapper rather than raw deflate +_GZIP_MAGIC = b"\x1f\x8b" def _bound_download(session: Session, limit: int) -> None: @@ -120,7 +121,11 @@ def _decompress_gzip(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: if not decompressor.eof: raise zlib.error("gzip stream ended mid-member") - remaining = decompressor.unused_data + # Another member only when the tail looks like one. Trailing padding is + # not an error: libcurl stopped at the stream end and ignored it, and + # the deflate helper tolerates the same thing. + tail = decompressor.unused_data + remaining = tail if tail.startswith(_GZIP_MAGIC) else b"" return output @@ -446,26 +451,40 @@ def __is_cloudflare_block(self) -> bool: def __decode_body(self, content: bytes) -> bytes: """Undo the `Content-Encoding` this response arrived with. - Header values are case-insensitive and may carry whitespace, so the - token is normalised before lookup: an exact match would send `GZIP` - down the identity path and return compressed bytes as if they were - content. An encoding this class does not implement warns rather than - passing silently, since nothing downstream can act on the result. + The header may stack encodings ("gzip, br" means gzip then brotli), so + they are undone in reverse. Values are case-insensitive and may carry + whitespace, so each token is normalised: an exact match would send + `GZIP` down the identity path and return compressed bytes as content. + A token with no decoder warns rather than passing silently, since + nothing downstream can act on the result. """ content_encoding = self.__response.headers.get("Content-Encoding", "") or "" - encoding = content_encoding.strip().lower() - decode = self.__content_encodings.get(encoding) - - if decode is None: - _logger.warning( - "APIRequest: no decoder for Content-Encoding=%r on %s. Returning the body " - "as received, which callers will not be able to read.", - content_encoding, self.url, - ) + tokens = [token.strip().lower() for token in content_encoding.split(",")] + applied = [token for token in tokens if token and token != "identity"] + + if not applied: return content + decoders = [] + + for token in applied: + decode = self.__content_encodings.get(token) + + if decode is None: + _logger.warning( + "APIRequest: no decoder for Content-Encoding=%r on %s. Returning the body " + "as received, which callers will not be able to read.", + content_encoding, self.url, + ) + return content + + decoders.append(decode) + try: - return decode(content, self.__max_response_bytes) + for decode in reversed(decoders): + content = decode(content, self.__max_response_bytes) + + return content except DecompressionLimitError: raise except Exception as err: @@ -485,8 +504,8 @@ def __decode_body(self, content: bytes) -> bytes: except UnicodeDecodeError: transport_decoded = False else: - corrupt_gzip = encoding == "gzip" and content.startswith(b"\x1f\x8b") - transport_decoded = encoding in ("gzip", "br", "deflate") and not corrupt_gzip + corrupt_gzip = applied[-1] == "gzip" and content.startswith(_GZIP_MAGIC) + transport_decoded = applied[-1] in ("gzip", "br", "deflate") and not corrupt_gzip _logger.log( logging.DEBUG if transport_decoded else logging.WARNING, diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index ad3058b..3e4cd3f 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -736,3 +736,58 @@ def test_both_deflate_shapes_still_round_trip(self, shape): blob = compressor.compress(body) + compressor.flush() assert _decompress_deflate(blob) == body + + +class TestStackedAndPaddedEncodings: + """libcurl handled both before this module took decoding over.""" + + def _serve(self, blob: bytes, encoding: str): + return TestBudgetAgainstARealTransport._serve(blob, encoding) + + @pytest.mark.parametrize("header", ["gzip, br", " gzip , br ", "gzip, deflate, br"]) + def test_stacked_encodings_are_undone_in_reverse(self, header): + import gzip as gzip_module + import json + import zlib as zlib_module + + import brotli + + from FlightRadarAPI.request import APIClient + + payload = {"rows": [{"n": "GRU"}] * 20} + blob = gzip_module.compress(json.dumps(payload).encode()) + + if "deflate" in header: + blob = zlib_module.compress(blob) + + blob = brotli.compress(blob) + server = self._serve(blob, header) + + try: + response = APIClient().request( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, deflate, br"}, + ) + assert response.get_json_content() == payload + finally: + server.shutdown() + + def test_padding_after_the_gzip_trailer_is_ignored(self): + """A stray tail must not restart a member and fail the whole body. + + The deflate helper already tolerated this for the zlib shape, so the + gzip helper being strict was an inconsistency, not a policy. + """ + import gzip as gzip_module + + from FlightRadarAPI.request import _decompress_gzip + + body = b'{"rows": []}' + + for trailer in (b"\x00\x00\x00", b"\n"): + assert _decompress_gzip(gzip_module.compress(body) + trailer) == body + + # Still reads a genuine second member rather than stopping at the first. + assert _decompress_gzip( + gzip_module.compress(b"first") + gzip_module.compress(b"second"), + ) == b"firstsecond" From 06c5af6e2082037cae2932ee7eec6dfc5905b7de Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 16:28:08 -0300 Subject: [PATCH 18/22] fix: restore the public cookie shape and bank cookies from blocked responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Object.create(null)` on the result's `cookies` map broke `cookies.hasOwnProperty(name)` in consumer code — a silent breaking change under a type that still says `Record`, shipping as a patch. That map is public; the prototype hardening belongs on the jar's internal maps, which is where it stays. The Cloudflare challenge page skipped the BOM strip every other body path goes through, so `err.body` could start with U+FEFF and defeat a JSON.parse or a prefix comparison. A stacked `Content-Encoding` that failed halfway returned the half-decoded intermediate as if it were the body, and picked its log level from the stage that had succeeded rather than the one that failed. The recovery is a claim about the body as received, so that is what it returns now. And cookies on an error response were dropped: `Session.request` only banked them on success, so a Cloudflare 403 — the response that hands out `cf_clearance` — left the jar empty and RetryPolicy replayed the identical blocked request. The errors carry their `Set-Cookie` headers now and the session stores them before rethrowing. --- nodejs/FlightRadarAPI/request.js | 44 +++++++++---- nodejs/tests/testRequestTransport.js | 88 ++++++++++++++++++++++++++ python/FlightRadarAPI/request.py | 14 +++- python/tests/test_request_transport.py | 25 ++++++++ 4 files changed, 155 insertions(+), 16 deletions(-) diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index b7f35d0..7665402 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -401,20 +401,28 @@ async function request(url, { clearTimeout(timer); } const statusCode = response.status; + const rawCookies = response.headers.getSetCookie() ?? []; + + // Attached to the errors below so the session can still bank them: a + // Cloudflare 403 is exactly the response that carries `cf_clearance`, and + // discarding it makes the retry replay the same blocked request. + const withCookies = (error) => Object.assign(error, { rawCookies }); // Cloudflare detection only when the caller did not opt-in to this status code. // `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. if (!allowedErrorCodes.includes(statusCode) && isCloudflareBlock(statusCode, response.headers)) { - throw new CloudflareError( + throw withCookies(new CloudflareError( "Blocked by Cloudflare. Perhaps you are making too many calls, " + "or the TLS impersonation needs to be updated.", response, - body.toString("utf-8"), - ); + decodeText(body), + )); } if (!allowedErrorCodes.includes(statusCode) && (statusCode < 200 || statusCode >= 300)) { - throw new Error(`Received status code '${statusCode}: ${response.statusText}' for the URL ${url}`); + throw withCookies( + new Error(`Received status code '${statusCode}: ${response.statusText}' for the URL ${url}`), + ); } const contentType = response.headers.get("content-type") ?? ""; @@ -430,10 +438,10 @@ async function request(url, { content = body.buffer.slice(body.byteOffset, body.byteOffset + body.byteLength); } - const rawCookies = response.headers.getSetCookie() ?? []; - // Null-prototype: a cookie named `toString` must read as absent, not as an - // inherited function, and one named `__proto__` must not vanish. - const responseCookies = Object.create(null); + // A plain object, unlike the jar's internal maps: this one is public and + // typed `Record`, and a null prototype would break + // `cookies.hasOwnProperty(name)` in consumer code. + const responseCookies = {}; for (const header of rawCookies) { const pair = String(header).split(";")[0]; @@ -598,11 +606,21 @@ class Session { const merged = Object.assign(Object.create(null), this.__cookiesFor(url), extraCookies ?? {}); const cookies = Object.keys(merged).length > 0 ? merged : null; - const result = await request(url, { - dispatcher: this.__dispatcher, - ...rest, - cookies, - }); + let result; + + try { + result = await request(url, { + dispatcher: this.__dispatcher, + ...rest, + cookies, + }); + } + catch (err) { + // Banked even on failure: the response that blocks a request is + // the one that hands out the cookie needed to pass next time. + if (err.rawCookies) this.__storeCookies(url, err.rawCookies); + throw err; + } this.__storeCookies(result.url || url, result.rawCookies); diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index 0b1c29d..277b50c 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -675,3 +675,91 @@ describe("Rejected Set-Cookie attributes (offline)", function() { .to.deep.equal({ kept: "1" }); }); }); + + +describe("Public shapes and the error path (offline)", function() { + const http = require("http"); + const { request, Session } = require("../FlightRadarAPI/request"); + const { CloudflareError } = require("../FlightRadarAPI/errors"); + + /** + * @param {Function} handler - node request handler + * @return {Promise} the listening server + */ + function serve(handler) { + return new Promise((resolve) => { + const server = http.createServer(handler); + server.listen(0, "127.0.0.1", () => resolve(server)); + }); + } + + it("returns cookies on an ordinary object", async function() { + // Typed `Record` and public, so a null prototype would + // break `cookies.hasOwnProperty(name)` in a patch release. + const server = await serve((req, res) => { + res.setHeader("Set-Cookie", "a=1; Path=/"); + res.setHeader("Content-Type", "application/json"); + res.end("{}"); + }); + + try { + const { cookies } = await request(`http://127.0.0.1:${server.address().port}/`); + expect(cookies.hasOwnProperty("a")).to.equal(true); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); + + it("strips a BOM from the Cloudflare challenge page", async function() { + const page = "challenge"; + const body = Buffer.concat([Buffer.from([0xEF, 0xBB, 0xBF]), Buffer.from(page)]); + const server = await serve((req, res) => { + res.writeHead(403, { + "cf-mitigated": "challenge", + "content-type": "text/html", + "content-length": String(body.length), + }); + res.end(body); + }); + + try { + await request(`http://127.0.0.1:${server.address().port}/`); + expect.fail("should have raised CloudflareError"); + } + catch (err) { + expect(err).to.be.instanceOf(CloudflareError); + expect(err.body).to.equal(page); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); + + it("banks cookies handed out by a blocked response", async function() { + // A Cloudflare 403 is the response that carries cf_clearance; dropping + // it makes the retry replay the same blocked request. + const server = await serve((req, res) => { + res.writeHead(403, { + "cf-mitigated": "challenge", + "set-cookie": "cf_clearance=token; Path=/", + "content-type": "text/html", + "content-length": "2", + }); + res.end("hi"); + }); + + try { + const session = new Session(); + await session.request(`http://127.0.0.1:${server.address().port}/`).catch(() => {}); + + expect(session.getCookie("cf_clearance")).to.equal("token"); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); +}); diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index 133cc35..ae9bc8c 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -480,14 +480,22 @@ def __decode_body(self, content: bytes) -> bytes: decoders.append(decode) + received = content + failed_at = applied[-1] + try: - for decode in reversed(decoders): + for token, decode in zip(reversed(applied), reversed(decoders)): + failed_at = token content = decode(content, self.__max_response_bytes) return content except DecompressionLimitError: raise except Exception as err: + # Back to the bytes as received. A chain that failed halfway leaves + # a half-decoded intermediate, and the recovery below is a claim + # about the body that arrived, not about a partial result. + content = received # Reached when the body is not encoded the way the header claims — # most often a transport that decoded it after all. Decided by the # body, not the header: undecodable text is genuinely broken and @@ -504,8 +512,8 @@ def __decode_body(self, content: bytes) -> bytes: except UnicodeDecodeError: transport_decoded = False else: - corrupt_gzip = applied[-1] == "gzip" and content.startswith(_GZIP_MAGIC) - transport_decoded = applied[-1] in ("gzip", "br", "deflate") and not corrupt_gzip + corrupt_gzip = failed_at == "gzip" and content.startswith(_GZIP_MAGIC) + transport_decoded = failed_at in ("gzip", "br", "deflate") and not corrupt_gzip _logger.log( logging.DEBUG if transport_decoded else logging.WARNING, diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index 3e4cd3f..386067c 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -791,3 +791,28 @@ def test_padding_after_the_gzip_trailer_is_ignored(self): assert _decompress_gzip( gzip_module.compress(b"first") + gzip_module.compress(b"second"), ) == b"firstsecond" + + +class TestPartialDecodingChain: + def test_a_chain_that_fails_midway_falls_back_to_the_body_as_received(self): + """The recovery claims the body arrived decoded, so it must return that + body — not the half-processed intermediate of a failed chain.""" + import json + + import brotli + + from FlightRadarAPI.request import APIClient + + # Header claims gzip then br, but only br was applied. + payload = {"ok": True} + blob = brotli.compress(json.dumps(payload).encode()) + server = TestBudgetAgainstARealTransport._serve(blob, "gzip, br") + + try: + response = APIClient().request( + f"http://127.0.0.1:{server.server_port}/", + headers={"accept-encoding": "gzip, br"}, + ) + assert response.get_response_object().content == blob + finally: + server.shutdown() From 42238184d0c4a9b0d150011451ff46e01235731b Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 17:34:42 -0300 Subject: [PATCH 19/22] fix(nodejs): stop the error-path cookies leaking and mis-attributing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are regressions from the commit that started banking cookies off failed responses, and both were reproduced before fixing. `rawCookies` was an enumerable own property on the error, so `JSON.stringify(err)` and any log line dumping it printed the session credentials the response had just handed out — in a branch whose whole point is keeping cookies away from where they do not belong. Defined non-enumerable now: `Object.keys` and `JSON.stringify` no longer see it, while the jar still does. The catch also credited the cookie to the requested URL rather than the one that answered, so a cookie set after a redirect was replayed to a host that never set it. That is the same defect the success path was fixed for earlier in this branch; the final URL travels with the error now. Note, unchanged and pre-existing: `util.inspect(err)` still reveals a Set-Cookie through `err.response`, because exposing the response is what that field is for. Left alone deliberately rather than widened into another change. --- nodejs/FlightRadarAPI/request.js | 25 ++++++++-- nodejs/tests/testRequestTransport.js | 73 ++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 5 deletions(-) diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index 7665402..fdb29fd 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -403,10 +403,25 @@ async function request(url, { const statusCode = response.status; const rawCookies = response.headers.getSetCookie() ?? []; - // Attached to the errors below so the session can still bank them: a - // Cloudflare 403 is exactly the response that carries `cf_clearance`, and - // discarding it makes the retry replay the same blocked request. - const withCookies = (error) => Object.assign(error, { rawCookies }); + /** + * Attach the response's cookies to an error so the session can still bank + * them: a Cloudflare 403 is exactly the response that carries + * `cf_clearance`, and discarding it makes the retry replay the same + * blocked request. + * + * Non-enumerable, and that is the point: an enumerable property puts the + * cookie values into `JSON.stringify(err)` and `util.inspect(err)`, so + * anything that logs the error would print the session credentials. + * + * @param {Error} error + * @return {Error} the same error + */ + const withCookies = (error) => Object.defineProperties(error, { + rawCookies: { value: rawCookies, enumerable: false }, + // The host that answered, which after a redirect is not the host that + // was asked — the jar has to credit the cookie to the right one. + cookieOrigin: { value: response.url || url, enumerable: false }, + }); // Cloudflare detection only when the caller did not opt-in to this status code. // `getAirlineLogo`/`getCountryFlag` allow 403 to mean "asset not found" on the CDN. @@ -618,7 +633,7 @@ class Session { catch (err) { // Banked even on failure: the response that blocks a request is // the one that hands out the cookie needed to pass next time. - if (err.rawCookies) this.__storeCookies(url, err.rawCookies); + if (err.rawCookies) this.__storeCookies(err.cookieOrigin || url, err.rawCookies); throw err; } diff --git a/nodejs/tests/testRequestTransport.js b/nodejs/tests/testRequestTransport.js index 277b50c..a096ec5 100644 --- a/nodejs/tests/testRequestTransport.js +++ b/nodejs/tests/testRequestTransport.js @@ -763,3 +763,76 @@ describe("Public shapes and the error path (offline)", function() { } }); }); + + +describe("Cookies attached to errors (offline)", function() { + const http = require("http"); + const { Session } = require("../FlightRadarAPI/request"); + + it("credits a cookie from a failed response to the host that answered", async function() { + // Regression: the error path attributed it to the requested URL, so a + // cookie set after a redirect was replayed to a host that never set it + // — the same bug the success path was fixed for earlier in this branch. + let port; + const server = http.createServer((req, res) => { + if (req.url === "/start") { + res.writeHead(302, { Location: `http://localhost:${port}/blocked` }); + res.end(); + return; + } + res.writeHead(403, { + "cf-mitigated": "challenge", + "set-cookie": "cf_clearance=token; Path=/", + "content-type": "text/html", + "content-length": "2", + }); + res.end("hi"); + }); + await new Promise((ready) => server.listen(0, () => { + port = server.address().port; + ready(); + })); + + try { + const session = new Session(); + await session.request(`http://127.0.0.1:${port}/start`).catch(() => {}); + + expect(session.__cookiesFor(`http://localhost:${port}/`)).to.deep.equal({ cf_clearance: "token" }); + expect(session.__cookiesFor(`http://127.0.0.1:${port}/`)).to.deep.equal({}); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); + + it("keeps those cookies out of anything that serialises the error", async function() { + // An enumerable property would put session credentials into + // JSON.stringify(err) and any log line that dumps the error. + const server = http.createServer((req, res) => { + res.writeHead(403, { + "cf-mitigated": "challenge", + "set-cookie": "cf_clearance=secret; Path=/", + "content-type": "text/html", + "content-length": "2", + }); + res.end("hi"); + }); + await new Promise((ready) => server.listen(0, "127.0.0.1", ready)); + + try { + await new Session().request(`http://127.0.0.1:${server.address().port}/`); + expect.fail("should have raised"); + } + catch (err) { + expect(Object.keys(err)).to.not.include("rawCookies"); + expect(JSON.stringify(err)).to.not.include("secret"); + // Still reachable for the jar. + expect(err.rawCookies).to.be.an("array"); + } + finally { + server.closeAllConnections?.(); + await new Promise((done) => server.close(done)); + } + }); +}); From 89ac1e6771d11dc06828cd0c1168849d36cc147e Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Thu, 20 Aug 2026 17:44:25 -0300 Subject: [PATCH 20/22] feat!: drop the deprecated FlightRadar24 import alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shim existed so `from FlightRadar24 import FlightRadar24API` kept working after the package was renamed to match the PyPI distribution and the Node package. It has been emitting a DeprecationWarning saying it would be removed in a future release; this is that release. Gone with it: the module itself, its backwards-compatibility tests, its entry in the wheel's package list, and the extra path in the flake8 step. Verified against a built wheel — it now ships `FlightRadarAPI` alone, and `import FlightRadar24` raises ModuleNotFoundError. Version moves to 1.6.0 in both packages, since publish.yml requires them to match. Note for the release: removing a public import path is not a minor change. Anyone still on `FlightRadar24` breaks on upgrade, which semver calls a major bump. 1.6.0 is what was asked for and nothing is irreversible until the tag is cut, but 2.0.0 is the number that matches what this does. --- .github/workflows/python-package.yml | 2 +- nodejs/package-lock.json | 6 ++-- nodejs/package.json | 2 +- python/FlightRadar24/__init__.py | 37 -------------------- python/FlightRadarAPI/__init__.py | 2 +- python/pyproject.toml | 2 +- python/tests/test_legacy_import.py | 51 ---------------------------- 7 files changed, 7 insertions(+), 95 deletions(-) delete mode 100644 python/FlightRadar24/__init__.py delete mode 100644 python/tests/test_legacy_import.py diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index dd79eb3..2ac44a1 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -34,7 +34,7 @@ jobs: pip install -e "./python[tests]" pip install flake8 mypy build pytest-cov pip-audit - name: Lint - run: cd python && python -m flake8 FlightRadarAPI FlightRadar24 tests + run: cd python && python -m flake8 FlightRadarAPI tests - name: Type check run: cd python && python -m mypy FlightRadarAPI --ignore-missing-imports - name: Offline tests (PR gate) diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 09c6e86..753e8cc 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,12 +1,12 @@ { "name": "flightradarapi", - "version": "1.5.4", + "version": "1.6.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "flightradarapi", - "version": "1.5.4", + "version": "1.6.0", "license": "MIT", "dependencies": { "node-html-parser": "^6.1.13", @@ -20,7 +20,7 @@ "tsd": "^0.31.0" }, "engines": { - "node": ">=18" + "node": ">=18.17" } }, "node_modules/@babel/code-frame": { diff --git a/nodejs/package.json b/nodejs/package.json index d0a0f5b..379cdb1 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "flightradarapi", - "version": "1.5.4", + "version": "1.6.0", "description": "SDK for FlightRadar24", "main": "./FlightRadarAPI/index.js", "types": "./FlightRadarAPI/index.d.ts", diff --git a/python/FlightRadar24/__init__.py b/python/FlightRadar24/__init__.py deleted file mode 100644 index ccf748f..0000000 --- a/python/FlightRadar24/__init__.py +++ /dev/null @@ -1,37 +0,0 @@ -# -*- coding: utf-8 -*- - -""" -Deprecated import alias for the FlightRadarAPI SDK. - -The package was renamed to ``FlightRadarAPI`` so the Python import name -matches the PyPI distribution name and the Node.js package name. This -module re-exports the public API and aliases every submodule so legacy -imports such as ``from FlightRadar24 import FlightRadar24API`` or -``from FlightRadar24.errors import CloudflareError`` keep working, but a -``DeprecationWarning`` is emitted on import. -""" - -import importlib -import pkgutil -import sys -import warnings - -import FlightRadarAPI as _pkg - -warnings.warn( - "Importing from 'FlightRadar24' is deprecated and will be removed in a " - "future release. Import from 'FlightRadarAPI' instead " - "(e.g. 'from FlightRadarAPI import FlightRadar24API').", - DeprecationWarning, - stacklevel=2, -) - -# Mirror every submodule of FlightRadarAPI under the legacy FlightRadar24 -# namespace so dotted imports keep resolving without touching disk. -for _info in pkgutil.walk_packages(_pkg.__path__, prefix=f"{_pkg.__name__}."): - _mod = importlib.import_module(_info.name) - sys.modules[_info.name.replace(_pkg.__name__, __name__, 1)] = _mod -del _info, _mod - -from FlightRadarAPI import * # noqa: E402, F401, F403 -from FlightRadarAPI import __all__, __author__, __version__ # noqa: E402, F401 diff --git a/python/FlightRadarAPI/__init__.py b/python/FlightRadarAPI/__init__.py index e48a9fe..5288e42 100644 --- a/python/FlightRadarAPI/__init__.py +++ b/python/FlightRadarAPI/__init__.py @@ -12,7 +12,7 @@ """ __author__ = "Jean Loui Bernard Silva de Jesus" -__version__ = "1.5.4" +__version__ = "1.6.0" from .api import FlightRadar24API from .core import Countries diff --git a/python/pyproject.toml b/python/pyproject.toml index 19cadd7..bf1ef3d 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -28,7 +28,7 @@ dependencies = [ exclude = ["tests", ".flake8"] [tool.hatch.build.targets.wheel] -packages = ["FlightRadarAPI", "FlightRadar24"] +packages = ["FlightRadarAPI"] # Force-include the PEP 561 marker so downstream type checkers always see it, # regardless of future changes to the global exclude list above. diff --git a/python/tests/test_legacy_import.py b/python/tests/test_legacy_import.py deleted file mode 100644 index 2cd78c8..0000000 --- a/python/tests/test_legacy_import.py +++ /dev/null @@ -1,51 +0,0 @@ -# -*- coding: utf-8 -*- -"""Backwards-compatibility tests for the deprecated `FlightRadar24` alias. - -The package was renamed to `FlightRadarAPI`, but `FlightRadar24` is kept as -a thin shim so existing user code keeps working through one release. These -tests guard the deprecation contract so that removing the shim later is a -deliberate decision, not an accidental regression. -""" - -import importlib -import sys -import warnings - - -def _reimport(name: str): - """Drop any cached copy of the module so `import` re-runs side effects.""" - for key in list(sys.modules): - if key == name or key.startswith(f"{name}."): - del sys.modules[key] - return importlib.import_module(name) - - -class TestLegacyAlias: - def test_import_emits_deprecation_warning(self): - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") - _reimport("FlightRadar24") - deprecations = [w for w in caught if issubclass(w.category, DeprecationWarning)] - assert deprecations, "Expected a DeprecationWarning from FlightRadar24 import" - assert "FlightRadarAPI" in str(deprecations[0].message) - - def test_public_api_is_re_exported(self): - legacy = _reimport("FlightRadar24") - new = importlib.import_module("FlightRadarAPI") - assert legacy.FlightRadar24API is new.FlightRadar24API - assert legacy.Countries is new.Countries - assert legacy.__version__ == new.__version__ - - def test_submodule_imports_resolve_to_new_package(self): - _reimport("FlightRadar24") - # Legacy submodules are registered dynamically via sys.modules in the - # FlightRadar24 shim, so static analyzers cannot resolve them. - from FlightRadar24.errors import CloudflareError as LegacyError # type: ignore[import-not-found] - from FlightRadarAPI.errors import CloudflareError as NewError - assert LegacyError is NewError - - def test_nested_subpackage_imports_resolve(self): - _reimport("FlightRadar24") - from FlightRadar24.entities.airport import Airport as LegacyAirport # type: ignore[import-not-found] - from FlightRadarAPI.entities.airport import Airport as NewAirport - assert LegacyAirport is NewAirport From 5d25e38d66fbfff0588299fb3a59e6b480f63315 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Fri, 21 Aug 2026 00:25:17 -0300 Subject: [PATCH 21/22] perf: collect gzip members instead of concatenating, and fix stale docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From a Copilot review of the PR, all three verified before acting. `_decompress_gzip` accumulated with `output += ...`, which copies everything decoded so far on every member: 1270ms for a 500-member body against 25ms. The review suggested a bytearray, but measuring says that is slower than the status quo on the single-member path nearly every response takes (23.1ms vs 15.2ms on 32 MiB), because converting back to bytes pays another full copy. Collecting the members and joining once wins both cases — 11.9ms and 19.0ms — and returns the sole member untouched when there is only one. The JSDoc on `request()` and the three methods wrapping it still described the old `{content, statusCode, cookies}` return, four places in all, after `rawCookies` and `url` were added. And the docs pin file justified itself with `contents: write`, which that workflow no longer has — the deploy moved to the Pages artifact earlier in this branch. The reason for pinning still stands, so the wording says what it actually is: those packages execute during `mkdocs build`. --- .github/docs-requirements.txt | 6 +++--- nodejs/FlightRadarAPI/request.js | 8 ++++---- python/FlightRadarAPI/request.py | 16 ++++++++++++---- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/.github/docs-requirements.txt b/.github/docs-requirements.txt index 1e580f2..383ae2d 100644 --- a/.github/docs-requirements.txt +++ b/.github/docs-requirements.txt @@ -1,5 +1,5 @@ -# Pinned because this workflow runs with `contents: write`: an automatic -# upgrade to a compromised release would execute with a token that can push -# to the repository. Bump deliberately, not implicitly. +# Pinned so a compromised release cannot be pulled in automatically: these +# packages execute during `mkdocs build`, inside the job that publishes the +# docs site. Bump deliberately, not implicitly. mkdocs-material==9.7.7 mkdocs-git-committers-plugin-2==2.5.0 diff --git a/nodejs/FlightRadarAPI/request.js b/nodejs/FlightRadarAPI/request.js index fdb29fd..a2db4b9 100644 --- a/nodejs/FlightRadarAPI/request.js +++ b/nodejs/FlightRadarAPI/request.js @@ -337,7 +337,7 @@ function isCloudflareBlock(statusCode, headers) { * @param {Array} [options.allowedErrorCodes=[]] - Status codes that should not throw * @param {number} [options.timeout=30000] - Request timeout in milliseconds * @param {number} [options.maxResponseBytes] - Maximum accepted response body size - * @return {Promise<{content: *, statusCode: number, cookies: object}>} + * @return {Promise<{content: *, statusCode: number, cookies: object, rawCookies: Array, url: string}>} */ async function request(url, { params = null, @@ -612,7 +612,7 @@ class Session { * * @param {string} url * @param {object} [options={}] - * @return {Promise<{content: *, statusCode: number, cookies: object}>} + * @return {Promise<{content: *, statusCode: number, cookies: object, rawCookies: Array, url: string}>} */ async request(url, options = {}) { const { cookies: extraCookies, ...rest } = options; @@ -666,7 +666,7 @@ class APIClient { * * @param {string} url * @param {object} [options={}] - * @return {Promise<{content: *, statusCode: number, cookies: object}>} + * @return {Promise<{content: *, statusCode: number, cookies: object, rawCookies: Array, url: string}>} */ async request(url, options = {}) { return runWithRetry(() => this.__session.request(url, options), this.__retry); @@ -681,7 +681,7 @@ class APIClient { * * @param {string} url * @param {object} [options={}] - * @return {Promise<{content: *, statusCode: number, cookies: object}>} + * @return {Promise<{content: *, statusCode: number, cookies: object, rawCookies: Array, url: string}>} */ async requestStandalone(url, options = {}) { return runWithRetry( diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index ae9bc8c..e552acb 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -104,16 +104,24 @@ def _decompress_gzip(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: tell truncated JSON from a malformed feed, and `get_content` treats a raised error as "the transport already decoded this" and hands back the raw bytes. """ - output = b"" + # Collected rather than concatenated: `output += ...` copies everything so + # far on each member, which is quadratic and measured 50x slower on a + # 500-member body. Joining also beats a bytearray, which pays one more + # full copy converting back to bytes on the single-member path that + # nearly every response takes. + members = [] + decoded = 0 remaining = data while remaining: # A new object per member: `Content-Encoding: gzip` may carry several, # and one decompressor stops at the first trailer. decompressor = zlib.decompressobj(_GZIP_WBITS) - output += decompressor.decompress(remaining, limit + 1 - len(output)) + member = decompressor.decompress(remaining, limit + 1 - decoded) + members.append(member) + decoded += len(member) - if len(output) > limit or decompressor.unconsumed_tail: + if decoded > limit or decompressor.unconsumed_tail: raise DecompressionLimitError( f"gzip body expands past the {limit} byte decompression limit." ) @@ -127,7 +135,7 @@ def _decompress_gzip(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: tail = decompressor.unused_data remaining = tail if tail.startswith(_GZIP_MAGIC) else b"" - return output + return members[0] if len(members) == 1 else b"".join(members) def _decompress_brotli(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: From 9964a626ec4fef845a7af1862bdca81a2a263415 Mon Sep 17 00:00:00 2001 From: JeanExtreme002 Date: Fri, 21 Aug 2026 00:41:34 -0300 Subject: [PATCH 22/22] perf(python): stop holding the decoded body three times over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The brotli helper's docstring claimed peak memory stays near the limit. Measured against a 7 MiB body under an 8 MiB cap, it was 3.0x the output: growing a bytearray copies each piece in, and converting back to bytes copies the whole thing again. Collecting the pieces and joining — the shape the gzip helper already uses — brings that to 1.0x, because the single-piece response almost every reply produces is returned without being copied at all. With the 64 MiB default that is the difference between a ~190 MiB peak and a ~64 MiB one, which matters to anyone sizing the limit for a small container. Re-checked the behaviours this must not disturb: empty body, ordinary round-trip, bomb refused, truncated stream still raising at 50/75/90% of a real stream, and the "transport already decoded this" fallback. A test now pins the memory shape, since a correctness test cannot see it. --- python/FlightRadarAPI/request.py | 21 ++++++++++----- python/tests/test_request_transport.py | 36 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/python/FlightRadarAPI/request.py b/python/FlightRadarAPI/request.py index e552acb..91468a3 100644 --- a/python/FlightRadarAPI/request.py +++ b/python/FlightRadarAPI/request.py @@ -142,15 +142,21 @@ def _decompress_brotli(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: """Decompress brotli bytes, refusing a body that expands past ``limit``. ``process`` takes a max-output argument, so the cap is enforced by the - decompressor rather than checked after the fact: peak memory stays near - ``limit`` no matter how far the body would have expanded. + decompressor rather than checked after the fact: the cost tracks ``limit`` + rather than however far the body would have expanded. + + Pieces are collected and joined, as in the gzip helper. Growing a bytearray + and converting it back pays two full copies of the output and measured 3x + the decoded size at peak, against 1x here — the usual single-piece response + is returned without being copied at all. """ decompressor = brotli.Decompressor() - output = bytearray() + pieces = [] + decoded = 0 fed = False while not decompressor.is_finished(): - room = limit + 1 - len(output) + room = limit + 1 - decoded if room <= 0: raise DecompressionLimitError( @@ -159,9 +165,10 @@ def _decompress_brotli(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: piece = decompressor.process(data if not fed else b"", room) fed = True - output += piece + pieces.append(piece) + decoded += len(piece) - if len(output) > limit: + if decoded > limit: raise DecompressionLimitError( f"brotli body expands past the {limit} byte decompression limit." ) @@ -174,7 +181,7 @@ def _decompress_brotli(data: bytes, limit: int = MAX_RESPONSE_BYTES) -> bytes: if not decompressor.is_finished(): raise brotli.error("brotli stream ended mid-message") - return bytes(output) + return pieces[0] if len(pieces) == 1 else b"".join(pieces) class RetryPolicy: diff --git a/python/tests/test_request_transport.py b/python/tests/test_request_transport.py index 386067c..50655c9 100644 --- a/python/tests/test_request_transport.py +++ b/python/tests/test_request_transport.py @@ -816,3 +816,39 @@ def test_a_chain_that_fails_midway_falls_back_to_the_body_as_received(self): assert response.get_response_object().content == blob finally: server.shutdown() + + +class TestDecompressionMemoryShape: + """The budget is only meaningful if the cost tracks it. + + A helper that decodes correctly but holds several copies of the output + turns a 64 MiB budget into a far larger peak, which matters to anyone + sizing the limit for a memory-constrained host. + """ + + @pytest.mark.parametrize("encoding", ["br", "gzip"]) + def test_a_decoded_body_is_not_held_several_times_over(self, encoding): + import gzip as gzip_module + import tracemalloc + + import brotli + + from FlightRadarAPI.request import _decompress_brotli, _decompress_gzip + + body = b"z" * (4 * 1024 * 1024) + blob, decompress = { + "br": (brotli.compress(body), _decompress_brotli), + "gzip": (gzip_module.compress(body), _decompress_gzip), + }[encoding] + + tracemalloc.start() + try: + assert decompress(blob, 8 * 1024 * 1024) == body + _, peak = tracemalloc.get_traced_memory() + finally: + tracemalloc.stop() + + # Accumulating into a bytearray and converting back measured 3x for + # brotli; collecting the pieces and joining returns the usual + # single-piece body without copying it at all. + assert peak < 2.5 * len(body), f"peaked at {peak / len(body):.1f}x the body"