-
Notifications
You must be signed in to change notification settings - Fork 23
feat: add option to enable cross-origin isolation #234
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
e01f5d9
87e319f
3cd4426
0aee155
0b7e643
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import {Middleware} from 'koa'; | ||
|
||
// Enable cross-origin isolation for more precise timers: | ||
// https://developer.chrome.com/blog/cross-origin-isolated-hr-timers/ | ||
export function crossOriginIsolation(): Middleware { | ||
// Based on https://github.com/fishel-feng/koa-isolated | ||
return async function isolated(ctx, next) { | ||
ctx.set('Cross-Origin-Opener-Policy', 'same-origin'); | ||
ctx.set('Cross-Origin-Embedder-Policy', 'require-corp'); | ||
await next(); | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import {nodeResolve} from 'koa-node-resolve'; | |
|
||
import {BenchmarkResponse, Deferred} from './types.js'; | ||
import {NpmInstall} from './versions.js'; | ||
import {crossOriginIsolation} from './cross-origin-isolation'; | ||
|
||
import * as url from 'url'; | ||
const __dirname = url.fileURLToPath(new URL('.', import.meta.url)); | ||
|
@@ -91,6 +92,7 @@ export class Server { | |
this.server = server; | ||
const app = new Koa(); | ||
|
||
app.use(crossOriginIsolation()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a chance this change could be breaking in the case where someone is explicitly measuring something where external resources are requested? In this case, may it be worth putting this change behind a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! I totally didn't think of that. 😅 I think making this disabled by default sort of defeats the purpose of improving the timing accuracy, but I also admit I don't have any evidence that CORP/COEP actually does this (maybe the browser treats So sure thing: I added a CLI option, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the benefits of having it (even if behind a flag):
I'm a fan of this change! |
||
app.use(bodyParser()); | ||
app.use(mount('/submitResults', this.submitResults.bind(this))); | ||
app.use(this.instrumentRequests.bind(this)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for adding tests! |
Uh oh!
There was an error while loading. Please reload this page.