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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace trivial usage of expand_path with getenv #11918

Merged
merged 1 commit into from Jun 3, 2019

Conversation

Projects
None yet
5 participants
@busterb
Copy link
Member

commented May 31, 2019

expand_path is not implemented consistently across platforms and sessions, which leads to confusing behavior. In places where we have trivial single variable expansions, this changes modules and library code to just use getenv. This gets us closer to being able to kill it 馃拃.

We'll look at the rest individually to see if they can also be reimplemented in terms of getenv.

Verification

  • Verify that everything works just like before!
replace trivial usage of expand_path with getenv
expand_path is not implemented consistently across platforms and
sessions, which leads to confusing behavior. In places where we have trivial
single variable expansions, this changes modules and library code to just use
getenv.

We'll look at the rest individually to see if they can also be reimplemented in
terms of getenv.

@busterb busterb added the library label May 31, 2019

@busterb busterb requested a review from wvu-r7 May 31, 2019

@sempervictus

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

Does this still work when automatic sysinfo collection at session init is off?

@busterb

This comment has been minimized.

Copy link
Member Author

commented Jun 1, 2019

@sempervictus it should, since the getenv call does not require cached output from sysinfo to function. I believe it always collects data synchronously from the session.

@wvu-r7

wvu-r7 approved these changes Jun 2, 2019

Copy link
Contributor

left a comment

This looks okay to me.

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

commented Jun 2, 2019

FWIW, I've pointed the payload tests to this on the windows side and I'm not seeing problems.

@bwatters-r7 bwatters-r7 self-assigned this Jun 3, 2019

@bwatters-r7 bwatters-r7 merged commit 53557cc into rapid7:master Jun 3, 2019

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

bwatters-r7 added a commit that referenced this pull request Jun 3, 2019

Land #11918, replace trivial usage of expand_path with getenv
Merge branch 'land-11918' into upstream-master
@bwatters-r7

This comment has been minimized.

Copy link
Contributor

commented Jun 3, 2019

Release Notes

Expand_path methods will be replaced whenever possible to reduce the code occurrences of expand_path, which is inconsistent.

jmartin-r7 added a commit that referenced this pull request Jun 4, 2019

Land #11918, replace trivial usage of expand_path with getenv
Merge branch 'land-11918' into upstream-master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.