Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CDP cookie matching algorithm #5862

Merged
merged 5 commits into from
Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions packages/server/lib/browsers/cdp_automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import Bluebird from 'bluebird'
import cdp from 'devtools-protocol'
import { cors } from '@packages/network'
import debugModule from 'debug'
import tough from 'tough-cookie'

const debugVerbose = debugModule('cypress-verbose:server:browsers:cdp_automation')

Expand All @@ -18,18 +17,29 @@ interface CyCookie {
httpOnly: boolean
}

// Cypress uses the webextension-style filtering
// https://developer.chrome.com/extensions/cookies#method-getAll
type CyCookieFilter = chrome.cookies.GetAllDetails
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not import the chrome types at the top vs putting them in the compilerOptions? I feel like this comes out of nowhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i do import 'chrome', build fails

if i do import chrome from 'chrome', it says 'chrome' is not a module

i can dig into why import 'chrome' makes the build fail if you'd like, i'm not sure this is the best way either

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ask @tgriesser in #typescript channel

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm he's on his way to the airport - @tgriesser or @thlorenz can you explain why this is necessary whenever you have time?


type SendDebuggerCommand = (message: string, data?: any) => Bluebird<any>

const cookieMatches = (cookie: CyCookie, data) => {
if (data.domain && !tough.domainMatch(cookie.domain, data.domain)) {
export const _domainIsWithinSuperdomain = (domain: string, suffix: string) => {
const suffixParts = suffix.split('.').filter(_.identity)
const domainParts = domain.split('.').filter(_.identity)

return _.isEqual(suffixParts, domainParts.slice(domainParts.length - suffixParts.length))
}

export const _cookieMatches = (cookie: CyCookie, filter: CyCookieFilter) => {
if (filter.domain && !(cookie.domain && _domainIsWithinSuperdomain(cookie.domain, filter.domain))) {
return false
}

if (data.path && !tough.pathMatch(cookie.path, data.path)) {
if (filter.path && filter.path !== cookie.path) {
return false
}

if (data.name && data.name !== cookie.name) {
if (filter.name && filter.name !== cookie.name) {
return false
}

Expand Down Expand Up @@ -86,12 +96,12 @@ export const CdpAutomation = (sendDebuggerCommandFn: SendDebuggerCommand) => {
return cookie
}

const getAllCookies = (filter) => {
const getAllCookies = (filter: CyCookieFilter) => {
return sendDebuggerCommandFn('Network.getAllCookies')
.then((result: cdp.Network.GetAllCookiesResponse) => {
return normalizeGetCookies(result.cookies)
.filter((cookie: CyCookie) => {
const matches = cookieMatches(cookie, filter)
const matches = _cookieMatches(cookie, filter)

debugVerbose('cookie matches filter? %o', { matches, cookie, filter })

Expand All @@ -100,7 +110,7 @@ export const CdpAutomation = (sendDebuggerCommandFn: SendDebuggerCommand) => {
})
}

const getCookiesByUrl = (url) => {
const getCookiesByUrl = (url): Bluebird<CyCookie[]> => {
return sendDebuggerCommandFn('Network.getCookies', {
urls: [url],
})
Expand All @@ -109,8 +119,8 @@ export const CdpAutomation = (sendDebuggerCommandFn: SendDebuggerCommand) => {
})
}

const getCookie = (data): Bluebird<CyCookie | null> => {
return getAllCookies(data)
const getCookie = (filter: CyCookieFilter): Bluebird<CyCookie | null> => {
return getAllCookies(filter)
.then((cookies) => {
return _.get(cookies, 0, null)
})
Expand Down
1 change: 1 addition & 0 deletions packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
"@cypress/json-schemas": "5.33.0",
"@cypress/sinon-chai": "1.1.0",
"@types/chai-as-promised": "7.1.2",
"@types/chrome": "0.0.91",
"babel-plugin-add-module-exports": "1.0.2",
"babelify": "10.0.0",
"bin-up": "1.2.2",
Expand Down
58 changes: 57 additions & 1 deletion packages/server/test/unit/browsers/cdp_automation_spec.coffee
Original file line number Diff line number Diff line change
@@ -1,7 +1,63 @@
require("../../spec_helper")
{ CdpAutomation } = require("#{root}../lib/browsers/cdp_automation")
{
CdpAutomation,
_cookieMatches,
_domainIsWithinSuperdomain
} = require("#{root}../lib/browsers/cdp_automation")

context "lib/browsers/cdp_automation", ->
context "._domainIsWithinSuperdomain", ->
it "matches as expected", ->
[
{
domain: 'a.com'
suffix: 'a.com'
expected: true
}
{
domain: 'a.com'
suffix: 'b.com'
expected: false
}
{
domain: 'c.a.com'
suffix: 'a.com'
expected: true
}
{
domain: 'localhost'
suffix: 'localhost'
expected: true
}
{
domain: '.localhost'
suffix: '.localhost'
expected: true
}
{
domain: '.localhost'
suffix: 'reddit.com'
expected: false
}
].forEach ({ domain, suffix, expected }, i) =>
expect(_domainIsWithinSuperdomain(domain, suffix)).to.eq(expected)

context "._cookieMatches", ->
it "matches as expected", ->
[
{
cookie: { domain: 'example.com' }
filter: { domain: 'example.com' }
expected: true
}
{
cookie: { domain: 'example.com' }
filter: { domain: '.example.com' }
expected: true
}
].forEach ({ cookie, filter, expected }) =>
expect(_cookieMatches(cookie, filter)).to.eq(expected)

context ".CdpAutomation", ->
beforeEach ->
@sendDebuggerCommand = sinon.stub()
Expand Down
5 changes: 4 additions & 1 deletion packages/server/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@
],
"files": [
"./../ts/index.d.ts"
]
],
"compilerOptions": {
"types": ["mocha", "node", "chrome"]
}
}