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

wasi: use missing validator #39070

Merged
merged 1 commit into from Jun 26, 2021
Merged

wasi: use missing validator #39070

merged 1 commit into from Jun 26, 2021

Conversation

@VoltrexMaster
Copy link
Contributor

@VoltrexMaster VoltrexMaster commented Jun 18, 2021

The wasi lib module's initialize() and start() methods are missing validators.

@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 18, 2021

I think you could add a validateUndefined function in validators, and maybe clean that code a bit more:

    validateUndefined(_start, 'start');
    if (_initialize !== undefined) {
      validateFunction(_initialize, 'instance.exports._initialize');
      _initialize();
    }
VoltrexMaster added a commit to VoltrexMaster/node that referenced this pull request Jun 18, 2021
lib/wasi.js Outdated Show resolved Hide resolved
@aduh95
aduh95 approved these changes Jun 18, 2021
@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 18, 2021

test/wasi/test-wasi-initialize-validation is failing.

@VoltrexMaster
Copy link
Contributor Author

@VoltrexMaster VoltrexMaster commented Jun 18, 2021

test/wasi/test-wasi-initialize-validation is failing.

Should we change the expected error message in that test?

@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 18, 2021

Yes please

@lpinca
lpinca approved these changes Jun 18, 2021
@github-actions
Copy link

@github-actions github-actions bot commented Jun 26, 2021

