-
Notifications
You must be signed in to change notification settings - Fork 9
Unify Vary and add Cache-Control on RDF variants (#315) #316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,6 @@ import { | |
| canAcceptInput, | ||
| toJsonLd, | ||
| fromJsonLd, | ||
| getVaryHeader, | ||
| RDF_TYPES | ||
| } from '../rdf/conneg.js'; | ||
| import { emitChange } from '../notifications/events.js'; | ||
|
|
@@ -22,6 +21,12 @@ import { generateDatabrowserHtml, generateModuleDatabrowserHtml, shouldServeMash | |
| */ | ||
| const LIVE_RELOAD_SCRIPT = `<script>(function(){var ws=new WebSocket((location.protocol==='https:'?'wss:':'ws:')+'//' +location.host+'/.notifications');ws.onopen=function(){ws.send('sub '+location.href)};ws.onmessage=function(e){if(e.data.startsWith('pub '))location.reload()};ws.onclose=function(){setTimeout(function(){location.reload()},1000)}})();</script>`; | ||
|
|
||
| // Cache-Control for RDF data responses: let clients keep the body but force | ||
| // revalidation via ETag on every use. This prevents stale bodies from leaking | ||
| // across auth-state changes (WAC) and closes the mashlib render-race window | ||
| // where a cached data variant was served on top-level navigation (#315). | ||
| const RDF_CACHE_CONTROL = 'private, no-cache, must-revalidate'; | ||
|
|
||
| /** | ||
| * Inject live reload script into HTML content | ||
| */ | ||
|
|
@@ -181,6 +186,7 @@ export async function handleGet(request, reply) { | |
| resourceUrl, | ||
| connegEnabled | ||
| }); | ||
| headers['Cache-Control'] = RDF_CACHE_CONTROL; | ||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
| return reply.send(turtleContent); | ||
|
|
@@ -194,6 +200,7 @@ export async function handleGet(request, reply) { | |
| resourceUrl, | ||
| connegEnabled | ||
| }); | ||
| headers['Cache-Control'] = RDF_CACHE_CONTROL; | ||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
| return reply.send(JSON.stringify(jsonLd, null, 2)); | ||
|
|
@@ -239,9 +246,9 @@ export async function handleGet(request, reply) { | |
| contentType: 'text/html', | ||
| origin, | ||
| resourceUrl, | ||
| connegEnabled | ||
| connegEnabled, | ||
| mashlibEnabled: request.mashlibEnabled | ||
| }); | ||
| headers['Vary'] = 'Accept'; | ||
| headers['X-Frame-Options'] = 'DENY'; | ||
| headers['Content-Security-Policy'] = "frame-ancestors 'none'"; | ||
| headers['Cache-Control'] = 'no-store'; | ||
|
|
@@ -274,9 +281,10 @@ export async function handleGet(request, reply) { | |
| contentType: 'text/turtle', | ||
| origin, | ||
| resourceUrl, | ||
| connegEnabled | ||
| connegEnabled, | ||
| mashlibEnabled: request.mashlibEnabled | ||
| }); | ||
| headers['Vary'] = 'Accept'; | ||
| headers['Cache-Control'] = RDF_CACHE_CONTROL; | ||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
| return reply.send(turtleContent); | ||
|
|
@@ -292,8 +300,10 @@ export async function handleGet(request, reply) { | |
| contentType: 'application/ld+json', | ||
| origin, | ||
| resourceUrl, | ||
| connegEnabled | ||
| connegEnabled, | ||
| mashlibEnabled: request.mashlibEnabled | ||
| }); | ||
| headers['Cache-Control'] = RDF_CACHE_CONTROL; | ||
|
Comment on lines
302
to
+306
|
||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
| return reply.send(serializeJsonLd(jsonLd)); | ||
|
|
@@ -315,9 +325,9 @@ export async function handleGet(request, reply) { | |
| contentType: 'text/html', | ||
| origin, | ||
| resourceUrl, | ||
| connegEnabled | ||
| connegEnabled, | ||
| mashlibEnabled: request.mashlibEnabled | ||
| }); | ||
| headers['Vary'] = 'Accept'; | ||
| headers['X-Frame-Options'] = 'DENY'; | ||
| headers['Content-Security-Policy'] = "frame-ancestors 'none'"; | ||
| // Don't cache the HTML wrapper - always negotiate fresh | ||
|
|
@@ -397,9 +407,10 @@ export async function handleGet(request, reply) { | |
| contentType: 'text/turtle', | ||
| origin, | ||
| resourceUrl, | ||
| connegEnabled | ||
| connegEnabled, | ||
| mashlibEnabled: request.mashlibEnabled | ||
| }); | ||
| headers['Vary'] = getVaryHeader(connegEnabled, request.mashlibEnabled); | ||
| headers['Cache-Control'] = RDF_CACHE_CONTROL; | ||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
| return reply.send(turtleContent); | ||
|
|
@@ -427,9 +438,10 @@ export async function handleGet(request, reply) { | |
| contentType: outputType, | ||
| origin, | ||
| resourceUrl, | ||
| connegEnabled | ||
| connegEnabled, | ||
| mashlibEnabled: request.mashlibEnabled | ||
| }); | ||
| headers['Vary'] = getVaryHeader(connegEnabled, request.mashlibEnabled); | ||
| headers['Cache-Control'] = RDF_CACHE_CONTROL; | ||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
| return reply.send(outputContent); | ||
|
|
@@ -455,9 +467,12 @@ export async function handleGet(request, reply) { | |
| contentType: actualContentType, | ||
| origin, | ||
| resourceUrl, | ||
| connegEnabled | ||
| connegEnabled, | ||
| mashlibEnabled: request.mashlibEnabled | ||
| }); | ||
| headers['Vary'] = getVaryHeader(connegEnabled, request.mashlibEnabled); | ||
| if (isRdfContentType(actualContentType)) { | ||
| headers['Cache-Control'] = RDF_CACHE_CONTROL; | ||
| } | ||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
|
|
||
|
|
@@ -671,9 +686,8 @@ export async function handlePut(request, reply) { | |
| } | ||
|
|
||
| const origin = request.headers.origin; | ||
| const headers = getAllHeaders({ isContainer: false, origin, resourceUrl, connegEnabled }); | ||
| const headers = getAllHeaders({ isContainer: false, origin, resourceUrl, connegEnabled, mashlibEnabled: request.mashlibEnabled }); | ||
| headers['Location'] = resourceUrl; | ||
| headers['Vary'] = getVaryHeader(connegEnabled, request.mashlibEnabled); | ||
|
|
||
| Object.entries(headers).forEach(([k, v]) => reply.header(k, v)); | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| /** | ||
| * Regression tests for #315 — inconsistent Vary / Cache-Control across | ||
| * conneg variants caused stale-render races on browser reload. | ||
| * | ||
| * What we guarantee now: | ||
| * - Every variant of the same URL returns an *identical* Vary header. | ||
| * - RDF data variants carry Cache-Control that forces revalidation via | ||
| * ETag, so a cached body cannot silently serve across auth changes or | ||
| * be picked up on a top-level navigation by mistake. | ||
| * - The mashlib HTML wrapper keeps `no-store` (it's a bootstrap template). | ||
| */ | ||
|
|
||
| import { describe, it, before, after } from 'node:test'; | ||
| import assert from 'node:assert'; | ||
| import { | ||
| startTestServer, | ||
| stopTestServer, | ||
| request, | ||
| createTestPod | ||
| } from './helpers.js'; | ||
|
|
||
| describe('Vary / Cache-Control consistency (#315)', () => { | ||
| before(async () => { | ||
| await startTestServer({ conneg: true, mashlibCdn: true }); | ||
| await createTestPod('varytest'); | ||
| // Create a JSON-LD resource to exercise all variants. | ||
| await request('/varytest/public/card.jsonld', { | ||
| method: 'PUT', | ||
| headers: { 'Content-Type': 'application/ld+json' }, | ||
| body: JSON.stringify({ | ||
| '@context': { foaf: 'http://xmlns.com/foaf/0.1/' }, | ||
| '@id': '#me', | ||
| 'foaf:name': 'Vary Test' | ||
| }), | ||
| auth: 'varytest' | ||
| }); | ||
| }); | ||
|
|
||
| after(async () => { await stopTestServer(); }); | ||
|
|
||
| it('Vary header is identical across all conneg variants of the same URL', async () => { | ||
| const accepts = [ | ||
| 'text/html,*/*;q=0.8', // mashlib HTML wrapper | ||
| 'text/turtle', // Turtle conversion | ||
| 'application/ld+json' // native JSON-LD | ||
| ]; | ||
| const varyValues = []; | ||
| for (const accept of accepts) { | ||
| const res = await request('/varytest/public/card.jsonld', { headers: { Accept: accept } }); | ||
| varyValues.push({ accept, vary: res.headers.get('vary') }); | ||
| } | ||
| // All three variants must carry the same Vary — inconsistent Vary is | ||
| // what confused browser caches into serving the wrong variant. | ||
| const uniqueVaryValues = new Set(varyValues.map((v) => v.vary)); | ||
| assert.strictEqual(uniqueVaryValues.size, 1, | ||
| `expected identical Vary across variants, got: ${JSON.stringify(varyValues)}`); | ||
| const vary = [...uniqueVaryValues][0]; | ||
| assert.ok(vary, `expected Vary header across variants, got: ${JSON.stringify(varyValues)}`); | ||
| assert.match(vary, /Accept/, 'Vary must include Accept (conneg active)'); | ||
| assert.match(vary, /Authorization/, 'Vary must include Authorization (WAC)'); | ||
| assert.match(vary, /Origin/, 'Vary must include Origin (CORS)'); | ||
| }); | ||
|
|
||
| it('mashlib HTML wrapper uses Cache-Control: no-store', async () => { | ||
| const res = await request('/varytest/public/card.jsonld', { | ||
| headers: { Accept: 'text/html,*/*;q=0.8' } | ||
| }); | ||
| assert.match(res.headers.get('content-type') || '', /text\/html/); | ||
| assert.strictEqual(res.headers.get('cache-control'), 'no-store'); | ||
| }); | ||
|
|
||
| it('RDF data variants force revalidation (no stale bodies across auth changes)', async () => { | ||
| // Full expected policy — pinning every directive so a regression that | ||
| // drops `private` or `must-revalidate` (both needed to prevent auth-state | ||
| // leakage and force freshness) fails the test. | ||
| const expected = 'private, no-cache, must-revalidate'; | ||
| for (const accept of ['text/turtle', 'application/ld+json']) { | ||
| const res = await request('/varytest/public/card.jsonld', { headers: { Accept: accept } }); | ||
| assert.strictEqual(res.headers.get('cache-control'), expected, | ||
| `Cache-Control mismatch on Accept: ${accept}`); | ||
| // ETag is preserved so revalidation is cheap (304). | ||
| assert.ok(res.headers.get('etag'), `expected ETag on ${accept} variant`); | ||
| } | ||
| }); | ||
|
|
||
| it('container index.html data-island variants also carry revalidating Cache-Control', async () => { | ||
| // Publish an index.html with a JSON-LD data island; conneg should extract | ||
| // and serve it as Turtle/JSON-LD. Those variants were missing | ||
| // Cache-Control pre-#315. | ||
| const html = [ | ||
| '<!doctype html><html><head>', | ||
| '<script type="application/ld+json">', | ||
| JSON.stringify({ | ||
| '@context': { foaf: 'http://xmlns.com/foaf/0.1/' }, | ||
| '@id': '#this', | ||
| 'foaf:name': 'Island' | ||
| }), | ||
| '</script></head><body>hi</body></html>' | ||
| ].join(''); | ||
| await request('/varytest/public/index.html', { | ||
| method: 'PUT', | ||
| headers: { 'Content-Type': 'text/html' }, | ||
| body: html, | ||
| auth: 'varytest' | ||
| }); | ||
|
|
||
| const expected = 'private, no-cache, must-revalidate'; | ||
| for (const accept of ['text/turtle', 'application/ld+json']) { | ||
| const res = await request('/varytest/public/', { headers: { Accept: accept } }); | ||
| assert.strictEqual(res.headers.get('cache-control'), expected, | ||
| `Cache-Control mismatch on island variant (Accept: ${accept})`); | ||
| } | ||
| }); | ||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RDF_CACHE_CONTROLis applied to most RDF data responses, but the index.html container code path that extracts a JSON-LD data island and returns Turtle/JSON-LD (handleGet() whenstats.isDirectoryandindexExists) still returns those RDF variants without setting this Cache-Control. That leaves a caching/revalidation gap for those variants; consider settingCache-Control: private, no-cache, must-revalidate(and keeping the ETag) for both the Turtle and JSON-LD responses from that branch as well.