From 9c01a546ae869fa842d3acafa9d686c4d8d2a7e5 Mon Sep 17 00:00:00 2001 From: Andrew Williams Date: Sun, 5 Sep 2021 15:00:41 -0700 Subject: [PATCH] Update existing cookie size tests and add new ones Updates several existing tests and adds new ones related to cookie name+value size and cookie attribute value size restrictions introduced recently into the latest draft of RFC6265bis. Bug: 1223516 Change-Id: I8c2857683b7fc7cfeebd6cc17605eb87dcbbdf94 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3097489 Commit-Queue: Andrew Williams Reviewed-by: Steven Bingler Cr-Commit-Position: refs/heads/main@{#918451} --- cookies/size/attributes.www.sub.html | 84 +++++++++++++++++++++++++--- cookies/size/name-and-value.html | 5 ++ cookies/value/value.html | 42 ++++++++++---- 3 files changed, 113 insertions(+), 18 deletions(-) diff --git a/cookies/size/attributes.www.sub.html b/cookies/size/attributes.www.sub.html index 3004007ec8e053..7270c011c4a39e 100644 --- a/cookies/size/attributes.www.sub.html +++ b/cookies/size/attributes.www.sub.html @@ -29,17 +29,85 @@ defaultPath: false, }, { - // This page opens on the www subdomain, so we set domain to {{host}} - // to see if anything works as expected. - cookie: `test=3; domain=${host}; domain=${"a".repeat(1024)}.com`, - expected: "test=3", - name: "Too long domain attribute (>1024 bytes) is ignored; previous valid domain wins." + // Look for the cookie using the default path to ensure that it + // doesn't show up if the path attribute actually takes effect. + cookie: `test=3; path=/${"a".repeat(1023)};`, + expected: "", + name: "Max size path attribute (1024 bytes) is not ignored", }, { - cookie: `test=4; domain=${"a".repeat(1024)}.com; domain=${host}`, + // Look for the cookie using the default path to ensure that it + // shows up if the path is ignored. + cookie: `test=4; path=/${"a".repeat(1024)};`, expected: "test=4", - name: "Too long domain attribute (>1024 bytes) is ignored; next valid domain wins." - } + name: "Too long path attribute (>1024 bytes) is ignored", + }, + { + // This page opens on the www subdomain, so we set domain to {{host}} + // to see if anything works as expected. Using a valid domain other + // than ${host} will cause the cookie to fail to be set. + + // NOTE: the domain we use for testing here is technically invalid per + // the RFCs that define the format of domain names, but currently + // neither RFC6265bis or the major browsers enforce those restrictions + // when parsing cookie domain attributes. If that changes, update these + // tests. + cookie: `test=5; domain=${host}; domain=${"a".repeat(1024)}.com`, + expected: "test=5", + name: "Too long domain attribute (>1024 bytes) is ignored; previous valid domain wins.", + }, + { + cookie: `test=6; domain=${"a".repeat(1024)}.com; domain=${host}`, + expected: "test=6", + name: "Too long domain attribute (>1024 bytes) is ignored; next valid domain wins.", + }, + { + cookie: `test=7; domain=${"a".repeat(1020)}.com;`, + expected: "", + name: "Max size domain attribute (1024 bytes) is not ignored" + }, + { + cookie: `test=8; domain=${"a".repeat(1021)}.com;`, + expected: "test=8", + name: "Too long domain attribute (>1024 bytes) is ignored" + }, + { + cookie: cookieStringWithNameAndValueLengths(2048, 2048) + + `; domain=${"a".repeat(1020)}.com; domain=${host}`, + expected: cookieStringWithNameAndValueLengths(2048, 2048), + name: "Set cookie with max size name/value pair and max size attribute value", + }, + { + // RFC6265bis doesn't specify a maximum size of the entire Set-Cookie + // header, although some browsers do + cookie: cookieStringWithNameAndValueLengths(2048, 2048) + + `; domain=${"a".repeat(1020)}.com` + + `; domain=${"a".repeat(1020)}.com` + + `; domain=${"a".repeat(1020)}.com` + + `; domain=${"a".repeat(1020)}.com; domain=${host}`, + expected: cookieStringWithNameAndValueLengths(2048, 2048), + name: "Set cookie with max size name/value pair and multiple max size attributes (>8k bytes total)", + }, + { + cookie: `test=11; max-age=${"1".repeat(1024)};`, + expected: "test=11", + name: "Max length Max-Age attribute value (1024 bytes) doesn't cause cookie rejection" + }, + { + cookie: `test=12; max-age=${"1".repeat(1025)};`, + expected: "test=12", + name: "Too long Max-Age attribute value (>1024 bytes) doesn't cause cookie rejection" + }, + { + cookie: `test=13; max-age=-${"1".repeat(1023)};`, + expected: "", + name: "Max length negative Max-Age attribute value (1024 bytes) doesn't get ignored" + }, + { + cookie: `test=14; max-age=-${"1".repeat(1024)};`, + expected: "test=14", + name: "Too long negative Max-Age attribute value (>1024 bytes) gets ignored" + }, ]; for (const test of attrSizeTests) { diff --git a/cookies/size/name-and-value.html b/cookies/size/name-and-value.html index 2122c0d461a07d..c38c5c8e5aedcc 100644 --- a/cookies/size/name-and-value.html +++ b/cookies/size/name-and-value.html @@ -65,6 +65,11 @@ expected: "", name: "Ignore name-less cookie (without leading =) with value larger than 4096 bytes", }, + { + cookie: cookieStringWithNameAndValueLengths(2048, 2048) + '; Max-Age:43110;', + expected: cookieStringWithNameAndValueLengths(2048, 2048), + name: "Set max-size cookie that also has an attribute", + }, ]; for (const test of nameAndValueSizeTests) { diff --git a/cookies/value/value.html b/cookies/value/value.html index d811e66b6b9a33..27c2765f26848c 100644 --- a/cookies/value/value.html +++ b/cookies/value/value.html @@ -34,7 +34,7 @@ { cookie: 'test="4zz ;', expected: 'test="4zz', - name: "Ingore whitespace at the end of value", + name: "Ignore whitespace at the end of value", }, { cookie: 'test="5zzz " "ppp" ;', @@ -67,19 +67,16 @@ name: "Set nameless cookie followed by '=' to its value", }, { - // 7 + 4089 = 4096 - cookie: `test=11${"a".repeat(4089)}`, - expected: `test=11${"a".repeat(4089)}`, - name: "Set cookie with large value ( = 4kb)", + // 4 + 2 + 4090 = 4096 + cookie: `test=11${"a".repeat(4090)}`, + expected: `test=11${"a".repeat(4090)}`, + name: "Set cookie with large name + value ( = 4kb)", }, { - // 7 + 4091 = 4098 - // Note: Chrome includes = in its length, Firefox does not - // For now, make this 4098 until the spec clarifies: - // https://github.com/httpwg/http-extensions/issues/1340 + // 4 + 2 + 4091 = 4097 cookie: `test=12${"a".repeat(4091)}`, expected: "", - name: "Ignore cookie with large value ( > 4kb)", + name: "Ignore cookie with large name + value ( > 4kb)", }, { cookie: `test=13\nZYX`, @@ -136,6 +133,31 @@ expected: "test=%32%33", name: "URL-encoded cookie value is not decoded", }, + { + cookie: "test24==", + expected: "test24==", + name: "Set cookie with value set to =", + }, + { + cookie: 'test=25=25', + expected: 'test=25=25', + name: "Set cookie with one = inside an unquoted value", + }, + { + cookie: 'test=26=26=26', + expected: 'test=26=26=26', + name: "Set cookie with two = inside an unquoted value", + }, + { + cookie: 'test=27 test', + expected: 'test=27 test', + name: "Set cookie with a space character in the value", + }, + { + cookie: ' test test28 ;', + expected: 'test test28', + name: "Set a nameless cookie with a space character in the value", + }, ]; for (const test of valueTests) {