Skip to content

Commit

Permalink
Revisit encoding/decoding implementation
Browse files Browse the repository at this point in the history
Our RFC 6265 based cookie name + value encoding was based on the
requirements for server implementers, and as such too strict.

We need to look at RFC 6265 section 5.2, requirements for user agents.

But even the algorithm described in section 5.2 isn't that relevant
for us, in that we had to follow the steps when writing a cookie with
this library. This is left to the browser vendors who have to implement
the setter functionality behind `document.cookie`. All we have to do is
avoid surprises when said algorithm is being followed while
`document.cookie = ...` is being executed with the cookie string we're
constructing.

It means we have to encode ";" and "=" in the cookie name, and ";" in
the cookie value.

Follow-up to discussion in #595.
  • Loading branch information
carhartl committed Mar 2, 2020
1 parent d727ba6 commit b086d52
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 1,416 deletions.
39 changes: 3 additions & 36 deletions Gruntfile.js
@@ -1,33 +1,8 @@
function encodingMiddleware (request, response, next) {
const URL = require('url').URL
var url = new URL(request.url, 'http://localhost')

if (url.pathname !== '/encoding') {
next()
return
}

var cookieName = url.searchParams.get('name')
var cookieValue = url.searchParams.get('value')

response.setHeader('content-type', 'application/json')
response.end(
JSON.stringify({
name: cookieName,
value: cookieValue
})
)
}

const config = {
qunit: {
all: {
options: {
urls: [
'http://127.0.0.1:9998/',
'http://127.0.0.1:9998/module.html',
'http://127.0.0.1:9998/encoding.html?integration_baseurl=http://127.0.0.1:9998/'
]
urls: ['http://127.0.0.1:9998/', 'http://127.0.0.1:9998/module.html']
}
}
},
Expand Down Expand Up @@ -58,11 +33,7 @@ const config = {
'build-qunit': {
options: {
port: 9998,
base: ['.', 'test'],
middleware: function (connect, options, middlewares) {
middlewares.unshift(encodingMiddleware)
return middlewares
}
base: ['.', 'test']
}
},
tests: {
Expand All @@ -71,11 +42,7 @@ const config = {
base: ['.', 'test'],
open: 'http://127.0.0.1:10000',
keepalive: true,
livereload: true,
middleware: function (connect, options, middlewares) {
middlewares.unshift(encodingMiddleware)
return middlewares
}
livereload: true
}
}
},
Expand Down
28 changes: 6 additions & 22 deletions src/api.mjs
Expand Up @@ -16,11 +16,9 @@ function init (converter, defaultAttributes) {
attributes.expires = attributes.expires.toUTCString()
}

value = converter.write(value, key)
key = rfc6265Converter.write(key).replace(/=/g, '%3D')

key = encodeURIComponent(key)
.replace(/%(2[346B]|5E|60|7C)/g, decodeURIComponent)
.replace(/[()]/g, escape)
value = converter.write(String(value), key)

var stringifiedAttributes = ''
for (var attributeName in attributes) {
Expand All @@ -34,13 +32,6 @@ function init (converter, defaultAttributes) {
continue
}

// Considers RFC 6265 section 5.2:
// ...
// 3. If the remaining unparsed-attributes contains a %x3B (";")
// character:
// Consume the characters of the unparsed-attributes up to,
// not including, the first %x3B (";") character.
// ...
stringifiedAttributes += '=' + attributes[attributeName].split(';')[0]
}

Expand All @@ -59,19 +50,12 @@ function init (converter, defaultAttributes) {
for (var i = 0; i < cookies.length; i++) {
var parts = cookies[i].split('=')
var cookie = parts.slice(1).join('=')
var name = rfc6265Converter.read(parts[0]).replace(/%3D/g, '=')
jar[name] = converter.read(cookie, name)

if (cookie[0] === '"') {
cookie = cookie.slice(1, -1)
if (key === name) {
break
}

try {
var name = rfc6265Converter.read(parts[0])
jar[name] = converter.read(cookie, name)

if (key === name) {
break
}
} catch (e) {}
}

return key ? jar[key] : jar
Expand Down
7 changes: 2 additions & 5 deletions src/rfc6265.mjs
@@ -1,11 +1,8 @@
export default {
read: function (value) {
return value.replace(/(%[\dA-F]{2})+/gi, decodeURIComponent)
return value.replace(/%3B/g, ';')
},
write: function (value) {
return encodeURIComponent(value).replace(
/%(2[346BF]|3[AC-F]|40|5[BDE]|60|7[BCD])/g,
decodeURIComponent
)
return value.replace(/;/g, '%3B')
}
}
18 changes: 0 additions & 18 deletions test/encoding.html

This file was deleted.

0 comments on commit b086d52

Please sign in to comment.