moovweb/gvm

`__gvm_munge_path` incorrectly reformats path incorrectly.

ItamarGronich opened this issue · 4 comments

Details

gvm shell integration messes up $PATH if one of the paths has a space in it.
It happens in __gvm_munge_path.

For example this path (notice /foo bar/bin):

> echo $PATH
/usr/local/bin:/usr/bin:/bin:/foo bar/bin

will result in this path after bootstrapping GVM:

> echo $PATH
/usr/local/bin:/usr/bin:/bin:/foo:bar/bin

System details

device: Macbook Pro 14inch 2023
CPU: M2 pro
Memory: 32GB
OS: Sonoma 14.4.1

I encountered the same issue and resolved it by modifying the __gvm_munge_path() function in ~/.gvm/scripts/function/munge_path. For reference, I'm using gvm v1.0.22.

The issue arises in a Bash script when adding elements to an array. If an element contains spaces, it gets split into multiple elements based on those spaces. To prevent this, you should enclose the element in double quotes when adding it to the array. This way, the element, even if it contains spaces, will be added as a single item.

Here are the changes I made to the __gvm_munge_path() function:

# ~/.gvm/scripts/function/munge_path

# line 51
IFS=': ' path_in_ary=( "$(printf "%s" "${path_in}")" ) IFS="$defaultIFS"

# line 60
gvm_ary+=("${_path}")

# line 64
rvm_ary+=("${_path}")

# line 66
general_ary+=("${_path}")

# line 72
path_out_ary=( "${rvm_ary[@]}" "${gvm_ary[@]}" "${general_ary[@]}" )

# line 89
path_out_ary=( "${_dedupe_path_out_ary[@]}" )

When adding a variable to an array in a bash script, you should wrap the variable in double quotes like this:
<variable>=("<variable_to_add_to_array>").
This ensures that strings with spaces are correctly treated as a single array element.

Here is the final revised ~/.gvm/scripts/function/munge_path file:

#!/usr/bin/env bash

. "$GVM_ROOT/scripts/function/_shell_compat" || return 1

# __gvm_munge_path()
# /*!
# @abstract Generate a new PATH with gvm and rvm elements in the correct order
# @discussion
# Scans path and Removes any existing elements that refer to the .gvm directory,
#   extracts and preserves elements that refer to the .rvm directory, preserves
#   all other elements. Builds a new PATH in this order:
#     * .rvm elements
#     * .gvm elements
#     * all other entries
#
#  The purpose of mungeing is to generate a new path dynamically so that user
#  changes to a PATH can be incorporated immediately.
# @param path [optional] Path to munge, defaults to $PATH
# @param dedupe [optional] Remove duplicates from the resulting output (true) or
#   leave them in place (false), defaults to true
# @return Returns a string containing munged path (status 0) or an empty string
#   (status 1) on failure.
# */
__gvm_munge_path() {
    if [[ -n $ZSH_VERSION ]]; then
        setopt KSH_ARRAYS
        setopt BASH_REMATCH
    fi
    local path_in="${1:-$PATH}"
    local path_in_ary; path_in_ary=()
    local path_out=""
    local path_out_ary; path_out_ary=()
    local path_dedupe_flag="${2:-true}"
    local rvm_regex='([^:\\\n\\\0]+\.rvm{1}[^:\\\n\\\0]+)'
    local rvm_ary; rvm_ary=()
    local gvm_regex='([^:\\\n\\\0]+\.gvm{1}[^:\\\n\\\0]+)'
    local gvm_ary; gvm_ary=()
    local general_ary; general_ary=()
    local defaultIFS="$IFS"
    local IFS="$defaultIFS"

    if [[ -z "${path_in}" ]]; then
        if [[ -n $ZSH_VERSION ]]; then
            unsetopt KSH_ARRAYS
            unsetopt BASH_REMATCH
        fi
        echo ""
        return 1
    fi

    IFS=': ' path_in_ary=( "$(printf "%s" "${path_in}")" ) IFS="$defaultIFS"

    # extract path elements
    local _path
    for _path in "${path_in_ary[@]}"
    do
        if [[ "${_path}" =~ $gvm_regex ]]
        then
            # echo "matched (gvm): ${_path}"
            gvm_ary+=("${_path}")
        elif [[ "${_path}" =~ $rvm_regex ]]
        then
            # echo "matched (rvm): ${_path}"
            rvm_ary+=("${_path}")
        else
            general_ary+=("${_path}")
        fi
    done
    unset _path

    # assemble path array
    path_out_ary=( "${rvm_ary[@]}" "${gvm_ary[@]}" "${general_ary[@]}" )

    # deduplicate path_out_ary if requested, do this just to be helpful :)
    if [[ "${path_dedupe_flag}" == true ]]
    then
        local _dedupe_path_out_ary; _dedupe_path_out_ary=()
        for (( i=0; i<${#path_out_ary[@]}; i++ ))
        do
            local __element="${path_out_ary[i]}"
            [[ -n "${__element}" ]] && _dedupe_path_out_ary+=( "${__element}" )
            # reset duplicates to an empty string so we can ignore them
            for (( j=${#path_out_ary[@]}-1; j>i; j-- ))
            do
                [[ "${__element}" == "${path_out_ary[j]}" ]] && path_out_ary[j]=""
            done
            unset __element
        done
        path_out_ary=( "${_dedupe_path_out_ary[@]}" )
        unset _dedupe_path_out_ary
    fi

    # reconstruct new path
    IFS=":" path_out="${path_out_ary[*]}" IFS="$defaultIFS"

    printf "%s" "${path_out}"

    if [[ -z "${path_out}" ]]; then
        if [[ -n $ZSH_VERSION ]]; then
            unsetopt KSH_ARRAYS
            unsetopt BASH_REMATCH
        fi
        return 1
    fi

    if [[ -n $ZSH_VERSION ]]; then
        unsetopt KSH_ARRAYS
        unsetopt BASH_REMATCH
    fi
    return 0
}

I hope this helps anyone facing a similar issue.

I've resolved the related issue and submitted a PR(#488 (comment)).

It's work for me! thanks!