Skip to content

Commit fe7614a

Browse files
tolyotolyo
committed
Cookie hardedning
1 parent e0845f5 commit fe7614a

File tree

4 files changed

+169
-47
lines changed

4 files changed

+169
-47
lines changed

‎@types/services/cookie/cookie.d.ts‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,15 @@ export class CookieService {
1010
/**
1111
* @param {ng.CookieOptions} defaults
1212
* Default cookie attributes defined by `$cookiesProvider.defaults`.
13+
* @param {ng.ExceptionHandlerService} $exceptionHandler
1314
*/
14-
constructor(defaults: ng.CookieOptions);
15-
/** @type {CookieOptions} */
16-
defaults: CookieOptions;
15+
constructor(
16+
defaults: ng.CookieOptions,
17+
$exceptionHandler: ng.ExceptionHandlerService,
18+
);
19+
/** @type {ng.CookieOptions} */
20+
defaults: ng.CookieOptions;
21+
$exceptionHandler: import("../exception/interface.ts").ErrorHandler;
1722
/**
1823
* Retrieves a raw cookie value.
1924
*
@@ -27,6 +32,7 @@ export class CookieService {
2732
* @template T
2833
* @param {string} key
2934
* @returns {T|null}
35+
* @throws {SyntaxError} if cookie JSON is invalid
3036
*/
3137
getObject<T>(key: string): T | null;
3238
/**
@@ -40,27 +46,30 @@ export class CookieService {
4046
*
4147
* @param {string} key
4248
* @param {string} value
43-
* @param {CookieOptions} [options]
49+
* @param {ng.CookieOptions} [options]
4450
*/
45-
put(key: string, value: string, options?: CookieOptions): void;
51+
put(key: string, value: string, options?: ng.CookieOptions): void;
4652
/**
4753
* Serializes an object as JSON and stores it as a cookie.
4854
*
4955
* @param {string} key
5056
* @param {any} value
51-
* @param {CookieOptions} [options]
57+
* @param {ng.CookieOptions} [options]
58+
* @throws {TypeError} if Object cannot be converted to JSON
5259
*/
53-
putObject(key: string, value: any, options?: CookieOptions): void;
60+
putObject(key: string, value: any, options?: ng.CookieOptions): void;
5461
/**
5562
* Removes a cookie by setting an expired date.
5663
*
5764
* @param {string} key
58-
* @param {CookieOptions} [options]
65+
* @param {ng.CookieOptions} [options]
5966
*/
60-
remove(key: string, options?: CookieOptions): void;
67+
remove(key: string, options?: ng.CookieOptions): void;
6168
}
6269
export class CookieProvider {
6370
defaults: {};
64-
$get(): CookieService;
71+
$get: (
72+
| string
73+
| (($exceptionHandler: ng.ExceptionHandlerService) => CookieService)
74+
)[];
6575
}
66-
export type CookieOptions = import("./interface.ts").CookieOptions;

