Set User-Agent header to window.navigator.userAgent #1237

shawwn · 2019-06-26T14:41:31Z

Browsers will typically disallow you from changing the User-Agent string. It's a security restriction. There is no way around it; you'll have to use a different header name, e.g. X-Custom-user-agent, and configure the server to read it.

This PR forces the User-Agent header of any window.fetch call to be window.navigator.userAgent.

(This behavior seems identical to what Chrome enforces.)

…adme Remove dead --experimental-worker references from README.md

Auto-enter tutorial.html

Unlock macos swap hacks

Added onbeforeload flag

…ead of the document

Add CanvasRenderingContext2D::setLineDash

HTMLLinkElement load emit timing

Emulate HTMLIFrameElement contentWindow/contentDocument better

…ead of function

Capture Buffer global in local closures in src

[Web Workers] Navigator

avaer · 2019-06-26T14:45:44Z

I think this is fine, but is there a plan to cover the rest of the cases here?

It seems that currently this PR adds disparity by fixing the one call point only.

shawwn · 2019-06-26T15:12:39Z

Done.

shawwn · 2019-06-26T15:28:15Z

I updated this PR with the latest changes from master.

It seems to cover all the cases you outlined. If you feel it's sufficient, then it's ready to merge.

avaer · 2019-06-26T16:01:37Z

src/Window.js

