From 52cef46d51d3af3eb0401cb292336a438bfb46cf Mon Sep 17 00:00:00 2001 From: Jan Scheffler Date: Thu, 19 Dec 2019 12:07:53 +0100 Subject: [PATCH] Refactor Cookie implementation This cl prepares the refactoring of how cookies are handled inside the devtools frontend by moving the Cookie class outside of CookieParser and removing the attributes method because it is only used in tests and duplicates already existing getter methods in an inconsistent way. Cl disabling the test: crrev.com/c/1960350 Cl reenabling the test: crrev.com/c/1960279 Bug: chromium:1030258 Change-Id: Idc20d67bd55cab21d2b8d037b33d445b44d79088 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1959984 Commit-Queue: Jan Scheffler Reviewed-by: Sigurd Schneider Reviewed-by: Mathias Bynens --- BUILD.gn | 2 + front_end/resources/CookieItemsView.js | 2 +- front_end/sdk/Cookie.js | 263 ++++++++++++++++++++++++ front_end/sdk/CookieModel.js | 4 +- front_end/sdk/CookieParser.js | 268 +------------------------ front_end/sdk/module.json | 1 + front_end/sdk/sdk.js | 2 + karma.conf.js | 3 +- test/unittests/front_end/sdk/Cookie.ts | 166 +++++++++++++++ 9 files changed, 443 insertions(+), 268 deletions(-) create mode 100644 front_end/sdk/Cookie.js create mode 100644 test/unittests/front_end/sdk/Cookie.ts diff --git a/BUILD.gn b/BUILD.gn index 5e94065b48..2905df97f0 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -954,6 +954,7 @@ all_devtools_modules = [ "front_end/sdk/CSSMatchedStyles.js", "front_end/sdk/CPUProfilerModel.js", "front_end/sdk/CPUProfileDataModel.js", + "front_end/sdk/Cookie.js", "front_end/sdk/CookieParser.js", "front_end/sdk/CookieModel.js", "front_end/sdk/CompilerSourceMappingContentProvider.js", @@ -1669,6 +1670,7 @@ copied_devtools_modules = [ "$resources_out_dir/sdk/CSSMatchedStyles.js", "$resources_out_dir/sdk/CPUProfilerModel.js", "$resources_out_dir/sdk/CPUProfileDataModel.js", + "$resources_out_dir/sdk/Cookie.js", "$resources_out_dir/sdk/CookieParser.js", "$resources_out_dir/sdk/CookieModel.js", "$resources_out_dir/sdk/CompilerSourceMappingContentProvider.js", diff --git a/front_end/resources/CookieItemsView.js b/front_end/resources/CookieItemsView.js index 5f5e093841..8b97d5b80c 100644 --- a/front_end/resources/CookieItemsView.js +++ b/front_end/resources/CookieItemsView.js @@ -160,7 +160,7 @@ Resources.CookieItemsView = class extends Resources.StorageItemsView { if (!this._model) { return Promise.resolve(false); } - if (oldCookie && (newCookie.name() !== oldCookie.name() || newCookie.url() !== oldCookie.url())) { + if (oldCookie && newCookie.key() !== oldCookie.key()) { this._model.deleteCookie(oldCookie); } return this._model.saveCookie(newCookie); diff --git a/front_end/sdk/Cookie.js b/front_end/sdk/Cookie.js new file mode 100644 index 0000000000..d9b4847311 --- /dev/null +++ b/front_end/sdk/Cookie.js @@ -0,0 +1,263 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/** + * @unrestricted + */ +export class Cookie { + /** + * @param {string} name + * @param {string} value + * @param {?Type} type + * @param {!Protocol.Network.CookiePriority=} priority + */ + constructor(name, value, type, priority) { + this._name = name; + this._value = value; + this._type = type; + this._attributes = {}; + this._size = 0; + this._priority = /** @type {!Protocol.Network.CookiePriority} */ (priority || 'Medium'); + /** @type {string|null} */ + this._cookieLine = null; + } + + /** + * @param {!Protocol.Network.Cookie} protocolCookie + * @return {!SDK.Cookie} + */ + static fromProtocolCookie(protocolCookie) { + const cookie = new Cookie(protocolCookie.name, protocolCookie.value, null, protocolCookie.priority); + cookie.addAttribute('domain', protocolCookie['domain']); + cookie.addAttribute('path', protocolCookie['path']); + cookie.addAttribute('port', protocolCookie['port']); + if (protocolCookie['expires']) { + cookie.addAttribute('expires', protocolCookie['expires'] * 1000); + } + if (protocolCookie['httpOnly']) { + cookie.addAttribute('httpOnly'); + } + if (protocolCookie['secure']) { + cookie.addAttribute('secure'); + } + if (protocolCookie['sameSite']) { + cookie.addAttribute('sameSite', protocolCookie['sameSite']); + } + cookie.setSize(protocolCookie['size']); + return cookie; + } + + /** + * @returns {string} + */ + key() { + return (this.domain() || '-') + ' ' + this.name() + ' ' + (this.path() || '-'); + } + + /** + * @return {string} + */ + name() { + return this._name; + } + + /** + * @return {string} + */ + value() { + return this._value; + } + + /** + * @return {?Type} + */ + type() { + return this._type; + } + + /** + * @return {boolean} + */ + httpOnly() { + return 'httponly' in this._attributes; + } + + /** + * @return {boolean} + */ + secure() { + return 'secure' in this._attributes; + } + + /** + * @return {!Protocol.Network.CookieSameSite} + */ + sameSite() { + // TODO(allada) This should not rely on _attributes and instead store them individually. + return /** @type {!Protocol.Network.CookieSameSite} */ (this._attributes['samesite']); + } + + /** + * @return {!Protocol.Network.CookiePriority} + */ + priority() { + return this._priority; + } + + /** + * @return {boolean} + */ + session() { + // RFC 2965 suggests using Discard attribute to mark session cookies, but this does not seem to be widely used. + // Check for absence of explicitly max-age or expiry date instead. + return !('expires' in this._attributes || 'max-age' in this._attributes); + } + + /** + * @return {string} + */ + path() { + return this._attributes['path']; + } + + /** + * @return {string} + */ + port() { + return this._attributes['port']; + } + + /** + * @return {string} + */ + domain() { + return this._attributes['domain']; + } + + /** + * @return {number} + */ + expires() { + return this._attributes['expires']; + } + + /** + * @return {string} + */ + maxAge() { + return this._attributes['max-age']; + } + + /** + * @return {number} + */ + size() { + return this._size; + } + + /** + * @return {string|null} + */ + url() { + if (!this.domain() || !this.path()) { + return null; + } + return (this.secure() ? 'https://' : 'http://') + this.domain() + this.path(); + } + + /** + * @param {number} size + */ + setSize(size) { + this._size = size; + } + + /** + * @return {!Date|null} + */ + expiresDate(requestDate) { + // RFC 6265 indicates that the max-age attribute takes precedence over the expires attribute + if (this.maxAge()) { + return new Date(requestDate.getTime() + 1000 * this.maxAge()); + } + + if (this.expires()) { + return new Date(this.expires()); + } + + return null; + } + + /** + * @param {string} key + * @param {string|number=} value + */ + addAttribute(key, value) { + const normalizedKey = key.toLowerCase(); + switch (normalizedKey) { + case 'priority': + this._priority = /** @type {!Protocol.Network.CookiePriority} */ (value); + break; + default: + this._attributes[normalizedKey] = value; + } + } + + /** + * @param {string} cookieLine + */ + setCookieLine(cookieLine) { + this._cookieLine = cookieLine; + } + + /** + * @return {string|null} + */ + getCookieLine() { + return this._cookieLine; + } +} + +/** + * @enum {number} + */ +export const Type = { + Request: 0, + Response: 1 +}; + +/** + * @enum {string} + */ +export const Attributes = { + Name: 'name', + Value: 'value', + Size: 'size', + Domain: 'domain', + Path: 'path', + Expires: 'expires', + HttpOnly: 'httpOnly', + Secure: 'secure', + SameSite: 'sameSite', + Priority: 'priority', +}; + +/* Legacy exported object */ +self.SDK = self.SDK || {}; + +/* Legacy exported object */ +SDK = SDK || {}; + +/** @constructor */ +SDK.Cookie = Cookie; + +/** + * @enum {number} + */ +SDK.Cookie.Type = Type; + +/** + * @enum {string} + */ +SDK.Cookie.Attributes = Attributes; diff --git a/front_end/sdk/CookieModel.js b/front_end/sdk/CookieModel.js index a8e6445aaf..09701a4304 100644 --- a/front_end/sdk/CookieModel.js +++ b/front_end/sdk/CookieModel.js @@ -107,8 +107,8 @@ export default class CookieModel extends SDK.SDKModel { return this.target() .networkAgent() .setCookie( - cookie.name(), cookie.value(), cookie.url(), domain, cookie.path(), cookie.secure(), cookie.httpOnly(), - cookie.sameSite(), expires, cookie.priority()) + cookie.name(), cookie.value(), cookie.url() || undefined, domain, cookie.path(), cookie.secure(), + cookie.httpOnly(), cookie.sameSite(), expires, cookie.priority()) .then(success => !!success); } diff --git a/front_end/sdk/CookieParser.js b/front_end/sdk/CookieParser.js index c5b7b1d68d..915ae9564a 100644 --- a/front_end/sdk/CookieParser.js +++ b/front_end/sdk/CookieParser.js @@ -86,7 +86,7 @@ export default class CookieParser { if (kv.key.charAt(0) === '$' && this._lastCookie) { this._lastCookie.addAttribute(kv.key.slice(1), kv.value); } else if (kv.key.toLowerCase() !== '$version' && typeof kv.value === 'string') { - this._addCookie(kv, Type.Request); + this._addCookie(kv, SDK.Cookie.Type.Request); } this._advanceAndCheckCookieDelimiter(); } @@ -106,7 +106,7 @@ export default class CookieParser { if (this._lastCookie) { this._lastCookie.addAttribute(kv.key, kv.value); } else { - this._addCookie(kv, Type.Response); + this._addCookie(kv, SDK.Cookie.Type.Response); } if (this._advanceAndCheckCookieDelimiter()) { this._flushCookie(); @@ -135,7 +135,7 @@ export default class CookieParser { _flushCookie() { if (this._lastCookie) { this._lastCookie.setSize(this._originalInputLength - this._input.length - this._lastCookiePosition); - this._lastCookie._setCookieLine(this._lastCookieLine.replace('\n', '')); + this._lastCookie.setCookieLine(this._lastCookieLine.replace('\n', '')); } this._lastCookie = null; this._lastCookieLine = ''; @@ -181,7 +181,7 @@ export default class CookieParser { /** * @param {!KeyValue} keyValue - * @param {!Type} type + * @param {!SDK.Cookie.Type} type */ _addCookie(keyValue, type) { if (this._lastCookie) { @@ -216,253 +216,6 @@ class KeyValue { } } - -/** - * @unrestricted - */ -export class Cookie { - /** - * @param {string} name - * @param {string} value - * @param {?Type} type - * @param {!Protocol.Network.CookiePriority=} priority - */ - constructor(name, value, type, priority) { - this._name = name; - this._value = value; - this._type = type; - this._attributes = {}; - this._size = 0; - this._priority = /** @type {!Protocol.Network.CookiePriority} */ (priority || 'medium'); - /** @type {string|null} */ - this._cookieLine = null; - } - - /** - * @param {!Protocol.Network.Cookie} protocolCookie - * @return {!SDK.Cookie} - */ - static fromProtocolCookie(protocolCookie) { - const cookie = new SDK.Cookie(protocolCookie.name, protocolCookie.value, null, protocolCookie.priority); - cookie.addAttribute('domain', protocolCookie['domain']); - cookie.addAttribute('path', protocolCookie['path']); - cookie.addAttribute('port', protocolCookie['port']); - if (protocolCookie['expires']) { - cookie.addAttribute('expires', protocolCookie['expires'] * 1000); - } - if (protocolCookie['httpOnly']) { - cookie.addAttribute('httpOnly'); - } - if (protocolCookie['secure']) { - cookie.addAttribute('secure'); - } - if (protocolCookie['sameSite']) { - cookie.addAttribute('sameSite', protocolCookie['sameSite']); - } - cookie.setSize(protocolCookie['size']); - return cookie; - } - - /** - * @returns {string} - */ - key() { - return this.domain() + ' ' + this.name() + ' ' + this.path(); - } - - /** - * @return {string} - */ - name() { - return this._name; - } - - /** - * @return {string} - */ - value() { - return this._value; - } - - /** - * @return {?Type} - */ - type() { - return this._type; - } - - /** - * @return {boolean} - */ - httpOnly() { - return 'httponly' in this._attributes; - } - - /** - * @return {boolean} - */ - secure() { - return 'secure' in this._attributes; - } - - /** - * @return {!Protocol.Network.CookieSameSite} - */ - sameSite() { - // TODO(allada) This should not rely on _attributes and instead store them individually. - return /** @type {!Protocol.Network.CookieSameSite} */ (this._attributes['samesite']); - } - - /** - * @return {!Protocol.Network.CookiePriority} - */ - priority() { - return this._priority; - } - - /** - * @return {boolean} - */ - session() { - // RFC 2965 suggests using Discard attribute to mark session cookies, but this does not seem to be widely used. - // Check for absence of explicitly max-age or expiry date instead. - return !('expires' in this._attributes || 'max-age' in this._attributes); - } - - /** - * @return {string} - */ - path() { - return this._attributes['path']; - } - - /** - * @return {string} - */ - port() { - return this._attributes['port']; - } - - /** - * @return {string} - */ - domain() { - return this._attributes['domain']; - } - - /** - * @return {number} - */ - expires() { - return this._attributes['expires']; - } - - /** - * @return {string} - */ - maxAge() { - return this._attributes['max-age']; - } - - /** - * @return {number} - */ - size() { - return this._size; - } - - /** - * @return {string} - */ - url() { - return (this.secure() ? 'https://' : 'http://') + this.domain() + this.path(); - } - - /** - * @param {number} size - */ - setSize(size) { - this._size = size; - } - - /** - * @return {?Date} - */ - expiresDate(requestDate) { - // RFC 6265 indicates that the max-age attribute takes precedence over the expires attribute - if (this.maxAge()) { - const targetDate = requestDate === null ? new Date() : requestDate; - return new Date(targetDate.getTime() + 1000 * this.maxAge()); - } - - if (this.expires()) { - return new Date(this.expires()); - } - - return null; - } - - /** - * @return {!Object} - */ - attributes() { - return this._attributes; - } - - /** - * @param {string} key - * @param {string|number=} value - */ - addAttribute(key, value) { - const normalizedKey = key.toLowerCase(); - switch (normalizedKey) { - case 'priority': - this._priority = /** @type {!Protocol.Network.CookiePriority} */ (value); - break; - default: - this._attributes[normalizedKey] = value; - } - } - - /** - * @param {string} cookieLine - */ - _setCookieLine(cookieLine) { - this._cookieLine = cookieLine; - } - - /** - * @return {string|null} - */ - getCookieLine() { - return this._cookieLine; - } -} - -/** - * @enum {number} - */ -export const Type = { - Request: 0, - Response: 1 -}; - -/** - * @enum {string} - */ -export const Attributes = { - Name: 'name', - Value: 'value', - Size: 'size', - Domain: 'domain', - Path: 'path', - Expires: 'expires', - HttpOnly: 'httpOnly', - Secure: 'secure', - SameSite: 'sameSite', - Priority: 'priority', -}; - /* Legacy exported object */ self.SDK = self.SDK || {}; @@ -471,16 +224,3 @@ SDK = SDK || {}; /** @constructor */ SDK.CookieParser = CookieParser; - -/** @constructor */ -SDK.Cookie = Cookie; - -/** - * @enum {number} - */ -SDK.Cookie.Type = Type; - -/** - * @enum {string} - */ -SDK.Cookie.Attributes = Attributes; diff --git a/front_end/sdk/module.json b/front_end/sdk/module.json index f07b5c74df..95504e250e 100644 --- a/front_end/sdk/module.json +++ b/front_end/sdk/module.json @@ -372,6 +372,7 @@ "TargetManager.js", "Connections.js", "CompilerSourceMappingContentProvider.js", + "Cookie.js", "CookieModel.js", "CookieParser.js", "ProfileTreeModel.js", diff --git a/front_end/sdk/sdk.js b/front_end/sdk/sdk.js index eed43e6c90..a14fe3e00a 100644 --- a/front_end/sdk/sdk.js +++ b/front_end/sdk/sdk.js @@ -17,6 +17,7 @@ import * as ChildTargetManager from './ChildTargetManager.js'; import * as CompilerSourceMappingContentProvider from './CompilerSourceMappingContentProvider.js'; import * as Connections from './Connections.js'; import * as ConsoleModel from './ConsoleModel.js'; +import * as Cookie from './Cookie.js'; import * as CookieModel from './CookieModel.js'; import * as CookieParser from './CookieParser.js'; import * as CPUProfileDataModel from './CPUProfileDataModel.js'; @@ -69,6 +70,7 @@ export { CompilerSourceMappingContentProvider, Connections, ConsoleModel, + Cookie, CookieModel, CookieParser, CPUProfileDataModel, diff --git a/karma.conf.js b/karma.conf.js index 8273b7fd7d..00af6fe5fc 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -34,7 +34,8 @@ module.exports = function(config) { './test/unittests/**/*.ts': ['karma-typescript'], './front_end/common/*.js': instrumenterPreprocessors, './front_end/workspace/*.js': instrumenterPreprocessors, - './front_end/ui/**/*.js': instrumenterPreprocessors + './front_end/ui/**/*.js': instrumenterPreprocessors, + './front_end/sdk/*.js':instrumenterPreprocessors, }, browsers, diff --git a/test/unittests/front_end/sdk/Cookie.ts b/test/unittests/front_end/sdk/Cookie.ts new file mode 100644 index 0000000000..a5ac76f1f4 --- /dev/null +++ b/test/unittests/front_end/sdk/Cookie.ts @@ -0,0 +1,166 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +const {assert} = chai; + +import {Cookie} from '../../../../front_end/sdk/Cookie.js'; + +describe('Cookie', () => { + after(() => { + // FIXME(https://crbug.com/1006759): Remove after ESM work is complete + delete (self as any).SDK; + }); + + it('can be instantiated without issues', () => { + const cookie = new Cookie('name', 'value'); + + assert.equal(cookie.key(), '- name -'); + assert.equal(cookie.name(), 'name'); + assert.equal(cookie.value(), 'value'); + + assert.equal(cookie.type(), undefined); + assert.equal(cookie.httpOnly(), false); + assert.equal(cookie.secure(), false); + assert.equal(cookie.sameSite(), undefined); + assert.equal(cookie.priority(), 'Medium'); + assert.equal(cookie.session(), true); + assert.equal(cookie.path(), undefined); + assert.equal(cookie.port(), undefined); + assert.equal(cookie.domain(), undefined); + assert.equal(cookie.expires(), undefined); + assert.equal(cookie.maxAge(), undefined); + assert.equal(cookie.size(), 0); + assert.equal(cookie.url(), null); + assert.equal(cookie.getCookieLine(), undefined); + }); + + it('can be created from a protocol Cookie with all optional fields set', () => { + const expires = new Date().getTime() + 3600 * 1000; + const cookie = Cookie.fromProtocolCookie({ + domain: '.example.com', + expires: expires / 1000, + httpOnly: true, + name: 'name', + path: '/test', + sameSite: 'Strict', + secure: true, + session: false, + size: 23, + value: 'value', + priority: 'High' + }); + + assert.equal(cookie.key(), '.example.com name /test'); + assert.equal(cookie.name(), 'name'); + assert.equal(cookie.value(), 'value'); + + assert.equal(cookie.type(), undefined); + assert.equal(cookie.httpOnly(), true); + assert.equal(cookie.secure(), true); + assert.equal(cookie.sameSite(), 'Strict'); + assert.equal(cookie.session(), false); + assert.equal(cookie.path(), '/test'); + assert.equal(cookie.port(), undefined); + assert.equal(cookie.domain(), '.example.com'); + assert.equal(cookie.expires(), expires); + assert.equal(cookie.maxAge(), undefined); + assert.equal(cookie.size(), 23); + assert.equal(cookie.url(), 'https://.example.com/test'); + assert.equal(cookie.getCookieLine(), undefined); + }); + + it('can be created from a protocol Cookie with no optional fields set', () => { + const cookie = Cookie.fromProtocolCookie({ + domain: '.example.com', + name: 'name', + path: '/test', + size: 23, + value: 'value', + }); + + assert.equal(cookie.key(), '.example.com name /test'); + assert.equal(cookie.name(), 'name'); + assert.equal(cookie.value(), 'value'); + + assert.equal(cookie.type(), undefined); + assert.equal(cookie.httpOnly(), false); + assert.equal(cookie.secure(), false); + assert.equal(cookie.sameSite(), undefined); + assert.equal(cookie.priority(), 'Medium'); + assert.equal(cookie.session(), true); + assert.equal(cookie.path(), '/test'); + assert.equal(cookie.port(), undefined); + assert.equal(cookie.domain(), '.example.com'); + assert.equal(cookie.expires(), undefined); + assert.equal(cookie.maxAge(), undefined); + assert.equal(cookie.size(), 23); + assert.equal(cookie.url(), 'http://.example.com/test'); + assert.equal(cookie.getCookieLine(), undefined); + }); + + it('can handle secure urls', () => { + const cookie = new Cookie('name', 'value'); + cookie.addAttribute('Secure'); + cookie.addAttribute('Domain', 'example.com'); + cookie.addAttribute('Path', '/test'); + assert.equal(cookie.url(), 'https://example.com/test'); + }); + + it('can handle insecure urls', () => { + const cookie = new Cookie('name', 'value'); + cookie.addAttribute('Domain', 'example.com'); + cookie.addAttribute('Path', '/test'); + assert.equal(cookie.url(), 'http://example.com/test'); + }); + + it('can set attributes used as flags', () => { + const cookie = new Cookie('name', 'value'); + cookie.addAttribute('HttpOnly'); + assert.equal(cookie.httpOnly(), true); + }); + + it('can set attributes used as key=value', () => { + const cookie = new Cookie('name', 'value'); + cookie.addAttribute('Path', '/test'); + assert.equal(cookie.path(), '/test'); + }); + + it('can set initialize with a different priority', () => { + const cookie = new Cookie('name', 'value', null, 'High'); + assert.equal(cookie.priority(), 'High'); + }); + + it('can change the priority', () => { + const cookie = new Cookie('name', 'value'); + cookie.addAttribute('Priority', 'Low'); + assert.equal(cookie.priority(), 'Low'); + }); + + it('can set the cookie line', () => { + const cookie = new Cookie('name', 'value'); + cookie.setCookieLine('name=value') + assert.equal(cookie.getCookieLine(), 'name=value'); + }); + + it('can calculate the expiration date for session cookies', () => { + const cookie = new Cookie('name', 'value'); + assert.equal(cookie.expiresDate(), null); + }); + + it('can calculate the expiration date for max age cookies', () => { + const cookie = new Cookie('name', 'value'); + const now = new Date(); + const expires = Math.floor(now.getTime()) + 3600 * 1000; + cookie.addAttribute('Max-Age', '3600'); + assert.equal(cookie.expiresDate(now).toISOString(), new Date(expires).toISOString()); + }); + + it('can calculate the expiration date for cookies with expires attribute', () => { + const cookie = new Cookie('name', 'value'); + const now = new Date(); + const expires = Math.floor(now.getTime()) + 3600 * 1000; + cookie.addAttribute('Expires', expires); + assert.equal(cookie.expiresDate(now).toISOString(), new Date(expires).toISOString()); + }); +}); \ No newline at end of file