feat(core): Parse individual cookies from cookie header #18325
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| } | ||
|
|
||
| return spanAttributes; | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
I think bugbot is right with the cookie/set-cookie difference, so let's double check this.
one more Q: Do we need to adjust the semantic conventions for "sub" http headers? From what I can tell the spec covers arbitrary keys afoter http.request.headers.<key> but I just want to make sure <key> could contain another . as in the cookie header attribute case.
| } else if (typeof value === 'string') { | ||
| spanAttributes[normalizedKey] = value; | ||
| const lowerCasedHeaderKey = key.toLowerCase(); | ||
| const isCookieHeader = lowerCasedHeaderKey === 'cookie' || lowerCasedHeaderKey === 'set-cookie'; |
There was a problem hiding this comment.
l: probably saves us a few bytes:
| const isCookieHeader = lowerCasedHeaderKey === 'cookie' || lowerCasedHeaderKey === 'set-cookie'; | |
| const isCookieHeader = /^(set-)cookie$?/.test(lowerCasedHeaderKey) |
| const lowerCasedHeaderKey = key.toLowerCase(); | ||
| const isCookieHeader = lowerCasedHeaderKey === 'cookie' || lowerCasedHeaderKey === 'set-cookie'; | ||
|
|
||
| if (isCookieHeader && typeof value === 'string' && value !== '') { |
There was a problem hiding this comment.
q: do we handle arrays of cookie headers? (or is this not relevant for cookie/set-cookie?)
There was a problem hiding this comment.
For the sake of simplicity (as cookies are one string), an array would just be parsed as ...header.cookie: [Filtered]. There is also a test that checks that a cookie attribute is always filtered.
| }); | ||
|
|
||
| it('attaches and filters sensitive a set-cookie header', () => { | ||
| const headers1 = { 'Set-Cookie': 'user_session=def456' }; |
There was a problem hiding this comment.
l: let's add or adjust a test here for a set-cookie header with additional properties (e.g. like max-age)
| const isSetCookie = lowerCasedHeaderKey === 'set-cookie'; | ||
| const semicolonIndex = value.indexOf(';'); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Parse each individual cookie header and filter sensitive cookies to at least know which keys the cookie string included.
Follow-up on #18311
Closes #18441