Commit Queue failed
- Loading data for nodejs/node/pull/39070
✔  Done loading data for nodejs/node/pull/39070
----------------------------------- PR info ------------------------------------
Title      wasi: use missing validator (#39070)
Author     Voltrex  (@VoltrexMaster)
Branch     VoltrexMaster:patch-3 -> nodejs:master
Labels     author ready, wasi
Commits    14
 - wasi: use missing validator
 - validators: add validateUndefined
 - fixup! wasi: use missing validator
 - fixup! wasi: add missing validator
 - fixup! wasi: use missing validator
 - fixup! wasi: use missing validator
 - tests: fix wasi initialize error message
 - tests: fix wasi start error message
 - fixup! tests: fix wasi initialize error message
 - fixup! tests: fix wasi start error message
 - fixup! tests: fix wasi initialize error message
 - fixup! tests: fix wasi start error message
 - fixup! tests: fix wasi initialize error message
 - fixup! tests: fix wasi start error message
Committers 1
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/39070
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
Reviewed-By: Darshan Sen 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/39070
Reviewed-By: Antoine du Hamel 
Reviewed-By: Luigi Pinca 
Reviewed-By: Darshan Sen 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 18 Jun 2021 02:09:54 GMT
   ✔  Approvals: 3
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/39070#pullrequestreview-687457964
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/39070#pullrequestreview-687699063
   ✔  - Darshan Sen (@RaisinTen): https://github.com/nodejs/node/pull/39070#pullrequestreview-687825693
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2021-06-20T20:56:06Z: https://ci.nodejs.org/job/node-test-pull-request/38708/
- Querying data for job/node-test-pull-request/38708/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/master up to date...
From https://github.com/nodejs/node
 * branch                  master     -> FETCH_HEAD
✔  origin/master is now up-to-date
- Downloading patch for 39070
From https://github.com/nodejs/node
 * branch                  refs/pull/39070/merge -> FETCH_HEAD
✔  Fetched commits as 19b80cc4ece0..c0600406d792
--------------------------------------------------------------------------------
[master b5e338beb5] wasi: use missing validator
 Author: Voltrex 
 Date: Fri Jun 18 06:39:40 2021 +0430
 1 file changed, 3 insertions(+), 3 deletions(-)
[master c978dbc81d] validators: add validateUndefined
 Author: Voltrex 
 Date: Fri Jun 18 17:25:49 2021 +0430
 1 file changed, 6 insertions(+)
[master 3ae9317301] fixup! wasi: use missing validator
 Author: Voltrex 
 Date: Fri Jun 18 17:32:02 2021 +0430
 1 file changed, 3 insertions(+), 4 deletions(-)
[master 578c58920d] fixup! wasi: add missing validator
 Author: Voltrex 
 Date: Fri Jun 18 17:36:41 2021 +0430
 1 file changed, 2 insertions(+), 6 deletions(-)
[master 145a629399] fixup! wasi: use missing validator
 Author: Voltrex 
 Date: Fri Jun 18 18:17:36 2021 +0430
 1 file changed, 2 insertions(+), 3 deletions(-)
[master f730ee6b05] fixup! wasi: use missing validator
 Author: Voltrex 
 Date: Fri Jun 18 19:47:43 2021 +0430
 1 file changed, 2 insertions(+), 5 deletions(-)
[master 009a0a1b7a] tests: fix wasi initialize error message
 Author: Voltrex 
 Date: Fri Jun 18 19:55:34 2021 +0430
 1 file changed, 2 insertions(+), 1 deletion(-)
[master ca7884a6fa] tests: fix wasi start error message
 Author: Voltrex 
 Date: Fri Jun 18 19:58:28 2021 +0430
 1 file changed, 2 insertions(+), 1 deletion(-)
[master b6a22fa381] fixup! tests: fix wasi initialize error message
 Author: Voltrex 
 Date: Fri Jun 18 20:01:05 2021 +0430
 1 file changed, 2 insertions(+), 2 deletions(-)
[master 1b2c80da06] fixup! tests: fix wasi start error message
 Author: Voltrex 
 Date: Fri Jun 18 20:01:55 2021 +0430
 1 file changed, 2 insertions(+), 2 deletions(-)
[master 43c11e93dd] fixup! tests: fix wasi initialize error message
 Author: Voltrex 
 Date: Fri Jun 18 20:08:20 2021 +0430
 1 file changed, 2 insertions(+), 2 deletions(-)
[master 3525400b1e] fixup! tests: fix wasi start error message
 Author: Voltrex 
 Date: Fri Jun 18 20:09:26 2021 +0430
 1 file changed, 2 insertions(+), 2 deletions(-)
[master 0273a95a89] fixup! tests: fix wasi initialize error message
 Author: Voltrex 
 Date: Fri Jun 18 21:47:25 2021 +0430
 1 file changed, 1 insertion(+), 1 deletion(-)
[master 8af5f7ed5b] fixup! tests: fix wasi start error message
 Author: Voltrex 
 Date: Fri Jun 18 21:48:27 2021 +0430
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 14 commits in the PR. Attempting autorebase.
Rebasing (2/19)
Rebasing (3/19)
error: could not apply 145a629399... fixup! wasi: use missing validator?
Resolve all conflicts manually, mark them as resolved with
"git add/rm ", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 145a629399... fixup! wasi: use missing validator
Auto-merging lib/wasi.js
CONFLICT (content): Merge conflict in lib/wasi.js
Couldn't rebase 14 commits in the PR automatically
Please run the following commands to complete landing

$ git rebase origin/master -i -x "git node land --amend" --autosquash
$ git node land --continue

https://github.com/nodejs/node/actions/runs/974326765

The `wasi` lib module's `initialize()` method is missing a validator.

PR-URL: #39070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@aduh95 aduh95 force-pushed the VoltrexMaster:patch-3 branch from c060040 to 46a7e61 Jun 26, 2021
@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 26, 2021

Landed in 46a7e61

@aduh95 aduh95 merged commit 46a7e61 into nodejs:master Jun 26, 2021
16 of 17 checks passed
16 of 17 checks passed
@github-actions
test-macOS test-macOS
Details
@github-actions
build-tarball
Details
@github-actions
lint-commit-message
Details
@github-actions
build-windows
Details
@github-actions
coverage-linux
Details
@github-actions
coverage-windows
Details
@github-actions
lint-addon-docs
Details
@github-actions
build-docs
Details
@github-actions
test-asan
Details
@github-actions
test-linux
Details
@github-actions
lint-cpp
Details
@github-actions
lint-md
Details
@github-actions
lint-js
Details
@github-actions
lint-py
Details
@github-actions
lint-sh
Details
@github-actions
lint-codeowners
Details
@github-actions
lint-pr-url
Details
@VoltrexMaster VoltrexMaster deleted the VoltrexMaster:patch-3 branch Jun 26, 2021
targos added a commit that referenced this pull request Jul 11, 2021
The `wasi` lib module's `initialize()` method is missing a validator.

PR-URL: #39070
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants