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

Fixed cookies parsing bug in auto captured cookies #1727

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

ninadbstack
Copy link
Contributor

@ninadbstack ninadbstack commented Sep 16, 2024

Context:

  • Cookie parsing logic failed when cookies were encoded as base64 and had = at the end.
  • When servers used strict base64 cookie check this ended up with error response causing ERR_HTTP2_PROTOCOL_ERROR in discovery chromium.

This PR fixes the logic and refactors the function.
Also few unrelated logging improvements.

Secure cookie ->

  • Sometime we are getting Protocol error (Network.setCookies): Invalid cookie fields because the percy captured cookie did not capture the secure flag
  • Adding support for secure flag that can be set by setting this env variable PERCY_SECURE_COOKIES to true.

@ninadbstack ninadbstack requested a review from a team as a code owner September 16, 2024 14:59
@ninadbstack ninadbstack changed the title Fixed cookies parsing bug Fixed cookies parsing bug in auto captured cookies Sep 16, 2024
@prklm10 prklm10 force-pushed the PER-3681-fix-cookies-parsing-bug branch from bef0dc4 to b98112e Compare September 18, 2024 05:49
@ninadbstack ninadbstack merged commit 10a443f into master Sep 18, 2024
71 checks passed
@ninadbstack ninadbstack deleted the PER-3681-fix-cookies-parsing-bug branch September 18, 2024 05:59
@ninadbstack ninadbstack added the 🐛 bug Something isn't working label Sep 18, 2024
Amit3200 pushed a commit that referenced this pull request Sep 29, 2024
chinmay-browserstack pushed a commit that referenced this pull request Sep 30, 2024
* Fixed cookies parsing bug (#1727)

* v1.29.4-beta.3 (#1729)

* adding secure flag for cookies that starts with __Secure (#1730)

* releasing v1.29.4-beta.4 (#1731)

* updating path-to-regexp to 6.3.0 where the vulnerabilities are fixed (#1733)

* releasing v1.29.4-beta.5 (#1735)

* ⬆️ Bump micromatch from 4.0.7 to 4.0.8 (#1706)

Bumps [micromatch](https://github.com/micromatch/micromatch) from 4.0.7 to 4.0.8.
- [Release notes](https://github.com/micromatch/micromatch/releases)
- [Changelog](https://github.com/micromatch/micromatch/blob/master/CHANGELOG.md)
- [Commits](micromatch/micromatch@4.0.7...4.0.8)

---
updated-dependencies:
- dependency-name: micromatch
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump @babel/preset-env from 7.19.4 to 7.25.4 (#1704)

Bumps [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) from 7.19.4 to 7.25.4.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.25.4/packages/babel-preset-env)

---
updated-dependencies:
- dependency-name: "@babel/preset-env"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump axios from 1.6.7 to 1.7.7 (#1737)

Bumps [axios](https://github.com/axios/axios) from 1.6.7 to 1.7.7.
- [Release notes](https://github.com/axios/axios/releases)
- [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md)
- [Commits](axios/axios@v1.6.7...v1.7.7)

---
updated-dependencies:
- dependency-name: axios
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump body-parser from 1.19.0 to 1.20.3 (#1720)

Bumps [body-parser](https://github.com/expressjs/body-parser) from 1.19.0 to 1.20.3.
- [Release notes](https://github.com/expressjs/body-parser/releases)
- [Changelog](https://github.com/expressjs/body-parser/blob/master/HISTORY.md)
- [Commits](expressjs/body-parser@1.19.0...1.20.3)

---
updated-dependencies:
- dependency-name: body-parser
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump eslint-plugin-import from 2.29.1 to 2.30.0 (#1740)

Bumps [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) from 2.29.1 to 2.30.0.
- [Release notes](https://github.com/import-js/eslint-plugin-import/releases)
- [Changelog](https://github.com/import-js/eslint-plugin-import/blob/main/CHANGELOG.md)
- [Commits](import-js/eslint-plugin-import@v2.29.1...v2.30.0)

---
updated-dependencies:
- dependency-name: eslint-plugin-import
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️👷 Bump github/codeql-action from 3.25.10 to 3.26.8 (#1741)

Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.10 to 3.26.8.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@23acc5c...294a9d9)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: packages/core/package.json to reduce vulnerabilities (#1732)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-MICROMATCH-6838728

Co-authored-by: snyk-bot <snyk-bot@snyk.io>

* ⬆️ Bump tsd from 0.28.1 to 0.31.2 (#1738)

Bumps [tsd](https://github.com/tsdjs/tsd) from 0.28.1 to 0.31.2.
- [Release notes](https://github.com/tsdjs/tsd/releases)
- [Commits](tsdjs/tsd@v0.28.1...v0.31.2)

---
updated-dependencies:
- dependency-name: tsd
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Throttle Logs Endpoint from CLI (#1734)

* v1.29.4-beta.6 (#1742)

* Multi DOM Capture JS-utils

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: ninadbstack <60422475+ninadbstack@users.noreply.github.com>
Co-authored-by: Pradum Kumar <pradumraj360@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: AgnellusX1 <43168803+AgnellusX1@users.noreply.github.com>
Co-authored-by: snyk-bot <snyk-bot@snyk.io>
Co-authored-by: Amandeep singh <amandeep@browserstack.com>
chinmay-browserstack added a commit that referenced this pull request Sep 30, 2024
…in single asset discovery phase (#1728)

* [Defer Uploads v2] Added support for handling multiple root resource in single asset discovery phase

* update domSnapshot structure in config + reload page in case on multidom

* Fix test

* Added tests for widths endpoint + multiple root resources

* add multi dom case

* Added case for percyCSS with multiple root resource

* Remove extra code and improve coverage

* Fix coverage

* Coverage resolved

* Addressed comments

* Minor improvement

* Addressed comments

* wait for page navigation while force reloading

* Updated test to have different resource in different doms

* Add logging for test browser userAgent

* Fix tests

* Change userAgent condition for responsiveSnapshotCapture

* Added test endpoint for changing widths

* Added waitForResize script

* Add flag for changing config in test api

* Added tests for waitForResize

* Added defer uploads option in test config endpoint

* Responsive DOM SDK Utils (#1746)

* Fixed cookies parsing bug (#1727)

* v1.29.4-beta.3 (#1729)

* adding secure flag for cookies that starts with __Secure (#1730)

* releasing v1.29.4-beta.4 (#1731)

* updating path-to-regexp to 6.3.0 where the vulnerabilities are fixed (#1733)

* releasing v1.29.4-beta.5 (#1735)

* ⬆️ Bump micromatch from 4.0.7 to 4.0.8 (#1706)

Bumps [micromatch](https://github.com/micromatch/micromatch) from 4.0.7 to 4.0.8.
- [Release notes](https://github.com/micromatch/micromatch/releases)
- [Changelog](https://github.com/micromatch/micromatch/blob/master/CHANGELOG.md)
- [Commits](micromatch/micromatch@4.0.7...4.0.8)

---
updated-dependencies:
- dependency-name: micromatch
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump @babel/preset-env from 7.19.4 to 7.25.4 (#1704)

Bumps [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) from 7.19.4 to 7.25.4.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.25.4/packages/babel-preset-env)

---
updated-dependencies:
- dependency-name: "@babel/preset-env"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump axios from 1.6.7 to 1.7.7 (#1737)

Bumps [axios](https://github.com/axios/axios) from 1.6.7 to 1.7.7.
- [Release notes](https://github.com/axios/axios/releases)
- [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md)
- [Commits](axios/axios@v1.6.7...v1.7.7)

---
updated-dependencies:
- dependency-name: axios
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump body-parser from 1.19.0 to 1.20.3 (#1720)

Bumps [body-parser](https://github.com/expressjs/body-parser) from 1.19.0 to 1.20.3.
- [Release notes](https://github.com/expressjs/body-parser/releases)
- [Changelog](https://github.com/expressjs/body-parser/blob/master/HISTORY.md)
- [Commits](expressjs/body-parser@1.19.0...1.20.3)

---
updated-dependencies:
- dependency-name: body-parser
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️ Bump eslint-plugin-import from 2.29.1 to 2.30.0 (#1740)

Bumps [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) from 2.29.1 to 2.30.0.
- [Release notes](https://github.com/import-js/eslint-plugin-import/releases)
- [Changelog](https://github.com/import-js/eslint-plugin-import/blob/main/CHANGELOG.md)
- [Commits](import-js/eslint-plugin-import@v2.29.1...v2.30.0)

---
updated-dependencies:
- dependency-name: eslint-plugin-import
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* ⬆️👷 Bump github/codeql-action from 3.25.10 to 3.26.8 (#1741)

Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.25.10 to 3.26.8.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@23acc5c...294a9d9)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: packages/core/package.json to reduce vulnerabilities (#1732)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-MICROMATCH-6838728

Co-authored-by: snyk-bot <snyk-bot@snyk.io>

* ⬆️ Bump tsd from 0.28.1 to 0.31.2 (#1738)

Bumps [tsd](https://github.com/tsdjs/tsd) from 0.28.1 to 0.31.2.
- [Release notes](https://github.com/tsdjs/tsd/releases)
- [Commits](tsdjs/tsd@v0.28.1...v0.31.2)

---
updated-dependencies:
- dependency-name: tsd
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Throttle Logs Endpoint from CLI (#1734)

* v1.29.4-beta.6 (#1742)

* Multi DOM Capture JS-utils

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: ninadbstack <60422475+ninadbstack@users.noreply.github.com>
Co-authored-by: Pradum Kumar <pradumraj360@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: AgnellusX1 <43168803+AgnellusX1@users.noreply.github.com>
Co-authored-by: snyk-bot <snyk-bot@snyk.io>
Co-authored-by: Amandeep singh <amandeep@browserstack.com>

* Handle case when cookie is passed as object from sdk

* Remove unnecessary fields from cookies

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Amit Singh Sansoya <tusharamit@yahoo.com>
Co-authored-by: ninadbstack <60422475+ninadbstack@users.noreply.github.com>
Co-authored-by: Pradum Kumar <pradumraj360@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: AgnellusX1 <43168803+AgnellusX1@users.noreply.github.com>
Co-authored-by: snyk-bot <snyk-bot@snyk.io>
Co-authored-by: Amandeep singh <amandeep@browserstack.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants