diff --git a/scripts/sh/pack-check.sh b/scripts/sh/pack-check.sh index da4a0c2..00ebb5a 100755 --- a/scripts/sh/pack-check.sh +++ b/scripts/sh/pack-check.sh @@ -150,6 +150,47 @@ if [[ -n "$forbidden" ]]; then exit 1 fi +# The beacon envelope, which has to stand on its own. +# +# `beaconQueryString` is the one thing in this package that runs on every page +# of a measured site. A bundler reaches it through the package root, and the +# root re-exports everything, so what actually decides the page weight is +# which modules the envelope itself pulls in. Its module answers that with +# nothing, and this is what keeps the answer that way. +# +# KensioSoftware/rainlytics#110 is what this is guarding. The SQL reading the +# same parameters back used to sit in the same file, and a minified bundle of +# a beacon-shaped entry carried `url_decode`, `url_extract_parameter` and +# `strpos` that no browser runs. Splitting the file took that bundle from 464 +# bytes to 241. +# +# The forbidden-import check above cannot see this. The root legitimately +# reaches SQL, and every one of those imports is a relative path it allows. + +envelope="$tmp/package/dist/beacon-events.js" + +if [[ ! -f "$envelope" ]]; then + echo "The beacon envelope is not at dist/beacon-events.js." >&2 + echo "Point this check at wherever beaconQueryString moved to." >&2 + exit 1 +fi + +envelope_imports="$( + grep --only-matching --extended-regexp \ + "(from|import)[[:space:]]*\(?[[:space:]]*[\"'][^\"']+[\"']" \ + "$envelope" || true +)" + +if [[ -n "$envelope_imports" ]]; then + echo "The beacon envelope imports something:" >&2 + sed 's/^/ /' <<<"$envelope_imports" >&2 + echo >&2 + echo "Every page of a measured site downloads this module. Whatever it" >&2 + echo "imports is downloaded with it, however little of that a browser" >&2 + echo "runs. SQL belongs in dist/beacon-rows.js, which no browser reaches." >&2 + exit 1 +fi + # The CLI, run out of the tarball. # # Running it proves more than that the file was packed. The extracted tarball diff --git a/src/beacon-events.test.ts b/src/beacon-events.test.ts index 33631ef..84dfa04 100644 --- a/src/beacon-events.test.ts +++ b/src/beacon-events.test.ts @@ -13,13 +13,10 @@ import { deployStacks } from "#test/simulated-deployment.js"; import { runAthenaQuery } from "./athena/athena-query.js"; import { - aBeaconEvent, - beaconEventColumn, beaconParameters, beaconQueryString, beaconSchemaVersion, defaultBeaconPath, - outsideTheBeaconPath, } from "./beacon-events.js"; import { CloudFrontLogDelivery } from "./cdk/log-delivery.js"; import { LogBucket } from "./cdk/log-bucket.js"; @@ -75,35 +72,6 @@ describe("the beacon event envelope", () => { expect(sent.split("&")).toHaveLength(3); }); - it("reads a row's event back off the column CloudFront wrote it to", () => { - // Given the SQL a rollup selects the event with. - // Then it reads the query string, which is where the payload is. No - // column of the table holds it, because a table column is a CloudFront - // field and CloudFront has no field for somebody else's payload. - expect(beaconEventColumn).toContain("cs_uri_query"); - expect(beaconEventColumn).toContain(`'${beaconParameters.event}'`); - }); - - it("counts only requests carrying an envelope", () => { - // Given the conditions a rollup filters beacon rows with. - // Then a request to the beacon's path with no version parameter is left - // out. A crawler that found the URL in a page's source sends one of - // those, and counting it would report an event nobody caused. - expect(aBeaconEvent).toContain("cs_uri_query <> '-'"); - expect(aBeaconEvent.join(" ")).toContain(`'${beaconParameters.version}'`); - }); - - it("names the path where the envelope's own conditions leave it out", () => { - // Given the condition status-codes takes the beacon's requests back out - // with. - // Then it names the path, where `aBeaconEvent` leaves the path to the - // request's own `paths`. The two run in opposite directions. It reads - // the column as delivered, since the path carries nothing a browser or - // CloudFront escapes. - expect(outsideTheBeaconPath).toContain(`'${defaultBeaconPath}'`); - expect(outsideTheBeaconPath).not.toContain("url_decode"); - }); - it("sends to a path a site is unlikely to serve already", () => { // Given the default path. // Then it is one path, absolute, and marked as not a page. Pointing the diff --git a/src/beacon-events.ts b/src/beacon-events.ts index ff4c497..af574da 100644 --- a/src/beacon-events.ts +++ b/src/beacon-events.ts @@ -1,5 +1,4 @@ -// What a beacon event is, and how it survives the round trip through a -// CloudFront access log. +// What a beacon event is, and the query string it travels in. // // The beacon sends a GET to a path on the site's own domain and puts its // payload in the query string. CloudFront records `cs-uri-query` whatever the @@ -31,12 +30,11 @@ // is for the beacon's own issue, and `version` is what lets that arrive // without reinterpreting rows already written. // -// Both halves of the package read this. The beacon builds a query string from -// it in a browser, so nothing here may reach a Node built-in or `aws-cdk-lib`, -// and a rollup reads the same parameters back as SQL. - -import { decodedParameter } from "./log-encoding.js"; -import { quoted } from "./sql-text.js"; +// This module is the browser's half of that definition, and it imports +// nothing. Every page of a measured site downloads it, so the SQL reading +// these same parameters back off a row lives in `beacon-rows.ts` next door. +// KensioSoftware/rainlytics#110 has what keeping the two together cost a +// bundle. /** * The path a beacon reports to, where a site chooses none. @@ -106,7 +104,7 @@ export interface BeaconEvent { * * The browser's own encoding, which is the single pass a request carries. * CloudFront adds its own on the way into the record, and - * {@link beaconEventColumn} reads both back off. + * `beaconEventColumn` in `beacon-rows.ts` reads both back off. * * No leading `?`. The caller joins it to the path it is sending to. * @@ -125,53 +123,3 @@ export function beaconQueryString(event: BeaconEvent): string { ) .join("&"); } - -/** The envelope version a row was written under, as SQL. */ -export const beaconVersionColumn = decodedParameter(beaconParameters.version); - -/** What happened, as SQL. */ -export const beaconEventColumn = decodedParameter(beaconParameters.event); - -/** The page it happened on, as SQL. */ -export const beaconPageColumn = decodedParameter(beaconParameters.page); - -/** - * The rows a beacon event is, as conditions for `rowsFor`. - * - * The path is not among them. A rollup narrows to the beacon's path through - * the request's own `paths`, the way any other question narrows to a section - * of a site, and a site that moved its beacon then says so in one place. - * - * These leave out anything else reaching the same path. A crawler following - * a beacon URL out of a page's source carries no version parameter, and the - * bot filter `rowsFor` applies has already taken most of them. - * - * ```typescript - * rowsFor({ ...request, paths: [defaultBeaconPath] }, aBeaconEvent); - * ``` - */ -export const aBeaconEvent: readonly string[] = [ - "cs_method = 'GET'", - "cs_uri_query <> '-'", - `${beaconVersionColumn} <> ''`, -]; - -/** - * The rows outside the beacon's path, as a condition for `rowsFor`. - * - * The other direction from {@link aBeaconEvent}, and it names the path that - * one leaves out. A question about beacon events narrows to the beacon - * through the request's own `paths`. A question about what the site answered - * has to take the beacon's requests back out, and `status-codes` is the one - * that does. - * - * Matched against the column as CloudFront delivered it, where `--path` - * decodes twice first. The path here is a constant this package chose and it - * carries nothing a browser or CloudFront escapes, so a record holds it as it - * was sent. An address somebody typed can hold anything. - * - * A prefix, the way every path match in Rainlytics is one. - */ -export const outsideTheBeaconPath = `strpos(cs_uri_stem, ${quoted( - defaultBeaconPath, -)}) <> 1`; diff --git a/src/beacon-rollup.ts b/src/beacon-rollup.ts index 9b2aed8..9fa4acb 100644 --- a/src/beacon-rollup.ts +++ b/src/beacon-rollup.ts @@ -30,12 +30,12 @@ // exported for a site running the beacon to add to its own summaries, and a // deployment with no beacon computes nothing for it. +import { defaultBeaconPath } from "./beacon-events.js"; import { aBeaconEvent, beaconEventColumn, beaconPageColumn, - defaultBeaconPath, -} from "./beacon-events.js"; +} from "./beacon-rows.js"; import { qualifiedTableName } from "./dataset.js"; import type { Rollup, RollupTotals } from "./rollups.js"; import { rowsFor } from "./rollups.js"; diff --git a/src/beacon-rows.test.ts b/src/beacon-rows.test.ts new file mode 100644 index 0000000..1f1b554 --- /dev/null +++ b/src/beacon-rows.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; + +import { beaconParameters, defaultBeaconPath } from "./beacon-events.js"; +import { + aBeaconEvent, + beaconEventColumn, + outsideTheBeaconPath, +} from "./beacon-rows.js"; + +describe("reading a beacon event off a row", () => { + it("reads a row's event back off the column CloudFront wrote it to", () => { + // Given the SQL a rollup selects the event with. + // Then it reads the query string, which is where the payload is. No + // column of the table holds it, because a table column is a CloudFront + // field and CloudFront has no field for somebody else's payload. + expect(beaconEventColumn).toContain("cs_uri_query"); + expect(beaconEventColumn).toContain(`'${beaconParameters.event}'`); + }); + + it("counts only requests carrying an envelope", () => { + // Given the conditions a rollup filters beacon rows with. + // Then a request to the beacon's path with no version parameter is left + // out. A crawler that found the URL in a page's source sends one of + // those, and counting it would report an event nobody caused. + expect(aBeaconEvent).toContain("cs_uri_query <> '-'"); + expect(aBeaconEvent.join(" ")).toContain(`'${beaconParameters.version}'`); + }); + + it("names the path where the envelope's own conditions leave it out", () => { + // Given the condition status-codes takes the beacon's requests back out + // with. + // Then it names the path, where `aBeaconEvent` leaves the path to the + // request's own `paths`. The two run in opposite directions. It reads + // the column as delivered, since the path carries nothing a browser or + // CloudFront escapes. + expect(outsideTheBeaconPath).toContain(`'${defaultBeaconPath}'`); + expect(outsideTheBeaconPath).not.toContain("url_decode"); + }); +}); diff --git a/src/beacon-rows.ts b/src/beacon-rows.ts new file mode 100644 index 0000000..f600825 --- /dev/null +++ b/src/beacon-rows.ts @@ -0,0 +1,67 @@ +// Reading a beacon event back off the rows CloudFront wrote. +// +// Apart from `beacon-events.ts` because the two halves have different +// readers. That module builds a query string inside a browser and this one +// builds SQL for Athena, and a site folding the beacon into its own bundle +// was carrying both. KensioSoftware/rainlytics#110 measured what that cost +// and split them. +// +// The payload stays in `cs_uri_query` and is read at query time, the way +// `searches` already reads a search term out of the same column. #100 settled +// that, and `beacon-events.ts` carries the rest of the reasoning along with +// the parameter names these expressions read. + +import { decodedParameter } from "./log-encoding.js"; +import { quoted } from "./sql-text.js"; + +import { beaconParameters, defaultBeaconPath } from "./beacon-events.js"; + +/** The envelope version a row was written under, as SQL. */ +export const beaconVersionColumn = decodedParameter(beaconParameters.version); + +/** What happened, as SQL. */ +export const beaconEventColumn = decodedParameter(beaconParameters.event); + +/** The page it happened on, as SQL. */ +export const beaconPageColumn = decodedParameter(beaconParameters.page); + +/** + * The rows a beacon event is, as conditions for `rowsFor`. + * + * The path is not among them. A rollup narrows to the beacon's path through + * the request's own `paths`, the way any other question narrows to a section + * of a site, and a site that moved its beacon then says so in one place. + * + * These leave out anything else reaching the same path. A crawler following + * a beacon URL out of a page's source carries no version parameter, and the + * bot filter `rowsFor` applies has already taken most of them. + * + * ```typescript + * rowsFor({ ...request, paths: [defaultBeaconPath] }, aBeaconEvent); + * ``` + */ +export const aBeaconEvent: readonly string[] = [ + "cs_method = 'GET'", + "cs_uri_query <> '-'", + `${beaconVersionColumn} <> ''`, +]; + +/** + * The rows outside the beacon's path, as a condition for `rowsFor`. + * + * The other direction from {@link aBeaconEvent}, and it names the path that + * one leaves out. A question about beacon events narrows to the beacon + * through the request's own `paths`. A question about what the site answered + * has to take the beacon's requests back out, and `status-codes` is the one + * that does. + * + * Matched against the column as CloudFront delivered it, where `--path` + * decodes twice first. The path here is a constant this package chose and it + * carries nothing a browser or CloudFront escapes, so a record holds it as it + * was sent. An address somebody typed can hold anything. + * + * A prefix, the way every path match in Rainlytics is one. + */ +export const outsideTheBeaconPath = `strpos(cs_uri_stem, ${quoted( + defaultBeaconPath, +)}) <> 1`; diff --git a/src/cdk/log-table.test.ts b/src/cdk/log-table.test.ts index 7024d6e..3cce127 100644 --- a/src/cdk/log-table.test.ts +++ b/src/cdk/log-table.test.ts @@ -10,13 +10,12 @@ import { describe, expect, it } from "vitest"; import { deployStacks, simStartedAt } from "#test/simulated-deployment.js"; import { defaultLogDataset, qualifiedTableName } from "../dataset.js"; +import { beaconQueryString, defaultBeaconPath } from "../beacon-events.js"; import { beaconEventColumn, beaconPageColumn, - beaconQueryString, beaconVersionColumn, - defaultBeaconPath, -} from "../beacon-events.js"; +} from "../beacon-rows.js"; import { deliveredLogColumnNames, logFieldNamesWithoutAddress, diff --git a/src/index.ts b/src/index.ts index 2520edf..15eca4d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -74,17 +74,19 @@ export { windowRange, } from "./summary-runs.js"; export { - aBeaconEvent, type BeaconEvent, - beaconEventColumn, - beaconPageColumn, beaconParameters, beaconQueryString, beaconSchemaVersion, - beaconVersionColumn, defaultBeaconPath, - outsideTheBeaconPath, } from "./beacon-events.js"; +export { + aBeaconEvent, + beaconEventColumn, + beaconPageColumn, + beaconVersionColumn, + outsideTheBeaconPath, +} from "./beacon-rows.js"; export { decodedColumn, decodedParameter } from "./log-encoding.js"; export { lastRange, diff --git a/src/rollup-questions.ts b/src/rollup-questions.ts index 1e98511..c94254a 100644 --- a/src/rollup-questions.ts +++ b/src/rollup-questions.ts @@ -6,7 +6,8 @@ // rollup that filtered differently would answer a different question from // its neighbours without saying so. -import { defaultBeaconPath, outsideTheBeaconPath } from "./beacon-events.js"; +import { defaultBeaconPath } from "./beacon-events.js"; +import { outsideTheBeaconPath } from "./beacon-rows.js"; import { qualifiedTableName } from "./dataset.js"; import { decodedColumn, decodedParameter } from "./log-encoding.js"; import { diff --git a/src/rollups.test.ts b/src/rollups.test.ts index e2846b5..95e2884 100644 --- a/src/rollups.test.ts +++ b/src/rollups.test.ts @@ -3,7 +3,8 @@ import { describe, expect, it } from "vitest"; import { defaultLogDataset, qualifiedTableName } from "./dataset.js"; import { decodedParameter } from "./log-encoding.js"; -import { defaultBeaconPath, outsideTheBeaconPath } from "./beacon-events.js"; +import { defaultBeaconPath } from "./beacon-events.js"; +import { outsideTheBeaconPath } from "./beacon-rows.js"; import { rollups } from "./rollup-questions.js"; import type { Rollup, RollupRequest } from "./rollups.js"; import {