@@ -102,6 +102,7 @@ const {
 GlobalContext.id = id;
 GlobalContext.args = args;
 GlobalContext.version = version;
+GlobalContext.userAgent = `Mozilla/5.0 (OS) AppleWebKit/999.0 (KHTML, like Gecko) Chrome/999.0.0.0 Safari/999.0 Exokit/${GlobalContext.version}`;


We shouldn't add anything new to GlobalContext unless there is no other way, Ideally, GlobalContext is deleted entirely.

Can fetch pull from the global navigator instead?

Any suggestions? The only alternative I can think of is to move this line into src/fetch.js. But then it will still depend on GlobalContext.version.

It could be a method on window-fetch that sets the interception user agent.

Okay, I've updated the code. src/fetch.js now pulls from Navigator.userAgent, which is a static get userAgent() { .... } property.

Alternatively, the navigator should be a global in all cases, where the userAgent can be accessed.

That makes sense, but I don't think it needs to be a static. We could just access the navigator instance without adding any new data shapes.

The navigator instance is per-window, and there can be multiple windows. As far as I know, there isn't a concept of "the global window value", so src/fetch.js can't really call window.navigator.userAgent unless the user passes in window, which would complicate the interface.

I think exokit wanted a constant, un-configurable User-Agent string, if I remember our conversation correctly from a few months ago when I proposed a configurable User-Agent string. If so, then it seems to make sense for it to be either attached to GlobalContext or a static getter on Navigator.

As far as I know, there isn't a concept of "the global window value"

There is, in the specs. For our purposes that is simply global, as set up by node. I think this should work for all of the worker cases, as well as window.

avaer · 2019-06-26T16:02:19Z

src/fetch.js

+const GlobalContext = require('./GlobalContext');
+const fetch = require('window-fetch');
+
+Object.assign(module.exports, fetch);


This isn't the usual way we export in the rest of the codebase. It's usually

module.exports.fetch = fetch;

src/fetch.js needs to export everything that window-fetch exports, including Header, Request, Response, and Blob. This accomplishes that.

The idea is that src/fetch.js is a drop-in replacement for window-fetch.

If that's the case, maybe it's best this is put upstream into window-fetch as an interception API?

Mm, I would say that overriding the User-Agent header and forcing it to be a specific value is a browser-specific concern. It might be nice if window-fetch provided a way to do that, but the tradeoff is that the API becomes more complicated.

I think that's cleaner than requiring a hack to collect and re-export properties from the module.

window-fetch is designed to implement the needs of a "browser", and setting a User-Agent is one of the spec requirements of that.

Malicious modules are a valid concern, but a malicious module could do much worse than setting a User-Agent -- and it's not clear that that itself would be a problem regardless.

The API wouldn't necessarily need to be forceTheUserAgentStringToBe -- it could be options on a constructor on the import. This would be roughly equivalent to fetch.js, except owned by window-fetch instead of Exokit.

This is fine -- Exokit allows user code to spoof.

Chrome does not: https://developer.chrome.com/multidevice/user-agent

You can reconfigure the user-agent string via extensions (https://chrome.google.com/webstore/detail/user-agent-switcher-for-c/djflhoibgkdhkhhcedjiklpkjnoahfmg?hl=en-US) but it's switched globally; there's no such thing as overriding it on a per-request basis.

This is important to prevent userland JS from spoofing other devices. Google Analytics also relies on standard User-Agent values, so allowing users to override it can break GA.

Those concerns are outside the scope of Exokit. Exokit is not a browser.

Fair enough. In that case, feel free to close this PR. I'll merge it into sweetiekit-dom which is what we bundle with SweetieKit.

That will fix our GA issue #1230

I was hoping to maintain parity with exokit's DOM implementation, but it sounds like it might take some time before the necessary functionality / fixes can be integrated into window-fetch and published. The window-fetch extensions also introduce risks due to the dependency on mutable global state.

It might be a better idea for us to switch over to the standard https://github.com/bitinn/node-fetch module. There are significant upstream differences, like support for stream.pipe.

sounds like it might take some time before the necessary functionality / fixes can be integrated into window-fetch and published.

Which timing does this refer to? It looks like most of that work is done in this PR.

The window-fetch extensions also introduce risks due to the dependency on mutable global state.

What are the risks?

There are significant upstream differences, like support for stream.pipe.

Indeed, window-fetch should pull from upstream.

avaer and others added 30 commits June 6, 2019 09:53

Bugfix set top level stencil geometry type check

c6aeb99

Bugfix set stencil geometry arguments

2e491ab

Bugfix portal shader shader position dimensions 2 -> 3

0bcdf49

Update PORTAL_POSITIONS geometry

7596688

Remove dead --experimental-worker references from README.md

1c26a89

Merge pull request exokitxr#1177 from exokitxr/experimental-worker-re…

819b4ce

…adme Remove dead --experimental-worker references from README.md

Auto-enter tutorial.html

d910adb

Merge pull request exokitxr#1179 from exokitxr/tutorial-auto

a2e3cf8

Auto-enter tutorial.html

Unlock macos swap hacks

53b005c

Merge pull request exokitxr#1180 from exokitxr/macos-swap

05dc395

Unlock macos swap hacks

Added onbeforeload flag

1b1cc67

Added docs for CLI flag

ac86d32

Renamed onbeforeload, removed default

7d6410c

Merge pull request exokitxr#1182 from RangerMauve/onbeforeload

f01d085

Added onbeforeload flag

Bugfix portalOffset initialization

b4bcccc

Native bindings emit local gl framebuffer set events

770aee1

Latch WindowVm window framebuffer

ff7469a

Compose all window layers regardless of layered status

6a0cfb5

Remove Window prepare local frame tick raf phase

50a14d1

Add rain.html cube rain example

7ea65a4

Do not setTopLevel during XR entry/exit

9447682

Move Window.js setTopLevel to per-frame basis

7db2d64

Major layer composition refactoring

f7aceab

Comment out depth population injection in realitytabs.html

3515984

Major work in portal.html

448852d

Optimize depth recovery in portal shader

9849a6b

Major portal.html matrices access optimization

65b9255

Add missing portal x.png

f0d772f

Merge branch 'master' into reality-bounds

f01b63b

Small Window.js dead code cleanup

c78fc82

avaer added 18 commits June 24, 2019 15:34

Merge branch 'electron-flip2'

694abd2

Make delayed documenmt event dispatch call on the actual element inst…

eaffa04

…ead of the document

Small HTMLScriptElement load cleanup

783c629

Major HTMLLinkElement load timing debugging

3f6d869

Add CanvasRenderingContext2D setLineDash method

5c06877

Merge pull request exokitxr#1228 from exokitxr/set-line-dash

235c5a2

Add CanvasRenderingContext2D::setLineDash

Merge pull request exokitxr#1227 from exokitxr/link-load

6b29bae

HTMLLinkElement load emit timing

Clean up HTMLIframeElement contentWindow/contentDocument initialization

e34b5f5

Bugfix HTMLIFrameElement contentWindow timing on dom element attach

567b4f9

Merge pull request exokitxr#1229 from exokitxr/content-window

56b0dcf

Emulate HTMLIFrameElement contentWindow/contentDocument better

Bugfix windowsystem plane shader size 2 instead of 1

1668f11

Bugfix CustomElementRegistry observedAttributes to expect getter inst…

34c8ace

…ead of function

Break out Navigator implementation to Navigator.js

647900a

Capture Buffer global in local closures in src

ad8da73

Merge pull request exokitxr#1236 from exokitxr/local-buffer

130dfae

Capture Buffer global in local closures in src

Window.js dead requires cleanup

d82bab3

Merge pull request exokitxr#1235 from exokitxr/worker-navigator

751ccd7

[Web Workers] Navigator

Add missing Navigator.js symbols require

988072a

shawwn mentioned this pull request Jun 26, 2019

For each call to fetch, set the User-Agent header #1234

Closed

Set User-Agent header to GlobalContext.userAgent

58ff970

shawwn force-pushed the 2019-06-26/fetch-user-agent branch from 5b27f89 to 58ff970 Compare June 26, 2019 15:25

avaer reviewed Jun 26, 2019

View reviewed changes

Define and use Navigator.userAgent

ba37a40

avaer closed this Aug 5, 2019

avaer force-pushed the master branch from 1f95e1f to 43edd39 Compare August 5, 2019 21:14

avaer mentioned this pull request Aug 5, 2019

Set User-Agent header to window.navigator.userAgent #1373

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set User-Agent header to window.navigator.userAgent #1237

Set User-Agent header to window.navigator.userAgent #1237

shawwn commented Jun 26, 2019

avaer commented Jun 26, 2019

shawwn commented Jun 26, 2019

shawwn commented Jun 26, 2019

avaer Jun 26, 2019

avaer Jun 26, 2019

shawwn Jun 26, 2019

avaer Jun 26, 2019

shawwn Jun 26, 2019

avaer Jun 26, 2019

avaer Jun 26, 2019

shawwn Jun 26, 2019

avaer Jun 26, 2019

avaer Jun 26, 2019

shawwn Jun 26, 2019

avaer Jun 26, 2019

shawwn Jun 26, 2019

avaer Jun 26, 2019

avaer Jun 26, 2019

shawwn Jun 26, 2019

avaer Jun 26, 2019

shawwn Jun 26, 2019 •

edited

avaer Jun 26, 2019 •

edited

Set User-Agent header to window.navigator.userAgent #1237

Set User-Agent header to window.navigator.userAgent #1237

Conversation

shawwn commented Jun 26, 2019

avaer commented Jun 26, 2019

shawwn commented Jun 26, 2019

shawwn commented Jun 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shawwn Jun 26, 2019 • edited

Choose a reason for hiding this comment

avaer Jun 26, 2019 • edited

Choose a reason for hiding this comment

shawwn Jun 26, 2019 •

edited

avaer Jun 26, 2019 •

edited