Tcl Source Code

View Ticket
Login
Ticket UUID: 3fc3287497a7e281b96661f883c20e0e74a0fca7
Title: TclGetProcessGlobalValue encodes information twice on Windows
Type: Bug Created on: 2024-05-22 16:29:42
Submitter: ralfixx Assigned to: jan.nijtmans
Subsystem: 44. UTF-8 Strings Severity: Minor
Priority: 5 Medium Last modified: 2024-05-23 15:39:13
Status: Closed Closed by: jan.nijtmans
Resolution: Fixed Closed on: 2024-05-23 15:39:13
Version: 8.6.14
Description:
If the hostname on Windows contains an Umlaut, [info hostname] will return a different value when invoked for the second time:

    % tclsh8.6
    % info hostname
    foo-läptop
    % info hostname
    foo-läpt

BUG: The second time, the hostname is encoded another time.

Looking at the code for [info hostname] on Windows, it seems to me:

- information is stored in global variable
    static ProcessGlobalValue hostName =
    	{0, 0, NULL, NULL, InitializeHostName, NULL, NULL};
  (win/tclWinSock.c)
  
- the first time the information is retrieved by
  TclGetProcessGlobalValue(), it is initialized by a
  call to InitializeHostName(), which sets the encoding of the
  information to utf-8 after retrieving it from windows and converting
  it to utf-8
        *encodingPtr = Tcl_GetEncoding(NULL, "utf-8");
  (win/tclWinSock.c)
  
- the next time the information is retrieved by
  TclGetProcessGlobalValue(), it sees that the encoding is non-NULL and
  compares it to the system encoding, which on Windows is not utf-8, but
  (in my case) cp1252.  So the information is converted again.

  TclGetProcessGlobalValue()
    ...
    if (pgvPtr->encoding) {
	Tcl_Encoding current = Tcl_GetEncoding(NULL, NULL);

	if (pgvPtr->encoding != current) {

This is due to a wrong first argument to Tcl_UtfToExternalDString() 
in TclGetProcessGlobalValue() in generic/tclUtil.c:

#4287	    Tcl_UtfToExternalDString(pgvPtr->encoding, pgvPtr->value,
		    pgvPtr->numBytes, &native);

The first argument is the encoding to convert TO, not convert FROM.

I first thought that TCL9 had this fixed by using NULL as first argument, but it actually hasn't.
User Comments:
ralfixx added on 2024-05-22 16:53:21:
Rats... Tcl-9 most probably still contains the same error as tcl-8, 
since the NULL is actually the interp argument to a different function call 
(Tcl_UtfToExternalDStringEx vs Tcl_UtfToExternalDString).

So it seems to me that a patch equivalent to the below is required there as well.

diff -u tcl8.6.14/generic/tclUtil.c\~ tcl8.6.14/generic/tclUtil.c
--- tcl8.6.14/generic/tclUtil.c~	2024-01-24 20:26:26.000000000 +0100
+++ tcl8.6.14/generic/tclUtil.c	2024-05-22 18:41:29.714937460 +0200
@@ -4284,7 +4284,7 @@
 
 	    Tcl_MutexLock(&pgvPtr->mutex);
 	    epoch = ++pgvPtr->epoch;
-	    Tcl_UtfToExternalDString(pgvPtr->encoding, pgvPtr->value,
+	    Tcl_UtfToExternalDString(current, pgvPtr->value,
 		    pgvPtr->numBytes, &native);
 	    Tcl_ExternalToUtfDString(current, Tcl_DStringValue(&native),
 	    Tcl_DStringLength(&native), &newValue);

Diff finished.  Wed May 22 18:41:54 2024

ralfixx added on 2024-05-22 17:32:59:
And yet another STOP... :-)

It seems to me that the real problem is setting encoding to "utf-8" in the first place in win/tclWinSock.c, proc InitializeHostName().

All other initialization routines for ProcessGlobalValue variables
use NULL as encoding, which would work as well here.

So in addition, the following patch should be applied:

diff -u tcl8.6.14/win/tclWinSock.c\~ tcl8.6.14/win/tclWinSock.c
--- tcl8.6.14/win/tclWinSock.c~	2024-02-20 18:10:06.000000000 +0100
+++ tcl8.6.14/win/tclWinSock.c	2024-05-22 19:31:05.113600529 +0200
@@ -393,7 +393,7 @@
 	}
     }
 
