GetComponents should abort on undefined variables

Issue #236 closed
Frank Löffler created an issue


I just combined two thornlists and "lost" (forgot) the definition of $ARR in the process. GetComponents used this thornlist and started to checkout stuff, into the current directory. It should probably abort in the case of undefined (but used) variables, shouldn't it? Could this lead to problems?


Comments (7)

  1. Eric Seidel
    • changed status to open
    • removed comment

    Did it not even report a Perl error, a la 'use of uninitialized variable?' Regardless, you're right that we should be checking for this somehow while parsing the thornlist.

  2. Eric Seidel
    • removed comment

    Ok, I see what's going on here. Since $ARR is not defined, !GetComponents treats it as a legitimate directory name and attempts to check the thorns into Cactus/$ARR/xxx. Then the shell obviously decides that $ARR is not defined and gets rid of it, so I end up with Cactus/xxx. This presents an interesting problem because, technically, '$ARR' is a valid directory name, although I doubt that anyone actually names their folders like that.

    So I see two possible solutions:

    1. Force the $ sigil to indicate a variable and throw an error if it was not previously defined. This would be very clean and prevent issues of the type you saw; however, if someone happens to prefix some folders with a $, they would be out of luck...

    2. Allow the $ sigil to be used NOT referring to a variable. Any paths of this type would have to be explicitly single-quoted; this could probably be handled by !GetComponents itself though. I think I could just add some logic to automatically enclose any paths in single-quotes before executing a command, which should change nothing outside of odd cases like this.

    Since this would mean altering the behavior of !GetComponents, we should probably ask for comments on the mailing list as well.

    What do you think?

  3. Frank Löffler reporter
    • removed comment

    I don't think that anyone would choose directory names starting with '$', however, if possible I wouldn't make it impossible to use them. In addition, someone could actually try to use an environment variable. Could GetComponents do this: produce a warning/error if an undefined $VAR is detected _and_ no environment variable of that name is defined. If an env-variable is defined: let the shell deal with it. Otherwise error out because the use of a simple, unescaped '$' without a CRL definition or an environment variable has to be a mistake and would not work in any case. I would then make '$' escapeable by '\$', or single quotes.

  4. Eric Seidel
    • changed status to resolved
    • removed comment

    Sorry this one took a while... GetComponents will now send escaped variables to the shell. If they are undefined, it will error out immediately. For now, you have to use a backslash to escape variables, but I can work on adding single quotes too if you want.

  5. Log in to comment