Scope the factorioprints API passthrough to the host that serves the API

[motlin]

Follow-up to #276, addressing the review it got there.

What this actually buys. Not a link that starts working — a factorioprints.com/api/... link fails either way, and the honest framing is that this decides which failure. Passed through, it fetches the site's own index.html and dies in pako as incorrect header check. Sent down the firebase rewrite, it fetches blueprints/blueprintData.json, which answers 200 with a body of null, and data.blueprintString throws. What the change buys is that .com keeps the behaviour it had before #276 rather than quietly acquiring a new one, and that the passthrough says out loud which host it is for.

The finding. The factorioprints arm is reached by first hostname label alone, so the /api/ passthrough #276 added fires for factorioprints.com as well as factorioprints.xyz. Only .xyz is the API; factorioprints.com is the site, and answers its own index.html at status 200 for an /api/ path. Against the deployed editor:

?source=https://factorioprints.xyz/api/blueprintData/9030bab…/position/1.0   197 entities, the nested leaf
?source=https://factorioprints.com/api/blueprintData/9030bab…/position/1.0   Error: incorrect header check

The nit. www.factorioprints.xyz joins the allowlist, and the host check strips a leading www. so both spellings pass through. Not hypothetical — that host serves the same API:

$ curl -s https://www.factorioprints.xyz/api/blueprintDetails/-Kn2afLokZdBO-uHcIAF | head -c 60
{"key":"-Kn2afLokZdBO-uHcIAF","version":{"number":59,…

Tests

Four cases now cover the arm, one per clause of the condition:

case	dies if you drop
factorioprints.com/view/<key> → firebase (pre-existing)	the whole arm
factorioprints.com/api/... → firebase	the host check
factorioprints.xyz/view/<key> → firebase	pathParts[0] === 'api'
www.factorioprints.xyz/api/... → passthrough	.replace(/^www\./, '')

Each of the two new ones was checked red against the mutation it is there for. tests/corsproxy.test.ts also gains www.factorioprints.xyz in the pass-through host list.

vp check, vp test and the full blueprint-sources spec pass locally.

[wormeyman]

The reasoning holds and both claims check out. I confirmed them separately: www.factorioprints.xyz answers the API at 200 with JSON, and factorioprints.com/api/blueprintData/... answers 6.4 KB of SPA index.html at 200.

Two things before it goes in, both about coverage rather than the code.

The change un-tests the clause it does not touch. The condition has two halves and only one of them is pinned now. I simulated each single-clause mutation against the three factorioprints tests:

mutation	caught?
revert the whole change	caught, by your new test
drop the host check	caught
drop pathParts[0] === 'api'	survives all three
drop .replace(/^www\./, '')	survives all three

On the base branch, deleting the path check failed factorioprints reads blueprintString off the firebase record, because the arm then passed everything through. The new host clause excludes .com on its own, so nothing holds the path clause up any more.

Nothing covers the www. strip. www.factorioprints appears under tests/ only at tests/corsproxy.test.ts:78, which asserts allowlist membership and never drives the host through bpString.ts. So the behaviour your allowlist line exists to support is asserted nowhere in the editor, and a www. API link would quietly take the firebase rewrite instead.

Two cases close both, and each one catches exactly one of the surviving mutations:

• https://factorioprints.xyz/view/abc987, expect the firebase rewrite
• https://www.factorioprints.xyz/api/blueprintData/xyz321/position/1.0, expect passthrough

Three smaller notes.

bpString.ts:302-303 still says this is "the same fork as the factorio.school arm below, and for the same reason". It is not any more, since one arm checks the host and the other does not. Worth a sentence saying which is which.

The comment at :310-311 says the fetch "fails outright". It does not. https://facorio-blueprints.firebaseio.com/blueprints/blueprintData.json answers 200 with the body null, so data.blueprintString throws a TypeError and the user gets "Blueprint string could not be loaded." That is worth fixing in the PR body too: both paths fail, so this swaps one error message for another rather than making a link work. The wins are the scoping itself and the www.xyz allowlist entry.

The commit body is empty. GitHub seeds the squash description from it, so the reasoning you put in the code comments will not reach the merged commit.

On window.__fbe_test: correct, and I checked it against a built bundle rather than the source. Worth knowing the repo already records it. The comment at packages/website/src/index.ts:435 says the hook is assigned unconditionally and sits on window in production, and exportGuardResult was rewritten for exactly that reason, since the earlier version performed a real clipboard write and PNG download for any script that called it. So it is a known state rather than a surprise. Filed as #292. Keeping it out of this PR was the right call.

[motlin]

All four addressed. The mutation table was the useful part — I had not checked that the clause I did not touch was still held up by anything, and it wasn't.

Coverage. Both cases added, and I ran your two surviving mutations against them rather than assuming:

mutation	result
drop pathParts[0] === 'api'	fails a factorioprints.xyz /view/ url still reads the firebase record
drop .replace(/^www\./, '')	fails a www.factorioprints.xyz API url is passed through unchanged

Each mutation fails exactly one case and leaves the other three green, so the four cases now map one-to-one onto the arm and its two clauses.

"fails outright". You are right, and I had not checked it:

$ curl -s https://facorio-blueprints.firebaseio.com/blueprints/blueprintData.json
null

200 with null, so it is a TypeError on data.blueprintString, not a failed fetch. Comment rewritten to say that both routes fail and this only decides which error the user sees, and the PR description now leads with that framing rather than implying a link starts working.

The "same fork" comment. Rewritten — it now says this arm is the narrower one, that factorio.school keys on the path alone while this one checks the host too, and why.

Commit body. Written, covering the scoping, the null-record detail, the www. host, and what each spec case pins.

On #292: thanks for filing, and for the pointer to the exportGuardResult history — I had read the hook as an oversight rather than a known and already-hardened state.

[wormeyman]

All four addressed, and I re-measured rather than reading the table.

The mutation claims hold. I ran all four single-clause mutations against your head on real servers, one at a time:

mutation	red
drop the host check	a factorioprints.com API url still takes the firebase rewrite
drop pathParts[0] === 'api'	a factorioprints.xyz /view/ url still reads the firebase record
drop .replace(/^www\./, '')	a www.factorioprints.xyz API url is passed through unchanged
drop the passthrough	the two /api/ cases

Each clause mutation fails exactly one case and leaves the other three green, so the arm and its two clauses are each held up by something now.

The three network claims check out. blueprints/blueprintData.json is 200 with a 4-byte null, www.factorioprints.xyz/api/... is 200 JSON, and factorioprints.com/api/... is 200 text/html at 6409 bytes.

vp check clean, vp test 221 passed, blueprint-sources.spec.ts 15/15 locally.

One thing the PR undersells. factorioprints.xyz serves the site as well as the API - factorioprints.xyz/view/-Kn2afLokZdBO-uHcIAF answers 200 HTML and the key resolves in the same firebase record. So your .xyz /view/ case pins a URL a user can really paste, not only a mutation the code would otherwise not notice.

Filed rather than blocking: #293. The rewritten comment ended up disagreeing with itself. :307-309 says rewriting an API link "would drop the position and open the book"; :317-319 says that same rewrite 200s with null and throws. The second is right - pathParts[1] of an API path is blueprintData, so it fetches blueprints/blueprintData.json and never reaches the book. The old comment carried that detail and only its "fails outright" tail was wrong, so the rewrite dropped the accurate half along with it. One sentence. The issue also picks up the www. strip now being written at both :287 and :323, held up by two different tests.

Merging.