Обсуждение: [PATCH] Fix POSIX compliance in pgwin32_unsetenv()

I noticed that pgwin32_unsetenv() in src/port/win32env.c lacks the input 
validation that its sibling function pgwin32_setenv() has (lines 126-132).

Without these checks, the function will crash on NULL input via 
strlen(NULL), and will accept empty strings or strings containing '=' in 
violation of POSIX.1-2008.

The attached patch adds the same validation that pgwin32_setenv already 
does, making the two functions consistent.  This is purely defensive - 
it only affects callers passing invalid arguments.

regards,

Bryan Green

On Sat, Oct 18, 2025 at 01:26:40PM -0500, Bryan Green wrote:
> I noticed that pgwin32_unsetenv() in src/port/win32env.c lacks the input
> validation that its sibling function pgwin32_setenv() has (lines 126-132).
>
> Without these checks, the function will crash on NULL input via
> strlen(NULL), and will accept empty strings or strings containing '=' in
> violation of POSIX.1-2008.
>
> The attached patch adds the same validation that pgwin32_setenv already
> does, making the two functions consistent.  This is purely defensive - it
> only affects callers passing invalid arguments.

I presume that you have tried to use this routine on some external
code on WIN32 to note that it was just crashing.

The current state of pgwin32_unsetenv() dates back to 0154345078fb.
The POSIX checks of setenv() are more recent than that, as in
7ca37fb0406b down to v14.  I agree that the inconsistency in handling
the input arguments is annoying, so if there are no objections let's
apply the same checks down to v14 like the setenv() piece.  It's
better than a hard crash.
--
Michael

On 10/19/25 20:02, Michael Paquier wrote:
> On Sat, Oct 18, 2025 at 01:26:40PM -0500, Bryan Green wrote:
>> I noticed that pgwin32_unsetenv() in src/port/win32env.c lacks the input
>> validation that its sibling function pgwin32_setenv() has (lines 126-132).
>>
>> Without these checks, the function will crash on NULL input via
>> strlen(NULL), and will accept empty strings or strings containing '=' in
>> violation of POSIX.1-2008.
>>
>> The attached patch adds the same validation that pgwin32_setenv already
>> does, making the two functions consistent.  This is purely defensive - it
>> only affects callers passing invalid arguments.
> 
> I presume that you have tried to use this routine on some external
> code on WIN32 to note that it was just crashing.
> 
> The current state of pgwin32_unsetenv() dates back to 0154345078fb.
> The POSIX checks of setenv() are more recent than that, as in
> 7ca37fb0406b down to v14.  I agree that the inconsistency in handling
> the input arguments is annoying, so if there are no objections let's
> apply the same checks down to v14 like the setenv() piece.  It's
> better than a hard crash.
> --
> Michael

I have been going through all of the windows code line by line.  That is 
how I initially noticed this.  I then wrote a program to exercise the 
code and confirm the crash. I agree it should be backported.

BG

On Sun, Oct 19, 2025 at 08:34:02PM -0500, Bryan Green wrote:
> I have been going through all of the windows code line by line.  That is how
> I initially noticed this.  I then wrote a program to exercise the code and
> confirm the crash. I agree it should be backported.

Applied down to v14.

Note that the unsetenv() WIN32 routine exists also in v13.  However, I
could not get excited about it with the branch going EOL soon, also
knowing that 7ca37fb0406b affects v14~.
--
Michael