bugGNU Octave - Bugs: bug #53922, setenv/unsetenv are case sensitive...

 
 

bug #53922: setenv/unsetenv are case sensitive on Windows while getenv is case insensitive

Submitter:  Ian McCallion <ianmcc>
Submitted:  Wed 16 May 2018 08:54:25 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 04 Oct 2018 05:16:26 PM UTC, comment #16: 

I pushed a patch here that fixes the issue for me and also allows setting, getting and un-setting environment variables with non-ASCII characters:
http://hg.savannah.gnu.org/hgweb/octave/rev/3f2282954600


>> setenv("test123", "aäu")
>> getenv("teST123")
ans = aäu
>> unsetenv("TEst123")
ans = 0
>> getenv("tEsT123")
ans =
>>


Markus Mützel <mmuetzel>
Group administrator
Fri 28 Sep 2018 11:56:52 PM UTC, comment #15: 

The "*env" functions also lack support for non-ASCII characters on Windows. Among other things, this leads to problems for users with non-ASCII user name.
Gnulib doesn't support the Windows Unicode API either. So we have to write our own wrappers anyway if we want to enable users with non-ASCII scripts to use Octave natively.

The attached patch switches to use the Windows Unicode API for setting, querying and un-setting environment variables. All of these are case-insensitive.

There was a bunch of FIXME notes that I just carried over. I don't know whether they all still apply. Particularly this one in lo-sysdep.cc (after the patch):

FIXME: make this do the right thing if we don't have a SMART_PUTENV.


Was this referring to the problems with Windows (only)?

See also this short exchange with Mike on the mailing list:
http://octave.1599824.n4.nabble.com/Code-organisation-td4689473.html

