[PATCH] ntdll: Expand environment variables when querying them
For a test program see the bug report. A test is hard to include since environment variables won't update without a reboot. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46901 Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/ntdll/env.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/env.c b/dlls/ntdll/env.c index 367d083211..e186e3b917 100644 --- a/dlls/ntdll/env.c +++ b/dlls/ntdll/env.c @@ -164,11 +164,16 @@ NTSTATUS WINAPI RtlQueryEnvironmentVariable_U(PWSTR env, var = ENV_FindVariable(var, name->Buffer, namelen); if (var != NULL) { - value->Length = strlenW(var) * sizeof(WCHAR); + WCHAR buffer[UNICODE_STRING_MAX_CHARS]; + SIZE_T len; + RtlExpandEnvironmentStrings(env, (WCHAR *)var, strlenW(var), buffer, ARRAY_SIZE(buffer), &len); + + value->Length = (len - 1) * sizeof(WCHAR); /* Length without '\0' terminator */ if (value->Length <= value->MaximumLength) { - memmove(value->Buffer, var, min(value->Length + sizeof(WCHAR), value->MaximumLength)); + memmove(value->Buffer, buffer, min(value->Length + sizeof(WCHAR), value->MaximumLength)); + nts = STATUS_SUCCESS; } else nts = STATUS_BUFFER_TOO_SMALL; -- 2.21.0
Fabian Maurer <dark.shadow4(a)web.de> writes:
For a test program see the bug report. A test is hard to include since environment variables won't update without a reboot.
You don't need a reboot, you can test it on a private environment, or modify the environment of the current process. In fact we already have tests for RtlQueryEnvironmentVariable_U(), and that's how they are implemented. -- Alexandre Julliard julliard(a)winehq.org
You don't need a reboot, you can test it on a private environment, or modify the environment of the current process. In fact we already have tests for RtlQueryEnvironmentVariable_U(), and that's how they are implemented.
I did some more testing, and the problem is that Wine populates the PEB Environment differently. So the problem is not actually RtlQueryEnvironmentVariable_U, but how the environment is loaded from the registry. An idea how I would write a test for that? Regards, Fabian Maurer
Fabian Maurer <dark.shadow4(a)web.de> writes:
You don't need a reboot, you can test it on a private environment, or
modify the environment of the current process. In fact we already have
tests for RtlQueryEnvironmentVariable_U(), and that's how they are
implemented.
Well, the problem only ever occured when going over the registry, that's why I meant I can't test it without reboot Unless I'm missing something here, but on windows I can't get the program to get an updated environment unless I restart the system.
I did some more testing, and the problem is that Wine populates the PEB Environment differently. So the problem is not actually RtlQueryEnvironmentVariable_U, but how the environment is loaded from the registry. An idea how I would write a test for that?
If the problem is the initial environment, it would be hard to write a test, yes. FWIW my reading of that bug is that Windows behaves the same way, except maybe that the registry key order is different. I'm not sure there's anything to fix here. -- Alexandre Julliard julliard(a)winehq.org
If the problem is the initial environment, it would be hard to write a test, yes. FWIW my reading of that bug is that Windows behaves the same way, except maybe that the registry key order is different. I'm not sure there's anything to fix here.
Well, my manual tests show there definitely is a bug here, wine doesn't expand variables properly. I'll rework it and send in a proper test and fix. On that matter, there's two ways we currently load variables from the environment: 1) In kernel32 - process.c: set_registry_variables 2) In userenv - userenv_main.c: CreateEnvironmentBlock - set_registry_variables Both seem to do approximately the same (and both need better expansion), so maybe we could have one logic there. But I need to look into that if that even makes sense. If that is the case, would a private exported function in kernel32 make sense, or how should I handle that? I mean, deduplication would make sense, no? Regards, Fabian Maurer
Fabian Maurer <dark.shadow4(a)web.de> writes:
If the problem is the initial environment, it would be hard to write a test, yes. FWIW my reading of that bug is that Windows behaves the same way, except maybe that the registry key order is different. I'm not sure there's anything to fix here.
Well, my manual tests show there definitely is a bug here, wine doesn't expand variables properly. I'll rework it and send in a proper test and fix.
On that matter, there's two ways we currently load variables from the environment: 1) In kernel32 - process.c: set_registry_variables 2) In userenv - userenv_main.c: CreateEnvironmentBlock - set_registry_variables
Both seem to do approximately the same (and both need better expansion), so maybe we could have one logic there. But I need to look into that if that even makes sense. If that is the case, would a private exported function in kernel32 make sense, or how should I handle that? I mean, deduplication would make sense, no?
We don't want to add private exports to kernel32, and I don't think we want to have kernel32 depend on userenv, so keeping the duplication is preferable. -- Alexandre Julliard julliard(a)winehq.org
Please consider this patch obsolete, I sent in a test for the underlying issue - see https://source.winehq.org/patches/data/161743 Regards, Fabian Maurer
For a test program see the bug report. A test is hard to include since environment variables won't update without a reboot.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46901 Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/ntdll/env.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/env.c b/dlls/ntdll/env.c index 367d083211..e186e3b917 100644 --- a/dlls/ntdll/env.c +++ b/dlls/ntdll/env.c @@ -164,11 +164,16 @@ NTSTATUS WINAPI RtlQueryEnvironmentVariable_U(PWSTR env, var = ENV_FindVariable(var, name->Buffer, namelen); if (var != NULL) { - value->Length = strlenW(var) * sizeof(WCHAR); + WCHAR buffer[UNICODE_STRING_MAX_CHARS]; + SIZE_T len; + RtlExpandEnvironmentStrings(env, (WCHAR *)var, strlenW(var), buffer, ARRAY_SIZE(buffer), &len); + + value->Length = (len - 1) * sizeof(WCHAR); /* Length without '\0' terminator */
if (value->Length <= value->MaximumLength) { - memmove(value->Buffer, var, min(value->Length + sizeof(WCHAR), value->MaximumLength)); + memmove(value->Buffer, buffer, min(value->Length + sizeof(WCHAR), value->MaximumLength)); + nts = STATUS_SUCCESS; } else nts = STATUS_BUFFER_TOO_SMALL; -- 2.21.0
participants (2)
-
Alexandre Julliard -
Fabian Maurer