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

[cr93 followup] Fix for webtorrent crash. #9858

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

mkarolin
Copy link
Collaborator

Fixes brave/brave-browser#17652

This is an upstream bug introduced in this Chromium change:

https://source.chromium.org/chromium/chromium/src/+/301dd60948a8b3a628cf6bd3a0f9da0ab91743ef

commit 301dd60948a8b3a628cf6bd3a0f9da0ab91743ef
Author: Nicolas Ouellet-Payeur nicolaso@chromium.org
Date: Tue Jun 15 13:43:57 2021 +0000

[Extensions] Migrate sockets_tcp_server_api.cc to SocketApiFunction

The new SocketApiFunction class has a more simple API than
SocketAsyncApiFunction. Change extension functions in
sockets_tcp_server_api.cc so they extend SocketApiFunction.

Bug: 1200440

The bug is that the SocketsTcpServerGetInfoFunction::Work was changed to
get args into local variable params, but then the instance variable
params_ was left to be used in the rest of the menthod's code.

The fix has already been merged upstream but has not made it to Cr93
yet:

https://chromium.googlesource.com/chromium/src/+/427152d3d98fce04457af56b0c362c45eb1ec042

commit 427152d3d98fce04457af56b0c362c45eb1ec042
Author: Reilly Grant reillyg@chromium.org
Date: Tue Aug 24 22:29:18 2021 +0000

Fix parameter validation for chrome.tcpServer.getInfo()

In crrev.com/c/2961688 the implementation of this function was
simplified however an error was introduced where the parsed function
parameters were stored in a local variable but still accessed from an
instance variable.

This change removes the instance variable as it should have been in the
original change.

Bug: 1239520

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@mkarolin mkarolin requested a review from a team as a code owner August 25, 2021 20:45
@mkarolin mkarolin self-assigned this Aug 25, 2021
This is an upstream bug introduced in this Chromium change:

https://source.chromium.org/chromium/chromium/src/+/301dd60948a8b3a628cf6bd3a0f9da0ab91743ef

commit 301dd60948a8b3a628cf6bd3a0f9da0ab91743ef
Author: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Date:   Tue Jun 15 13:43:57 2021 +0000

    [Extensions] Migrate sockets_tcp_server_api.cc to SocketApiFunction

    The new SocketApiFunction class has a more simple API than
    SocketAsyncApiFunction. Change extension functions in
    sockets_tcp_server_api.cc so they extend SocketApiFunction.

    Bug: 1200440

The bug is that the SocketsTcpServerGetInfoFunction::Work was changed to
get args into local variable params, but then the instance variable
params_ was left to be used in the rest of the menthod's code.

The fix has already been merged upstream but has not made it to Cr93
yet:

https://chromium.googlesource.com/chromium/src/+/427152d3d98fce04457af56b0c362c45eb1ec042

commit 427152d3d98fce04457af56b0c362c45eb1ec042
Author: Reilly Grant <reillyg@chromium.org>
Date:   Tue Aug 24 22:29:18 2021 +0000

    Fix parameter validation for chrome.tcpServer.getInfo()

    In crrev.com/c/2961688 the implementation of this function was
    simplified however an error was introduced where the parsed function
    parameters were stored in a local variable but still accessed from an
    instance variable.

    This change removes the instance variable as it should have been in the
    original change.

    Bug: 1239520
@mkarolin mkarolin force-pushed the maxk-cr93fu-fix-webtorrent-crash branch from 8296cab to 9a54992 Compare August 25, 2021 22:09
@mkarolin mkarolin merged commit 14d12a9 into master Aug 26, 2021
@mkarolin mkarolin deleted the maxk-cr93fu-fix-webtorrent-crash branch August 26, 2021 01:54
@mkarolin mkarolin added this to the 1.30.x - Nightly milestone Aug 26, 2021
mkarolin added a commit that referenced this pull request Aug 26, 2021
[cr93 followup] Fix for webtorrent crash.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webTorrent crashing on both torrent & magnet links
2 participants