FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

process: move POSIX credential accessors into node_credentials.cc by joyeecheung · Pull Request #25066 · nodejs/node · GitHub

/ node Public

process: move POSIX credential accessors into node_credentials.cc - #25066

Closed
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:mov-cred
Closed

process: move POSIX credential accessors into node_credentials.cc#25066
joyeecheung wants to merge 1 commit into
nodejs:masterfrom
joyeecheung:mov-cred

Conversation

Copy link
Copy Markdown
Member

Expose the POSIX credential accessors through
internalBinding('credentials') instead of setting them on the
process or bootstrapper object from C++ directly. Also moves
SafeGetEnv from internalBinding('util') to
internalBinding('credentials') since it's closely related to
the credentials.

In the JS land, instead of wrapping the bindings then writing
to the process object directly in main_thread_only.js, return
the wrapped functions back to bootstrap/node.js where they get
written to the process object conditionally for clarity.

Refs: #24961

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Copy link
Copy Markdown
Collaborator

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Dec 15, 2018
Expose the POSIX credential accessors through
`internalBinding('credentials')` instead of setting them on the
process or bootstrapper object from C++ directly. Also moves
`SafeGetEnv` from `internalBinding('util')` to
`internalBinding('credentials')` since it's closely related to
the credentials.

In the JS land, instead of wrapping the bindings then writing
to the process object directly in main_thread_only.js, return
the wrapped functions back to bootstrap/node.js where they get
written to the process object conditionally for clarity.

Refs: nodejs#24961

Copy link
Copy Markdown
Member Author

joyeecheung added process Issues and PRs related to the process subsystem. author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Dec 18, 2018

Copy link
Copy Markdown
Member Author

CI is green. Landed in 321e296, thanks!

joyeecheung added a commit that referenced this pull request Dec 18, 2018
Expose the POSIX credential accessors through
`internalBinding('credentials')` instead of setting them on the
process or bootstrapper object from C++ directly. Also moves
`SafeGetEnv` from `internalBinding('util')` to
`internalBinding('credentials')` since it's closely related to
the credentials.

In the JS land, instead of wrapping the bindings then writing
to the process object directly in main_thread_only.js, return
the wrapped functions back to bootstrap/node.js where they get
written to the process object conditionally for clarity.

Refs: #24961

PR-URL: #25066
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Copy link
Copy Markdown
Contributor

this does not land cleanly on v11.x, should it be backported?

BridgeAR commented Jan 9, 2019

Copy link
Copy Markdown
Member

Ping @joyeecheung

addaleax commented Jan 14, 2019
edited
Loading

Copy link
Copy Markdown
Member

Applies cleanly now.

Edit: But once again, I had to adjust test/parallel/test-bootstrap-modules.js slightly.

addaleax pushed a commit that referenced this pull request Jan 14, 2019
Expose the POSIX credential accessors through
`internalBinding('credentials')` instead of setting them on the
process or bootstrapper object from C++ directly. Also moves
`SafeGetEnv` from `internalBinding('util')` to
`internalBinding('credentials')` since it's closely related to
the credentials.

In the JS land, instead of wrapping the bindings then writing
to the process object directly in main_thread_only.js, return
the wrapped functions back to bootstrap/node.js where they get
written to the process object conditionally for clarity.

Refs: #24961

PR-URL: #25066
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
Expose the POSIX credential accessors through
`internalBinding('credentials')` instead of setting them on the
process or bootstrapper object from C++ directly. Also moves
`SafeGetEnv` from `internalBinding('util')` to
`internalBinding('credentials')` since it's closely related to
the credentials.

In the JS land, instead of wrapping the bindings then writing
to the process object directly in main_thread_only.js, return
the wrapped functions back to bootstrap/node.js where they get
written to the process object conditionally for clarity.

Refs: nodejs#24961

PR-URL: nodejs#25066
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR mentioned this pull request Jan 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
Expose the POSIX credential accessors through
`internalBinding('credentials')` instead of setting them on the
process or bootstrapper object from C++ directly. Also moves
`SafeGetEnv` from `internalBinding('util')` to
`internalBinding('credentials')` since it's closely related to
the credentials.

In the JS land, instead of wrapping the bindings then writing
to the process object directly in main_thread_only.js, return
the wrapped functions back to bootstrap/node.js where they get
written to the process object conditionally for clarity.

Refs: nodejs#24961

PR-URL: nodejs#25066
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Jan 24, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. lib / src Issues and PRs related to general changes in the lib or src directory. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants


Back | FazBrowse Home | New Git URL