Skip to content

Commit 7ecf3bd

Browse files
code-asherZauberNerd
authored andcommitted
Set remote authority on frontend (microsoft#25)
Trying to determine the remote authority on the backend is brittle because it does not work behind reverse proxies unless they send the right headers containing information about the proxied source. We could require users add the relevant configuration or provide the remote authority via a flag but neither are user-friendly options. We can make it work out of the box by changing the frontend to make requests to its current address (which is what we try to set the remote authority to anyway). This actually already happens for the most part except in some UI and logs although recent issues suggest there might be other problems which should be entirely resolved by setting this on the frontend. In other words, the remote authority we set on the backend should never be used so we set it to something invalid to ensure we notice (the alternative is to rip it out but that is probably a bigger patch thus generating more conflicts). One scenario where we might want to set the remote authority from the backend is if the frontend is served from a different location than the backend but that is not supported behavior at the moment. Even if we did support this we still cannot determine the authority from the backend (even for non-proxy scenarios in this case) and would need to add a flag for it so this change would still be necessary. coder/code-server#4604 coder/code-server#4607 coder/code-server#4608
1 parent 206b283 commit 7ecf3bd

File tree

4 files changed

+16
-36
lines changed

4 files changed

+16
-36
lines changed

package.json

-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@
6666
"@vscode/vscode-languagedetection": "1.0.21",
6767
"applicationinsights": "1.4.2",
6868
"cookie": "^0.4.1",
69-
"forwarded-parse": "^2.1.2",
7069
"graceful-fs": "4.2.8",
7170
"http-proxy-agent": "^2.1.0",
7271
"https-proxy-agent": "^2.2.3",

src/vs/code/browser/workbench/workbench.ts

+6
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,12 @@ function doCreateUri(path: string, queryValues: Map<string, string>): URI {
474474
enabled: config.settingsSyncOptions.enabled,
475475
} : undefined,
476476
workspaceProvider: WorkspaceProvider.create(config),
477+
/**
478+
* Ensure the remote authority points to the current address since we cannot
479+
* determine this reliably on the backend.
480+
* @author coder
481+
*/
482+
remoteAuthority: location.host,
477483
/**
478484
* Override relative URLs in the product configuration against the window
479485
* location as necessary. Only paths that must be absolute need to be

src/vs/server/webClientServer.ts

+10-30
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import * as url from 'url';
99
import * as util from 'util';
1010
import * as cookie from 'cookie';
1111
import * as crypto from 'crypto';
12-
import parseForwardHeader = require('forwarded-parse');
1312
import { isEqualOrParent, sanitizeFilePath } from 'vs/base/common/extpath';
1413
import { getMediaMime } from 'vs/base/common/mime';
1514
import { isLinux } from 'vs/base/common/platform';
@@ -27,7 +26,6 @@ import type { IWorkbenchConstructionOptions } from 'vs/workbench/workbench.web.a
2726
import { editorBackground, editorForeground } from 'vs/platform/theme/common/colorRegistry';
2827
import { ClientTheme, getOriginalUrl, HTTPNotFoundError, relativePath, relativeRoot, WebManifest } from 'vs/server/common/net';
2928
import { IServerThemeService } from 'vs/server/serverThemeService';
30-
import { isFalsyOrWhitespace } from 'vs/base/common/strings';
3129

3230
const textMimeType = {
3331
'.html': 'text/html',
@@ -166,29 +164,6 @@ export class WebClientServer {
166164

167165
private _iconSizes = [192, 512];
168166

169-
private getRemoteAuthority(req: http.IncomingMessage): URL {
170-
if (req.headers.forwarded) {
171-
const [parsedHeader] = parseForwardHeader(req.headers.forwarded);
172-
return new URL(`${parsedHeader.proto}://${parsedHeader.host}`);
173-
}
174-
175-
/* Return first non-empty header. */
176-
const parseHeaders = (headerNames: string[]): string | undefined => {
177-
for (const headerName of headerNames) {
178-
const header = req.headers[headerName]?.toString();
179-
if (!isFalsyOrWhitespace(header)) {
180-
return header;
181-
}
182-
}
183-
return undefined;
184-
};
185-
186-
const proto = parseHeaders(['X-Forwarded-Proto']) || 'http';
187-
const host = parseHeaders(['X-Forwarded-Host', 'host']) || 'localhost';
188-
189-
return new URL(`${proto}://${host}`);
190-
}
191-
192167
/**
193168
* PWA manifest file. This informs the browser that the app may be installed.
194169
*/
@@ -291,9 +266,15 @@ export class WebClientServer {
291266
// return this.serveError(req, res, 403, `Forbidden.`, parsedUrl);
292267
// }
293268

294-
const remoteAuthority = this.getRemoteAuthority(req);
295-
296-
const transformer = createRemoteURITransformer(remoteAuthority.host);
269+
/**
270+
* It is not possible to reliably detect the remote authority on the server
271+
* in all cases. Set this to something invalid to make sure we catch code
272+
* that is using this when it should not.
273+
*
274+
* @author coder
275+
*/
276+
const remoteAuthority = 'remote';
277+
const transformer = createRemoteURITransformer(remoteAuthority);
297278
const { workspacePath, isFolder } = await this._getWorkspaceFromCLI();
298279

299280
function escapeAttribute(value: string): string {
@@ -342,8 +323,7 @@ export class WebClientServer {
342323
},
343324
folderUri: (workspacePath && isFolder) ? transformer.transformOutgoing(URI.file(workspacePath)) : undefined,
344325
workspaceUri: (workspacePath && !isFolder) ? transformer.transformOutgoing(URI.file(workspacePath)) : undefined,
345-
// Add port to prevent client-side mismatch for reverse proxies.
346-
remoteAuthority: `${remoteAuthority.hostname}:${remoteAuthority.port || (remoteAuthority.protocol === 'https:' ? '443' : '80')}`,
326+
remoteAuthority,
347327
_wrapWebWorkerExtHostInIframe,
348328
developmentOptions: {
349329
enableSmokeTestDriver: this._environmentService.driverHandle === 'web' ? true : undefined,

yarn.lock

-5
Original file line numberDiff line numberDiff line change
@@ -4344,11 +4344,6 @@ form-data@~2.3.2:
43444344
combined-stream "^1.0.6"
43454345
mime-types "^2.1.12"
43464346

4347-
forwarded-parse@^2.1.2:
4348-
version "2.1.2"
4349-
resolved "https://registry.yarnpkg.com/forwarded-parse/-/forwarded-parse-2.1.2.tgz#08511eddaaa2ddfd56ba11138eee7df117a09325"
4350-
integrity sha512-alTFZZQDKMporBH77856pXgzhEzaUVmLCDk+egLgIgHst3Tpndzz8MnKe+GzRJRfvVdn69HhpW7cmXzvtLvJAw==
4351-
43524347
fragment-cache@^0.2.1:
43534348
version "0.2.1"
43544349
resolved "https://registry.yarnpkg.com/fragment-cache/-/fragment-cache-0.2.1.tgz#4290fad27f13e89be7f33799c6bc5a0abfff0d19"

0 commit comments

Comments
 (0)