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.

Sign up for GitHub

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

Jump to bottom

fix: treat form-data bodies as binary #20144

Merged
merged 2 commits into from
Feb 17, 2022
Merged

fix: treat form-data bodies as binary #20144

merged 2 commits into from
Feb 17, 2022

Conversation

mlauinger
Copy link
Contributor

@mlauinger mlauinger commented Feb 10, 2022 •
edited by flotwig
Loading

User facing changelog

Http-Requests with content-type multipart/form-data will no longer stringified when they are intercepted

Additional details

This change is necessary to guarantee that form-data requests which contain a binary file and a certain amount of plain text will not be serialized when they are intercepted. The serialization breaks binary files (like PDFs).

PR Tasks

  • Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • Has a PR for user-facing changes been opened in cypress-documentation?
  • Have API changes been updated in the type definitions?
  • [N/A] Have new configuration options been added to the cypress.schema.json?

@mlauinger mlauinger requested a review from a team as a code owner February 10, 2022 12:54
@mlauinger mlauinger requested review from jennifer-shehane and removed request for a team February 10, 2022 12:54
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 10, 2022

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Feb 10, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

jennifer-shehane
jennifer-shehane previously requested changes Feb 10, 2022
View reviewed changes
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

@mlauinger Can you please describe the issue this is solving in the PR description. Can you also sign our CLA? These are required to merge.

@jennifer-shehane jennifer-shehane dismissed their stale review February 11, 2022 14:26

Dismissing my previous review

@jennifer-shehane jennifer-shehane removed their request for review February 14, 2022 14:13
@BlueWinds BlueWinds assigned BlueWinds and unassigned BlueWinds Feb 14, 2022
BlueWinds
BlueWinds approved these changes Feb 14, 2022
View reviewed changes
Copy link
Contributor

@BlueWinds BlueWinds left a comment

Choose a reason for hiding this comment

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

This change looks correct to me. Even though "binary" encoding in nodejs is an alias for "latin1", and we're trying to move away from it and towards using null as an encoding to refer to binary files, here the "binary" string is being used as a flag ("should we call .toString() on this") rather than as an encoding for the buffer.