‎src/core/di/injector.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,16 +216,16 @@ export function createInjector(modulesToLoad, strictDi = false) {
216216
const cookieOpts = options.cookie ?? {};
217217

218218
return createPersistentProxy(instance, name, {
219-
getItem: (key) => {
219+
getItem(key) {
220220
const raw = $cookie.get(key);
221221
return raw == null ? null : raw;
222222
},
223223

224-
setItem: (key, value) => {
224+
setItem(key, value) {
225225
$cookie.put(key, value, cookieOpts);
226226
},
227227

228-
removeItem: (key) => {
228+
removeItem(key) {
229229
$cookie.remove(key, cookieOpts);
230230
},
231231

‎src/services/cookie/cookie.js‎

Lines changed: 111 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
/**
2-
* @typedef {import("./interface.ts").CookieOptions} CookieOptions
3-
*/
1+
import { $injectTokens } from "../../injection-tokens.js";
2+
import {
3+
assert,
4+
isDefined,
5+
isFunction,
6+
isNullOrUndefined,
7+
isNumber,
8+
isObject,
9+
isString,
10+
} from "../../shared/utils.js";
411

512
/**
613
* @returns {Record<string,string>}
@@ -13,6 +20,7 @@ function parseCookies() {
1320
const parts = document.cookie.split("; ");
1421
for (const part of parts) {
1522
const eq = part.indexOf("=");
23+
if (eq === -1) continue; // skip malformed cookie
1624
const key = decodeURIComponent(part.substring(0, eq));
1725
const val = decodeURIComponent(part.substring(eq + 1));
1826
out[key] = val;
@@ -23,27 +31,70 @@ function parseCookies() {
2331
/** Utility: stringify options */
2432
/**
2533
*
26-
* @param {CookieOptions} opts
34+
* @param {ng.CookieOptions} opts
35+
* @returns {string}
36+
*/
37+
/**
38+
* Build cookie options string from an options object.
39+
* Safely validates types for path, domain, expires, secure, and samesite.
40+
*
41+
* @param {ng.CookieOptions} opts
2742
* @returns {string}
43+
* @throws {TypeError} if any of options are invalid
2844
*/
2945
function buildOptions(opts = {}) {
30-
let s = "";
46+
const parts = [];
3147

32-
if (opts.path) s += `;path=${opts.path}`;
33-
if (opts.domain) s += `;domain=${opts.domain}`;
48+
// Path
49+
if (isDefined(opts.path)) {
50+
if (!isString(opts.path)) throw new TypeError(`badarg:path ${opts.path}`);
51+
parts.push(`path=${opts.path}`);
52+
}
3453

35-
if (opts.expires) {
36-
const exp =
37-
opts.expires instanceof Date
38-
? opts.expires.toUTCString()
39-
: new Date(opts.expires).toUTCString();
40-
s += `;expires=${exp}`;
54+
// Domain
55+
if (isDefined(opts.domain)) {
56+
if (!isString(opts.domain))
57+
throw new TypeError(`badarg:domain ${opts.domain}`);
58+
parts.push(`domain=${opts.domain}`);
4159
}
4260

43-
if (opts.secure) s += `;secure`;
44-
if (opts.samesite) s += `;samesite=${opts.samesite}`;
61+
// Expires
62+
if (opts.expires != null) {
63+
let expDate;
64+
65+
if (opts.expires instanceof Date) {
66+
expDate = opts.expires;
67+
} else if (isNumber(opts.expires) || isString(opts.expires)) {
68+
expDate = new Date(opts.expires);
69+
} else {
70+
throw new TypeError(`badarg:expires ${String(opts.expires)}`);
71+
}
72+
73+
if (isNaN(expDate.getTime())) {
74+
throw new TypeError(`badarg:expires ${String(opts.expires)}`);
75+
}
4576

46-
return s;
77+
parts.push(`expires=${expDate.toUTCString()}`);
78+
}
79+
80+
// Secure
81+
if (opts.secure) {
82+
parts.push("secure");
83+
}
84+
85+
// SameSite
86+
if (isDefined(opts.samesite)) {
87+
if (!isString(opts.samesite))
88+
throw new TypeError(`badarg:samesite ${opts.samesite}`);
89+
const s = opts.samesite.toLowerCase();
90+
if (!["lax", "strict", "none"].includes(s)) {
91+
throw new TypeError(`badarg:samesite ${opts.samesite}`);
92+
}
93+
parts.push(`samesite=${s}`);
94+
}
95+
96+
// Join all parts with semicolons
97+
return parts.length ? ";" + parts.join(";") : "";
4798
}
4899

49100
/**
@@ -58,10 +109,14 @@ export class CookieService {
58109
/**
59110
* @param {ng.CookieOptions} defaults
60111
* Default cookie attributes defined by `$cookiesProvider.defaults`.
112+
* @param {ng.ExceptionHandlerService} $exceptionHandler
61113
*/
62-
constructor(defaults) {
63-
/** @type {CookieOptions} */
64-
this.defaults = defaults;
114+
constructor(defaults, $exceptionHandler) {
115+
assert(isObject(defaults), "badarg");
116+
assert(isFunction($exceptionHandler), "badarg");
117+
/** @type {ng.CookieOptions} */
118+
this.defaults = Object.freeze({ ...defaults });
119+
this.$exceptionHandler = $exceptionHandler;
65120
}
66121

67122
/**
@@ -71,6 +126,7 @@ export class CookieService {
71126
* @returns {string|null}
72127
*/
73128
get(key) {
129+
assert(isString(key), "badarg");
74130
const all = parseCookies();
75131
return all[key] || null;
76132
}
@@ -81,14 +137,18 @@ export class CookieService {
81137
* @template T
82138
* @param {string} key
83139
* @returns {T|null}
140+
* @throws {SyntaxError} if cookie JSON is invalid
84141
*/
85142
getObject(key) {
143+
assert(isString(key), "badarg");
86144
const raw = this.get(key);
87145
if (!raw) return null;
88146
try {
89147
return /** @type {T} */ (JSON.parse(raw));
90-
} catch {
91-
return null;
148+
} catch (err) {
149+
const error = new SyntaxError(`badparse: "${key}" => ${err.message}`);
150+
this.$exceptionHandler(error);
151+
throw error;
92152
}
93153
}
94154

@@ -106,35 +166,53 @@ export class CookieService {
106166
*
107167
* @param {string} key
108168
* @param {string} value
109-
* @param {CookieOptions} [options]
169+
* @param {ng.CookieOptions} [options]
110170
*/
111171
put(key, value, options = {}) {
172+
assert(isString(key), "badarg: key");
173+
assert(isString(value), "badarg: value");
112174
const encodedKey = encodeURIComponent(key);
113175
const encodedVal = encodeURIComponent(value);
114176

115-
document.cookie =
116-
`${encodedKey}=${encodedVal}` +
117-
buildOptions({ ...this.defaults, ...options });
177+
try {
178+
document.cookie =
179+
`${encodedKey}=${encodedVal}` +
180+
buildOptions({ ...this.defaults, ...options });
181+
} catch (e) {
182+
this.$exceptionHandler(e);
183+
throw e;
184+
}
118185
}
119186

120187
/**
121188
* Serializes an object as JSON and stores it as a cookie.
122189
*
123190
* @param {string} key
124191
* @param {any} value
125-
* @param {CookieOptions} [options]
192+
* @param {ng.CookieOptions} [options]
193+
* @throws {TypeError} if Object cannot be converted to JSON
126194
*/
127195
putObject(key, value, options) {
128-
this.put(key, JSON.stringify(value), options);
196+
assert(isString(key), "badarg: key");
197+
assert(!isNullOrUndefined(key), "badarg: key");
198+
try {
199+
const str = JSON.stringify(value);
200+
this.put(key, str, options);
201+
} catch (err) {
202+
const error = new TypeError(`badserialize: "${key}" => ${err.message}`);
203+
this.$exceptionHandler(error);
204+
throw error;
205+
}
129206
}
130207

131208
/**
132209
* Removes a cookie by setting an expired date.
133210
*
134211
* @param {string} key
135-
* @param {CookieOptions} [options]
212+
* @param {ng.CookieOptions} [options]
136213
*/
137214
remove(key, options = {}) {
215+
assert(isString(key), "badarg");
138216
this.put(key, "", {
139217
...this.defaults,
140218
...options,
@@ -148,7 +226,9 @@ export class CookieProvider {
148226
this.defaults = {};
149227
}
150228

151-
$get() {
152-
return new CookieService(this.defaults);
153-
}
229+
$get = [
230+
$injectTokens.$exceptionHandler,
231+
/** @param {ng.ExceptionHandlerService} $exceptionHandler */
232+
($exceptionHandler) => new CookieService(this.defaults, $exceptionHandler),
233+
];
154234
}

‎src/services/cookie/cookie.spec.js‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,12 @@ describe("$cookie service", () => {
5151
expect($cookie.getObject("user")).toEqual(obj);
5252
});
5353

54-
it("should return null for invalid JSON in getObject", () => {
54+
it("should throw SyntaxError for invalid JSON in getObject", () => {
5555
document.cookie = "broken={unclosed";
56-
expect($cookie.getObject("broken")).toBeNull();
56+
expect(() => $cookie.getObject("broken")).toThrowError(
57+
SyntaxError,
58+
/^badparse: "broken" =>/,
59+
);
5760
});
5861

5962
it("getAll should return all cookies as an object", () => {
@@ -107,6 +110,36 @@ describe("$cookie service", () => {
107110
expect($cookie.get("c2")).toBe("v2");
108111
});
109112

113+
it("should throw TypeError if expires is invalid type", () => {
114+
const invalidValues = [true, {}, [], () => {}, Symbol()];
115+
invalidValues.forEach((val) => {
116+
expect(() => $cookie.put("badExp", "x", { expires: val })).toThrowError(
117+
TypeError,
118+
/^badarg:expires/,
119+
);
120+
});
121+
});
122+
123+
it("should throw TypeError if expires is an invalid date", () => {
124+
expect(() =>
125+
$cookie.put("badDate", "x", { expires: "invalid-date" }),
126+
).toThrowError(TypeError, /^badarg:expires/);
127+
});
128+
129+
it("should accept a valid Date object in expires", () => {
130+
const exp = new Date(Date.now() + 1000 * 60); // 1 minute in future
131+
expect(() => $cookie.put("goodDate", "v", { expires: exp })).not.toThrow();
132+
expect($cookie.get("goodDate")).toBe("v");
133+
});
134+
135+
it("should accept a valid date string in expires", () => {
136+
const exp = new Date(Date.now() + 1000 * 60).toUTCString();
137+
expect(() =>
138+
$cookie.put("goodString", "v", { expires: exp }),
139+
).not.toThrow();
140+
expect($cookie.get("goodString")).toBe("v");
141+
});
142+
110143
function clearCookies() {
111144
const cookies = document.cookie.split(";").map((c) => c.trim());
112145
cookies.forEach((c) => {

0 commit comments

Comments
 (0)