Skip to content

Adds a fuzzer for LoadEnvironment - #34844

Closed
DavidKorczynski wants to merge 5 commits into
nodejs:masterfrom
DavidKorczynski:master
Closed

Adds a fuzzer for LoadEnvironment#34844
DavidKorczynski wants to merge 5 commits into
nodejs:masterfrom
DavidKorczynski:master

Conversation

@DavidKorczynski

@DavidKorczynskiDavidKorczynski commented Aug 19, 2020

Copy link
Copy Markdown
Contributor

Adds a new fuzzer to run under OSS-Fuzz. The main function that this fuzzer targets is node::LoadEnvironment.

The fuzzer is inspired by the cctest test case.

Cross-reference: #34761

Checklist

@nodejs-github-botnodejs-github-bot added the build Issues and PRs related to build files or the CI. label Aug 19, 2020
@addaleaxaddaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2020
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2020
bnoordhuis
bnoordhuis reviewed Aug 19, 2020
View reviewed changes
Comment threadtest/fuzzers/fuzz_env.cc Outdated
Comment threadtest/fuzzers/fuzz_env.cc Outdated
Comment threadtest/fuzzers/fuzz_env.cc Outdated
Comment threadtest/fuzzers/fuzz_env.cc
Comment threadtest/fuzzers/fuzz_env.cc Outdated
@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 21, 2020
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 21, 2020
@nodejs-github-bot

nodejs-github-bot commented Aug 21, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32878/

joyeecheung
joyeecheung approved these changes Aug 21, 2020
View reviewed changes
Comment threadtest/fuzzers/fuzz_env.cc
mhdawson
mhdawson approved these changes Sep 9, 2020
View reviewed changes

@mhdawsonmhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@DavidKorczynski

DavidKorczynski commented Sep 28, 2020

Copy link
Copy Markdown
ContributorAuthor

I was wondering if there is something in the PR I can help with in order to get this merged in?

@mhdawsonmhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 8, 2020
@mhdawson

mhdawson commented Oct 8, 2020

Copy link
Copy Markdown
Member

@DavidKorczynski just looks like somebody needs to land it. Have requested a new CI and will try to remember to do it tomorrow.

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 8, 2020
@nodejs-github-bot

nodejs-github-bot commented Oct 8, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/33481/

@mhdawson

mhdawson commented Oct 9, 2020

Copy link
Copy Markdown
Member

There was a failure in CI run but looks like an issue that has been reported before: #34730

@DavidKorczynski

DavidKorczynski commented Oct 9, 2020

Copy link
Copy Markdown
ContributorAuthor

The code in this PR is self-contained in that it doesn't influence other aspects of NodeJS except for the fuzzer itself. And the fuzzer is not run in automated tests but only by way of https://github.com/google/oss-fuzz

@mhdawson

mhdawson commented Oct 9, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Link to rebuild on OSX: https://ci.nodejs.org/job/node-test-pull-request/33493/

@nodejs-github-bot

nodejs-github-bot commented Oct 9, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/33493/

@mhdawson

mhdawson commented Oct 9, 2020

Copy link
Copy Markdown
Member

New CI was good. Landing

@DavidKorczynski

DavidKorczynski commented Oct 9, 2020

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot @mhdawson

mhdawson pushed a commit that referenced this pull request Oct 9, 2020
@DavidKorczynski@mhdawson
build: fuzzer that targets node::LoadEnvironment()
de7a1ab
Refs: #34761
Refs: #33724
PR-URL: #34844
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@mhdawson

mhdawson commented Oct 9, 2020

Copy link
Copy Markdown
Member

Landed as de7a1ab

@mhdawsonmhdawson closed this Oct 9, 2020
BethGriggs pushed a commit that referenced this pull request Oct 14, 2020
@DavidKorczynski@BethGriggs
build: fuzzer that targets node::LoadEnvironment()
9c74d45
Refs: #34761
Refs: #33724
PR-URL: #34844
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@MylesBorinsMylesBorins mentioned this pull request Oct 14, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Nov 3, 2020
@DavidKorczynski@MylesBorins
build: fuzzer that targets node::LoadEnvironment()
2767dce
Refs: #34761
Refs: #33724
PR-URL: #34844
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@MylesBorinsMylesBorins mentioned this pull request Nov 3, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Nov 16, 2020
@DavidKorczynski@MylesBorins
build: fuzzer that targets node::LoadEnvironment()
f3a0457
Refs: #34761
Refs: #33724
PR-URL: #34844
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@DavidKorczynski@joesepi
build: fuzzer that targets node::LoadEnvironment()
add2f26
Refs: nodejs#34761
Refs: nodejs#33724
PR-URL: nodejs#34844
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuisbnoordhuisbnoordhuis approved these changes
@jasnelljasnelljasnell approved these changes
@addaleaxaddaleaxaddaleax approved these changes
@joyeecheungjoyeecheungjoyeecheung approved these changes
+1 more reviewer
@mhdawsonmhdawsonmhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

buildIssues and PRs related to build files or the CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@DavidKorczynski@nodejs-github-bot@mhdawson@bnoordhuis@jasnell@addaleax@joyeecheung@richardlau