@flotwig flotwig merged commit 0d3e645 into cypress-io:develop Feb 17, 2022
@flotwig flotwig mentioned this pull request Feb 21, 2022
Merged
flotwig added a commit to cypress-io/cypress-example-recipes that referenced this pull request Feb 21, 2022
@flotwig
update form-spec for cypress-io/cypress#20144
149ad0e
tgriesser added a commit that referenced this pull request Feb 22, 2022
@tgriesser
Merge branch 'develop' into 10.0-release
47b1f38 
* develop: (35 commits)
  fix(deps): update dependency url-parse to v1.5.6 [security] (#20270)
  chore: fix cache keys to include PLATFORM (#20279)
  chore: fix server performance flake (#20271)
  test(system-tests): support docker-based tests against built binary (#20250)
  chore: fix system-test-firefox screenshots_spec flake (#20268)
  chore(deps): update dependency fs-extra to v9 🌟 (#19939)
  fix: Wait for child process to be ready (#19792)
  fix: treat form-data bodies as binary (#20144)
  test: replace cypress-test-example-repos coverage + remove bump (#20186)
  fix(driver): update wrapErr to ignore number and boolean values (#20172)
  release 9.5.0 [skip ci]
  chore: Update Chrome (stable) to 98.0.4758.102 (#20192)
  chore: enable volar.takeOverMode
  Add span names, merge develop
  fix: Update `.type(' ')` to not emit clicks when the keyup event has been prevented (#20156)
  test: remove redundant "other projects" CI jobs (#20133)
  chore(driver): move cy.focused and cy.root into their own files (#20054)
  Move sending root event to own script
  chore: release @cypress/vue-v3.1.1
  chore: release @cypress/react-v5.12.3
  ...
mschile added a commit that referenced this pull request Feb 23, 2022
@mschile
Squashed commit of the following:
96d86c7 
commit fc7149e
Merge: 0e942ed 0143e13
Author: Chris Breiding <[email protected]>
Date:   Wed Feb 23 10:28:34 2022 -0500

    Merge branch 'develop' into feature-multidomain

commit 0e942ed
Author: Chris Breiding <[email protected]>
Date:   Wed Feb 23 10:28:03 2022 -0500

    chore: Refactor multi-domain communication lifecycle (#20247)

commit 0143e13
Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Date:   Mon Feb 21 23:01:35 2022 +0000

    fix(deps): update dependency url-parse to v1.5.6 [security] (#20270)

    Co-authored-by: Renovate Bot <[email protected]>

commit 1fb16b0
Author: Zach Bloomquist <[email protected]>
Date:   Sun Feb 20 16:22:08 2022 -0500

    chore: fix cache keys to include PLATFORM (#20279)

    * chore: fix cache keys to include PLATFORM

    * build this branch

    * try using platform_key

commit 65ea8f7
Author: Ryan Manuel <[email protected]>
Date:   Fri Feb 18 17:57:34 2022 -0600

    chore: fix server performance flake (#20271)

commit ad2f4de
Author: Zach Bloomquist <[email protected]>
Date:   Fri Feb 18 18:37:22 2022 -0500

    test(system-tests): support docker-based tests against built binary (#20250)

    Co-authored-by: Ryan Manuel <[email protected]>

commit 75c8750
Author: Ryan Manuel <[email protected]>
Date:   Fri Feb 18 12:54:26 2022 -0600

    chore: fix system-test-firefox screenshots_spec flake (#20268)

commit 8d28261
Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Date:   Fri Feb 18 10:33:27 2022 -0600

    chore(deps): update dependency fs-extra to v9 🌟 (#19939)

    Co-authored-by: Renovate Bot <[email protected]>
    Co-authored-by: Emily Rohrbough <[email protected]>
    Co-authored-by: David Munechika <[email protected]>

commit ad50062
Author: Johannes Ewald <[email protected]>
Date:   Thu Feb 17 19:21:08 2022 +0100

    fix: Wait for child process to be ready (#19792)

    Co-authored-by: Zach Bloomquist <[email protected]>
    Co-authored-by: Zach Bloomquist <[email protected]>

commit 0d3e645
Author: Marco Lauinger <[email protected]>
Date:   Thu Feb 17 19:20:33 2022 +0100

    fix: treat form-data bodies as binary (#20144)

commit 42b0fce
Author: Zach Bloomquist <[email protected]>
Date:   Wed Feb 16 13:53:04 2022 -0500

    test: replace cypress-test-example-repos coverage + remove bump (#20186)

commit e55974c
Merge: f84bac5 ce956de
Author: Blue F <[email protected]>
Date:   Wed Feb 16 09:47:06 2022 -0800

    Merge pull request #20079 from cypress-io/issue-19403-perf-reporter-changes

    chore: Performance reporter changes

commit f84bac5
Author: Ali Kireçligöl <[email protected]>
Date:   Wed Feb 16 20:22:57 2022 +0300

    fix(driver): update wrapErr to ignore number and boolean values (#20172)

    Co-authored-by: Emily Rohrbough <[email protected]>

commit 1e6f51a
Author: BlueWinds <[email protected]>
Date:   Tue Feb 15 10:18:49 2022 -0800

    release 9.5.0 [skip ci]

commit 507b96f
Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Date:   Tue Feb 15 06:35:08 2022 -0600

    chore: Update Chrome (stable) to 98.0.4758.102 (#20192)

    Co-authored-by: cypress-bot[bot] <2f0651858c6e38e0+cypress-bot[bot]@users.noreply.github.com>

commit e8d7120
Merge: dff45ca bb26e98
Author: BlueWinds <[email protected]>
Date:   Mon Feb 14 14:15:27 2022 -0800

    Merge remote-tracking branch 'origin/master' into develop

commit dff45ca
Merge: a3f0d63 2bad703
Author: Blue F <[email protected]>
Date:   Mon Feb 14 12:59:52 2022 -0800

    Merge pull request #20142 from cypress-io/9b967e06f5-master-into-develop

    chore: merge master (9b967e0) into develop

commit 2bad703
Merge: 9b967e0 a3f0d63
Author: Blue F <[email protected]>
Date:   Mon Feb 14 12:26:14 2022 -0800

    Merge branch 'develop' into 9b967e0-master-into-develop

commit ce956de
Author: BlueWinds <[email protected]>
Date:   Mon Feb 14 08:21:15 2022 -0800

    Add span names, merge develop

commit ac1faf4
Merge: 961e764 2c88f0c
Author: BlueWinds <[email protected]>
Date:   Mon Feb 14 08:19:17 2022 -0800

    Merge remote-tracking branch 'origin/develop' into issue-19403-perf-reporter-changes

commit 961e764
Author: BlueWinds <[email protected]>
Date:   Thu Feb 10 14:21:44 2022 -0800

    Move sending root event to own script

commit bb26e98
Author: semantic-release-bot <[email protected]>
Date:   Thu Feb 10 15:16:39 2022 -0500

    chore: release @cypress/vue-v3.1.1

    [skip ci]

commit 6a96ca5
Author: semantic-release-bot <[email protected]>
Date:   Thu Feb 10 15:16:23 2022 -0500

    chore: release @cypress/react-v5.12.3

    [skip ci]

commit 9b967e0
Author: Lachlan Miller <[email protected]>
Date:   Thu Feb 10 16:26:20 2022 +1000

    fix: set correct default when using react-scripts plugin (#20141)

commit e709184
Merge: e0bf811 97e6c14
Author: Barthélémy Ledoux <[email protected]>
Date:   Wed Feb 9 20:03:05 2022 -0600

    Merge pull request #20132 from cypress-io/elevatebart/trigger-vue-release

commit 97e6c14
Author: ElevateBart <[email protected]>
Date:   Wed Feb 9 17:52:38 2022 -0600

    fix: create a dummy commit to trigger release

commit fa0b68a
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 14:50:04 2022 -0800

    Fix path

commit f7c46fc
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 14:35:00 2022 -0800

    Refactor async data into more convenient helper

commit 57decf4
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 13:21:57 2022 -0800

    Include honeycomb key so we can send root event

commit 3cf4a2b
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 12:55:50 2022 -0800

    Reduce event duplication

commit 9d433c7
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 12:50:24 2022 -0800

    Send root honeycomb event even if node_modules cache already exists

commit 705262f
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 10:48:17 2022 -0800

    Fix

commit b9c0d5e
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 10:37:52 2022 -0800

    fix indentation

commit f694bd2
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 10:33:15 2022 -0800

    Another attempt to wrap all events into one trace

commit 5e96b5c
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 09:52:17 2022 -0800

    Attepmt to associate all circleci jobs for a given build into one trace

commit 980b951
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 09:34:59 2022 -0800

    Tweak honeycomb events further

commit b8f1b4f
Merge: 0d834bc 52ed6ed
Author: BlueWinds <[email protected]>
Date:   Tue Feb 8 09:12:20 2022 -0800

    Merge remote-tracking branch 'origin/develop' into issue-19403-perf-reporter-changes

commit 0d834bc
Author: BlueWinds <[email protected]>
Date:   Mon Feb 7 12:25:35 2022 -0800

    Add data to all events sent, not just top level

commit ac5bfd3
Author: BlueWinds <[email protected]>
Date:   Mon Feb 7 11:37:55 2022 -0800

    Use honeycomb tracing

commit 304d8bd
Author: BlueWinds <[email protected]>
Date:   Mon Feb 7 10:22:04 2022 -0800

    Make performance-reporter for system tests use get-next-version script
flotwig added a commit that referenced this pull request Feb 23, 2022
@flotwig
Revert "fix: treat form-data bodies as binary (#20144)"
493f515 
This reverts commit 0d3e645.
@flotwig flotwig mentioned this pull request Feb 23, 2022
Merged
flotwig added a commit that referenced this pull request Feb 23, 2022
@flotwig
fix: revert "fix: treat form-data bodies as binary"
9402b78 
Revert "fix: treat form-data bodies as binary (#20144)"

This reverts commit 0d3e645.
@flotwig flotwig mentioned this pull request Feb 23, 2022
Open
@JoshAGordon
Copy link

We are currently awaiting for this to be fixed in one of my projects, is there any update as to when it is expected to be fixed?

@BlueWinds
Copy link
Contributor

No update, and not currently in our planned work; If you'd like to take a stab using this PR as a basis and fix the issues that caused a revert (see discussion in #20343), we'd welcome the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BlueWinds BlueWinds BlueWinds approved these changes

@flotwig flotwig flotwig approved these changes

@jennifer-shehane jennifer-shehane jennifer-shehane left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Form-Data not treated as binary when request is intercepted
6 participants
@mlauinger @CLAassistant @JoshAGordon @BlueWinds @flotwig @jennifer-shehane