From 28e91ba70cd70fa9adf3f2e3e3b87631b5667ecf Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 13 Apr 2020 16:14:40 -0500 Subject: [PATCH] Fix domain issues when setting the cookie Fixes #1507. --- package.json | 2 +- src/node/http.ts | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index 842aed16..ffd7d67f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "code-server", "license": "MIT", - "version": "3.0.2", + "version": "3.1.1", "scripts": { "clean": "ci/clean.sh", "vscode": "ci/vscode.sh", diff --git a/src/node/http.ts b/src/node/http.ts index 654a9d79..dd5ff581 100644 --- a/src/node/http.ts +++ b/src/node/http.ts @@ -590,9 +590,6 @@ export class HttpServer { this.heart.beat() const route = this.parseUrl(request) const write = (payload: HttpResponse): void => { - const host = request.headers.host || "" - const idx = host.indexOf(":") - const domain = idx !== -1 ? host.substring(0, idx) : host response.writeHead(payload.redirect ? HttpCode.Redirect : payload.code || HttpCode.Ok, { "Content-Type": payload.mime || getMediaMime(payload.filePath), ...(payload.redirect ? { Location: this.constructRedirect(request, route, payload as RedirectResponse) } : {}), @@ -603,7 +600,7 @@ export class HttpServer { "Set-Cookie": [ `${payload.cookie.key}=${payload.cookie.value}`, `Path=${normalize(payload.cookie.path || "/", true)}`, - domain ? `Domain=${this.getCookieDomain(domain)}` : undefined, + this.getCookieDomain(request.headers.host || ""), // "HttpOnly", "SameSite=lax", ] @@ -822,20 +819,39 @@ export class HttpServer { } /** - * Get the domain that should be used for setting a cookie. This will allow - * the user to authenticate only once. This will return the highest level + * Get the value that should be used for setting a cookie domain. This will + * allow the user to authenticate only once. This will use the highest level * domain (e.g. `coder.com` over `test.coder.com` if both are specified). */ - private getCookieDomain(host: string): string { - let current: string | undefined + private getCookieDomain(host: string): string | undefined { + const idx = host.lastIndexOf(":") + host = idx !== -1 ? host.substring(0, idx) : host + if ( + // Might be blank/missing, so there's nothing more to do. + !host || + // IP addresses can't have subdomains so there's no value in setting the + // domain for them. Assume anything with a : is ipv6 (valid domain name + // characters are alphanumeric or dashes). + host.includes(":") || + // Assume anything entirely numbers and dots is ipv4 (currently tlds + // cannot be entirely numbers). + !/[^0-9.]/.test(host) || + // localhost subdomains don't seem to work at all (browser bug?). + host.endsWith(".localhost") || + // It might be localhost (or an IP, see above) if it's a proxy and it + // isn't setting the host header to match the access domain. + host === "localhost" + ) { + return undefined + } + this.proxyDomains.forEach((domain) => { - if (host.endsWith(domain) && (!current || domain.length < current.length)) { - current = domain + if (host.endsWith(domain) && domain.length < host.length) { + host = domain } }) - // Setting the domain to localhost doesn't seem to work for subdomains (for - // example dev.localhost). - return current && current !== "localhost" ? current : host + + return host ? `Domain=${host}` : undefined } /**