(file #45114)

Markus Mützel <mmuetzel>
Group administrator
Sat 30 Jun 2018 12:33:35 AM UTC, comment #14: 

Yes, the POSIX documentation instructs programmers to prefer 'setenv' to 'putenv'. However, if you look in the Gnulib source code, the 'putenv' wrapper function is the only one that references the Windows API function SetEnvironmentVariable. And I can't tell if that's a benefit or a liability.

The Gnulib docs and code don't make it clear which of these functions should be used together or what the expected behavior or compatibility is.

Mike Miller <mtmiller>
Group Member
Fri 29 Jun 2018 11:01:47 PM UTC, comment #13: 

putenv is described in Gnulib here: https://www.gnu.org/software/gnulib/manual/html_node/putenv.html#index-putenv.

If you then follow the link for the documentation of how putenv should behave you get to http://pubs.opengroup.org/onlinepubs/9699919799/functions/putenv.html.

In that documentation there is this line


The setenv() function is preferred over this function. One reason is that putenv() is optional and therefore less portable. Another is that using putenv() can slow down environment searches, as explained in the RATIONALE section for getenv


So maybe we shouldn't be switching over to putenv?

In any case, this is more complicated that I thought.  setenv/unsetenv only work within the context of Octave.  If you use


system ('get varname')


then you find that the variable still persists in the Windows/bash environment after unsetenv.  Using


setenv ('varname', '')


merely sets the variable to be empty, but it continues to exist.

This is not the way it behaves on Linux where the variable is truly removed from the environment.


Rik <rik5>
Group administrator
Fri 29 Jun 2018 10:34:17 PM UTC, comment #12: 

Yes, I confirm that unsetenv will clear an environment variable provided that the case of the variable matches the original call to setenv.  In particular, I tried this


setenv ('Ab', 'xyz');
unsetenv ('AB')  # fails
unsetenv ('Ab')  # works


But, to your specific question, it works for setenv ('AB', ...) followed by unsetenv ('AB').

Rik <rik5>
Group administrator
Fri 29 Jun 2018 10:10:29 PM UTC, comment #11: 

Rik - so you can confirm that 'unsetenv' does currently work when using an all-caps environment variable name?

The Gnulib function wrapper 'putenv' already supports unsetting environment variables by passing in an empty second argument. That's what I would like to see us use if that is the appropriate solution, needs testing.

The liboctave class octave::env has 'getenv' and 'putenv' functions, I think it should also have an 'unsetenv' function to absract this, whatever the right solution is.

Mike Miller <mtmiller>
Group Member
Fri 29 Jun 2018 10:06:19 PM UTC, comment #10: 

I can confirm that unsetenv is case sensitive which leads to some counter-inuitive results.

Would it be that harmful to have an #ifdef for Windows and replace calls to unsetenv (XXX) with setenv (XXX, "")?

Rik <rik5>
Group administrator
Fri 29 Jun 2018 08:51:02 PM UTC, comment #9: 

And there is absolutely no problem with implementing Unix environment variable semantics in a program on Windows, as long as any case-insensitive name conflicts are resolved when a new process is started by the Unix program. I refer again to bash.

Mike Miller <mtmiller>
Group Member
Fri 29 Jun 2018 08:37:48 PM UTC, comment #8: 

As I mentioned earlier in comment #4, we need someone to test the Gnulib function wrappers for 'putenv', 'setenv', and 'unsetenv', to see how they behave and interact with Windows environment variables, and determine which functions we should be using in Octave.

Octave currently uses both 'putenv' and 'unsetenv', but it's not clear to me that they were intended to be used together. As consumers of Gnulib we should work with those functions and feed back any changes or bugs we find so that Gnulib can be improved for everyone.

Can one of you confirm that Octave does correctly unset environment variables using 'unsetenv' when everything is upper case? I only have access to Wine, and under Wine 'unsetenv' doesn't work at all for me.

This is what I get with the 4.4.0 release under Wine:


>> getenv ('FOO')
ans =
>> setenv ('FOO', 'something');
>> getenv ('FOO')
ans = something
>> unsetenv ('FOO');
>> getenv ('FOO')
ans = something


So does 'unsetenv' not work at all, or is it only when the environment variable has lowercase letters in its name?

Mike Miller <mtmiller>
Group Member
Fri 29 Jun 2018 08:04:09 AM UTC, comment #7: 

I agree there is unlikely to be a real-world use case for environment variables that only differ in case. So what Windows does, which is store the name with its case for decorative reasons but compare ignoring case, is not unreasonable. It is just not the same as what Unix does.

Octave programs that use environment variables cannot be shielded from this unix-windows difference because implementing the windows rule on unix would prevent access to variables that differ only in case, and implementing the unix rule in windows would require storing "A" and "a" independently in the windows environment variable store which is impossible. (I am ignoring the "solution" in which on windows Octave environment variables are disconnected from windows environment variables)

Hence Octave can only document that there is the difference between windows and Octave.

Of course, resolving the unix-windows difference by documentation does not alter the fact that there is a bug in unsetenv as the following octave script attempts to show:

% demonstration that
(1) setenv uses windows to store environment variables
(2) setenv passes the name in its origianl case to windows,
(3) windows stores the name with its original case but does not allow
    different values for names that are the same apart from case

>> setenv('ZU','123');system('set ZU');
ZU=123
>> setenv('ZU','123');system('set zu');
ZU=123
>> setenv('zl','123');system('set ZL');
zl=123
>> setenv('zl','123');system('set zl');
zl=123
>> setenv('zl','123');system('set zl');

>> system('set z');
zl=123
ZU=123

% demonstration that
% (5) getenv is case-insensitive, the supplied name and the variable name
%     can have different case

>> setenv('ll','123');getenv('ll')
ans = 123

>> setenv('UU','123');getenv('UU')
ans = 123

>> setenv('UL','123');getenv('ul')
ans = 123

>> setenv('lu','123');getenv('LU')
ans = 123

% demonstration that
% (6) unsetenv is case-sensitive on windows

>> setenv('xll','123');unsetenv('xll'); getenv('xll')
ans =

>> setenv('XUU','123');unsetenv('XUU'); getenv('xuu')
ans =

>> setenv('xlu','123');unsetenv('XLU'); getenv('xlu')
ans = 123

>> setenv('XUL','123');unsetenv('xul'); getenv('xul')
ans = 123


The fix is obviously complicated by the fact that Octave passes via a unix emulation layer on its way to windows. I suspect that layer is where the fix would ideally be applied.

Alterntively Octave unsetenv (on windows) could be implemented  by setetenv('name',''). Job done AFAICS.

Ian McCallion <ianmcc>
Fri 29 Jun 2018 06:46:31 AM UTC, comment #6: 

AFAICS on my Windows 7 box, all Windows environment variables are copied verbatim (preserving case) into an MSYS2 bash environment.

Just curious: I'm wondering what a real-world use case would be for two or more environment variables that only differ in case. In my programs I try to avoid such potential confusion at all cost.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 17 May 2018 11:07:14 PM UTC, comment #5: 

I can see the appeal of supporting case-sensitive environment variables, but it would break some existing applications (including mine by the way!)

Are bash environment variables connected to windows environment variables at all?

Ian McCallion <ianmcc>
Thu 17 May 2018 06:00:47 PM UTC, comment #4: 

Octave uses Gnulib for the 'putenv' and 'unsetenv' functions. The 'putenv' function also allows unsetting an environment variable, but Octave does not currently use that option. And Gnulib also offers a 'setenv' function that Octave does not use, maybe it should use that instead of 'putenv'.

I think someone who can test all of the various combinations of these Gnulib functions in Windows with differently cased environment variables should do so, and Octave will use whatever the results indicate to use. It's not clear to me which functions are meant to be used with which other functions.

IMHO, it would be best if Octave supported case sensitive environment variables even on Windows, just as bash running on Windows appears to.

Mike Miller <mtmiller>
Group Member
Thu 17 May 2018 10:39:23 AM UTC, comment #3: 

unsetenv does work sometimes, as this octave command sequence shows:

>> setenv('aa','1')
>> getenv('aa')
ans = 1
>> getenv('AA')
ans = 1
>> unsetenv('aa')
ans = 0
>> getenv('aa')
ans =


However weirdly, in spite of all the environment getting and setting operations being case-insensitive, windows still saves the environment variable with its name in its original case as its value is changed, as this windows commandline sequence shows:


c:\temp3>set aa=1

c:\temp3>set AA=2

c:\temp3>set aa
aa=2

c:\temp3>set AA
aa=2


Hence I reckon unsetenv merely needs to do a case-insensitive comparison (on Windows only of course) when searching for the environment variable to unset.

Ian McCallion <ianmcc>
Wed 16 May 2018 08:10:37 PM UTC, comment #2: 

Ok, now I see that Octave uses Gnulib's 'putenv' function, which calls the Winows function SetEnvironmentVariable, which is probably where the case insensitivity comes in. But Gnulib's 'unsetenv' function iterates over the 'environ' global variable directly, and doesn't seem to correspond to anything set with SetEnvironmentVariable.

It looks to me like 'unsetenv' simply does not work at all with 'putenv', even ignoring case sensitivity. The 'unsetenv' function simply does not work on Windows right now.

Mike Miller <mtmiller>
Group Member
Wed 16 May 2018 06:19:14 PM UTC, comment #1: 

Yes, environment variable names can include both uppercase and lowercase ASCII letters, and Octave's getenv, setenv, putenv, and unsetenv functions all support that.

I think the bug is that the environment variable should not be cleared in both cases.

The Gnulib setenv and unsetenv replacement functions should use case-sensitive string matching to add, replace, and remove environment variables. If that's not true, then that's a bug.

Mike Miller <mtmiller>
Group Member
Wed 16 May 2018 08:54:25 AM UTC, original submission:  

Environment variable names on Windows are case insensitive (I don't know about on unix). The setenv and getenv functions implement this correctly but unsetenv does not.

Invoking the function below with no parameters causes it to use setenv to clear the environment variable, which is as it should be. Invoking it supplying the parameter "unset" causes it to use unset and the variables are not unset.

function []= unsetbug(x)
    setenv('E1','E1value');
    setenv('e2','e2value');
    if exist('x','var') && strcmpi(x, 'unset')
        unsetenv('E2');
        unsetenv('e1');
    else
        setenv('E2','');
        setenv('e1','');
    end
    v1=getenv('E1')
    v2=getenv('e2')
end


Ian McCallion <ianmcc>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #45114:  bug53922_getenv_unicode.patch added by mmuetzel (7KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by ianmcc (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-10-19 mtmiller Carbon-CopyRemoved 80942 -
    2018-10-12 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2018-10-04 mmuetzel StatusPatch Submitted Ready For Test
    2018-09-28 mmuetzel Attached File- Added bug53922_getenv_unicode.patch, #45114
        StatusConfirmed Patch Submitted
        Release4.4.0 dev
    2018-06-29 rik5 Summaryunsetenv is case sensitive on Windows setenv/unsetenv are case sensitive on Windows while getenv is case insensitive
    2018-06-29 rik5 Summaryunsetenv does not unset environment variables on Windows unsetenv is case sensitive on Windows
    2018-05-16 mtmiller StatusNeed Info Confirmed
        Summaryunset case sensitivity unsetenv does not unset environment variables on Windows
    2018-05-16 mtmiller StatusNone Need Info
        Operating SystemMac OS Microsoft Windows

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code