-    *encodingPtr = Tcl_GetEncoding(NULL, "utf-8");
+    *encodingPtr = NULL;
     *lengthPtr = Tcl_DStringLength(&ds);
     *valuePtr = (char *)ckalloc(*lengthPtr + 1);
     memcpy(*valuePtr, Tcl_DStringValue(&ds), *lengthPtr + 1);

Diff finished.  Wed May 22 19:31:12 2024

ralfixx added on 2024-05-22 17:48:23:
...or rather (as in unix/tclUnixSock.c, InitializeHostName())

diff -u tcl8.6.14/win/tclWinSock.c\~ tcl8.6.14/win/tclWinSock.c
--- tcl8.6.14/win/tclWinSock.c~	2024-02-20 18:10:06.000000000 +0100
+++ tcl8.6.14/win/tclWinSock.c	2024-05-22 19:47:33.105220627 +0200
@@ -393,7 +393,7 @@
 	}
     }
 
-    *encodingPtr = Tcl_GetEncoding(NULL, "utf-8");
+    *encodingPtr = Tcl_GetEncoding(NULL, NULL);
     *lengthPtr = Tcl_DStringLength(&ds);
     *valuePtr = (char *)ckalloc(*lengthPtr + 1);
     memcpy(*valuePtr, Tcl_DStringValue(&ds), *lengthPtr + 1);

Diff finished.  Wed May 22 19:47:39 2024

oehhar added on 2024-05-23 07:38:12:

I can confirm the behavior on Windows 10:

* TCL 8.6: "läpt" on 2nd call * TCL 9.0b2: does not have the issue

To reproduce, I had to:

* go in windows to System control->System * In section "Settings for computer name, domain and workgroup", press on the change link on the right * In the next Window, press on "Change" on the "Computer name" tab * Change the computer name * Press ok * reboot

I am asking myself, who may look into this? Ashok, Jan, Sergey ?

Thank you all, Harald


jan.nijtmans added on 2024-05-23 10:24:57:

First fix attempt [e99ba027563f4781|here]

On UNIX/MacOS, and Tcl9.0b2 there is no problem, because the system encoding is always utf-8. So converting between native encoding and utf-8 doesn't change anything.

If the system encoding is not utf-8, I see two different problems. The GlobalValues are internally stored in native encoding, and also stored in a separate cache. The cache storage should be in utf-8, but it isn't always!

Further on, on Windows, the "hostname" value is stored in utf-8, in both the internal storage and in the cache. For the cache this is OK, but the native storage should have been done in the native encoding.

I don't have a hostname with non-ascii characters here. Can someone confirm whether it works or not?


oehhar added on 2024-05-23 10:40:42:

Yes, computer reboot to test is no fun...

The branch is effective:

% info hostname
läptop
% info hostname
läptop

I have compiled it with prehistoric MS-VC6++ /PSDK2003SP1 / 32 bit

Thanks for the great analysis and resolution !

Harald

P.S.: as you normally hunt any warnings, here is the current list ;-).

win\..\generic\tclIO.c(480) : warning C4761: integral size mismatch in argument; conversion supplied
win\..\generic\tclIO.c(480) : warning C4761: integral size mismatch in argument; conversion supplied
win\..\generic\tclIO.c(480) : warning C4761: integral size mismatch in argument; conversion supplied
win\..\generic\tclProc.c(2275) : warning C4550: expression evaluates to a function which is missing an argument list
win\..\generic\tclStringObj.c(2383) : warning C4761: integral size mismatch in argument; conversion supplied


oehhar added on 2024-05-23 11:00:50:

The test run has finished. I see the following test failures which were not present in 8.6.14: fCmd-9.3, fCmd-9.5, fCmd-9.7, fCmd-9.10, winFCmd-3.10

I may run test current core-8-6-branch to verify, if those failures are also there.

Take care, Harald

==== fCmd-9.3 file rename: comprehensive: file to new name FAILED
==== Contents of test case:

    createfile tf1
    createfile tf2
    testchmod 0o444 tf2
    file rename tf1 tf3
    file rename tf2 tf4
    list [lsort [glob tf*]] [file writable tf3] [file writable tf4]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: error renaming "tf2" to "tf4": permission denied
    while executing
"file rename tf2 tf4"
    ("uplevel" body line 6)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== fCmd-9.3 FAILED

---- fCmd-9.5 start


==== fCmd-9.5 file rename: comprehensive: file to self FAILED
==== Contents of test case:

    createfile tf1 tf1
    createfile tf2 tf2
    testchmod 0o444 tf2
    file rename -force tf1 tf1
    file rename -force tf2 tf2
    list [contents tf1] [contents tf2] [file writable tf1] [file writable tf2]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: error renaming "tf2": permission denied
    while executing
"file rename -force tf2 tf2"
    ("uplevel" body line 6)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== fCmd-9.5 FAILED

---- fCmd-9.7 start


==== fCmd-9.7 file rename: comprehensive: file to existing file FAILED
==== Contents of test case:

    createfile tf1
    createfile tf2
    createfile tfs1
    createfile tfs2
    createfile tfs3
    createfile tfs4
    createfile tfd1
    createfile tfd2
    createfile tfd3
    createfile tfd4
    testchmod 0o444 tfs3
    testchmod 0o444 tfs4
    testchmod 0o444 tfd2
    testchmod 0o444 tfd4
    set msg [list [catch {file rename tf1 tf2} msg] $msg]
    file rename -force tfs1 tfd1
    file rename -force tfs2 tfd2
    file rename -force tfs3 tfd3
    file rename -force tfs4 tfd4
    list [lsort [glob tf*]] $msg [file writable tfd1] [file writable tfd2] [file writable tfd3] [file writable tfd4]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: error renaming "tfs2" to "tfd2": permission denied
    while executing
"file rename -force tfs2 tfd2"
    ("uplevel" body line 18)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== fCmd-9.7 FAILED

---- fCmd-9.9 start
---- fCmd-9.10 start


==== fCmd-9.10 file rename: comprehensive: file to new name and dir FAILED
==== Contents of test case:

    createfile tf1
    createfile tf2
    file mkdir td1
    testchmod 0o444 tf2
    file rename tf1 [file join td1 tf3]
    file rename tf2 [file join td1 tf4]
    list [catch {glob tf*}] [lsort [glob -directory td1 t*]]  [file writable [file join td1 tf3]] [file writable [file join td1 tf4]]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: error renaming "tf2" to "td1/tf4": permission denied
    while executing
"file rename tf2 [file join td1 tf4]"
    ("uplevel" body line 7)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== fCmd-9.10 FAILED

==== winFCmd-3.10 TclpDeleteFile: path is readonly FAILED
==== Contents of test case:

    createfile tf1
    testchmod 0 tf1
    testfile rm tf1
    file exists tf1

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: EACCES
    while executing
"testfile rm tf1"
    ("uplevel" body line 4)
    invoked from within
"uplevel 1 $script"
---- errorCode: NONE
==== winFCmd-3.10 FAILED


oehhar added on 2024-05-23 11:58:25:

I can confirm, that the observed test failures are also in current core-8-6-branch. So, this branch does not introduce new test failures.

Thank you and take care, Harald


jan.nijtmans added on 2024-05-23 15:39:13:

Fixed on all branches now.

Thanks, @, for the report. And thanks, Harald for confirming the fix.

Closing.