Skip to content

fix path escaping#975

Merged
zimbatm merged 1 commit intomasterfrom
fix-path-escaping
Jul 26, 2022
Merged

fix path escaping#975
zimbatm merged 1 commit intomasterfrom
fix-path-escaping

Conversation

@zimbatm
Copy link
Copy Markdown
Member

@zimbatm zimbatm commented Jul 26, 2022

If a path would include $, it was getting expanded during the
evaluation.

Reported by Fabian Thorand

If a path would include $, it was getting expanded during the
evaluation.

Reported by Fabian Thorand
@zimbatm zimbatm merged commit fb0ee1b into master Jul 26, 2022
@zimbatm zimbatm deleted the fix-path-escaping branch July 26, 2022 16:30
SingingTree added a commit to SingingTree/direnv that referenced this pull request Apr 8, 2023
direnv#975 had the unintended consequence
of causing the `RC::Load` call to sometimes fail when running under
git-bash (and some others Windows specific configs).

The `RC::Load` -> `cmd.Output()` calls end up interpreting
characters like \b or \t as escape sequences, which is problematic if
for paths like 'myProject\buckets\test'.

This patch addresses this by forcing the use of `/` as a separator, even
on Windows. This works fine in my testing in git-bash. This shouldn't
impact systems where `/` is already in use.

This also adds a test case that covers the behaviour being adjusted. In
order to run the bash tests on Windows, I had to disabled a number that
appear to not play nice -- but after hacking those out I was able to
verify the new test case.

Fixes:
- direnv#1020
- direnv#1079
zimbatm pushed a commit to SingingTree/direnv that referenced this pull request May 19, 2023
direnv#975 had the unintended consequence
of causing the `RC::Load` call to sometimes fail when running under
git-bash (and some others Windows specific configs).

The `RC::Load` -> `cmd.Output()` calls end up interpreting
characters like \b or \t as escape sequences, which is problematic if
for paths like 'myProject\buckets\test'.

This patch addresses this by forcing the use of `/` as a separator, even
on Windows. This works fine in my testing in git-bash. This shouldn't
impact systems where `/` is already in use.

This also adds a test case that covers the behaviour being adjusted. In
order to run the bash tests on Windows, I had to disabled a number that
appear to not play nice -- but after hacking those out I was able to
verify the new test case.

Fixes:
- direnv#1020
- direnv#1079
zimbatm pushed a commit that referenced this pull request May 20, 2023
#975 had the unintended consequence
of causing the `RC::Load` call to sometimes fail when running under
git-bash (and some others Windows specific configs).

The `RC::Load` -> `cmd.Output()` calls end up interpreting
characters like \b or \t as escape sequences, which is problematic if
for paths like 'myProject\buckets\test'.

This patch addresses this by forcing the use of `/` as a separator, even
on Windows. This works fine in my testing in git-bash. This shouldn't
impact systems where `/` is already in use.

This also adds a test case that covers the behaviour being adjusted. In
order to run the bash tests on Windows, I had to disabled a number that
appear to not play nice -- but after hacking those out I was able to
verify the new test case.

Fixes:
- #1020
- #1079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant