Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Move GSSAPI includes into its own header

  2. Tighten test_predtest's input checks, and improve error messages.

  3. Collation documentation fixes.

  4. contrib/sslinfo needs a fix too to make hamerkop happy.

  5. Second attempt to silence SSL compile failures on hamerkop.

  1. Windows: openssl & gssapi dislike each other

    Dave Page <dpage@pgadmin.org> — 2024-05-28T19:13:17Z

    I recently noticed that at least with PostgreSQL 16, it is not possible to
build using MSVC if both OpenSSL and gssapi (MIT Kerberos) are enabled.
Both work if the other isn't included though..

I briefly tried to test with PG17 to see if it has the same issue, but it
seems like gssapi has the same problem I recently found with zlib (
https://www.postgresql.org/message-id/CA%2BOCxozrPZx57ue8rmhq6CD1Jic5uqKh80%3DvTpZurSKESn-dkw%40mail.gmail.com
).

I've yet to find time to look into this - reporting anyway rather than
sitting on it until I get round to it...

Build FAILED.

"C:\Users\dpage\Downloads\postgresql-16.3\pgsql.sln" (default target) (1) ->
"C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj" (default
target) (9) ->
(ClCompile target) ->
  C:\build64\openssl\include\openssl\x509v3.h(201,1): warning C4228:
nonstandard extension used: qualifiers after comma in declarator list are
ignored [C:\User
s\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): warning C4228:
nonstandard extension used: qualifiers after comma in declarator list are
ignored [C:\User
s\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): warning C4228:
nonstandard extension used: qualifiers after comma in declarator list are
ignored [C:\User
s\dpage\Downloads\postgresql-16.3\postgres.vcxproj]

C:\Users\dpage\Downloads\postgresql-16.3\src\backend\utils\sort\tuplesort.c(2000,1):
warning C4724: potential mod by 0 [C:\Users\dpage\Downloads\postgresql-1
6.3\postgres.vcxproj]


"C:\Users\dpage\Downloads\postgresql-16.3\pgsql.sln" (default target) (1) ->
"C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj" (default
target) (9) ->
(ClCompile target) ->
  C:\build64\openssl\include\openssl\x509v3.h(181,9): error C2059: syntax
error: '(' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(188,9): error C2059: syntax
error: '<parameter-list>'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(193,5): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(194,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(198,5): error C2061: syntax
error: identifier 'GENERAL_NAME'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.v
cxproj]
  C:\build64\openssl\include\openssl\x509v3.h(199,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2143: syntax
error: missing ')' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2143: syntax
error: missing '{' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2143: syntax
error: missing ';' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2059: syntax
error: ')' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2373: 'a':
redefinition; different type modifiers
[C:\Users\dpage\Downloads\postgresql-16.3\postgr
es.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2054: expected
'(' to follow 'ptr'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2146: syntax
error: missing ')' before identifier 'cmp'
[C:\Users\dpage\Downloads\postgresql-16.3\
postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2061: syntax
error: identifier 'cmp'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2059: syntax
error: ';' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2449: found
'{' at file scope (missing function header?)
[C:\Users\dpage\Downloads\postgresql-16.3
\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2146: syntax
error: missing ')' before identifier 'fr'
[C:\Users\dpage\Downloads\postgresql-16.3\p
ostgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(201,1): error C2061: syntax
error: identifier 'fr'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2236:
unexpected token 'struct'. Did you forget a ';'?
[C:\Users\dpage\Downloads\postgresql-16.3\p
ostgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2143: syntax
error: missing ')' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2143: syntax
error: missing '{' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2143: syntax
error: missing ';' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2059: syntax
error: ')' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2373: 'a':
redefinition; different type modifiers
[C:\Users\dpage\Downloads\postgresql-16.3\postgr
es.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2054: expected
'(' to follow 'ptr'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2146: syntax
error: missing ')' before identifier 'cmp'
[C:\Users\dpage\Downloads\postgresql-16.3\
postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2061: syntax
error: identifier 'cmp'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2059: syntax
error: ';' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2449: found
'{' at file scope (missing function header?)
[C:\Users\dpage\Downloads\postgresql-16.3
\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2146: syntax
error: missing ')' before identifier 'fr'
[C:\Users\dpage\Downloads\postgresql-16.3\p
ostgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(227,1): error C2061: syntax
error: identifier 'fr'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(295,5): error C2059: syntax
error: '(' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(296,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(313,5): error C2061: syntax
error: identifier 'DIST_POINT_NAME'
[C:\Users\dpage\Downloads\postgresql-16.3\postgre
s.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(317,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(522,5): error C2061: syntax
error: identifier 'GENERAL_NAME'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.v
cxproj]
  C:\build64\openssl\include\openssl\x509v3.h(525,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2143: syntax
error: missing ')' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2143: syntax
error: missing '{' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2143: syntax
error: missing ';' before '*'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxp
roj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2059: syntax
error: ')' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2373: 'a':
redefinition; different type modifiers
[C:\Users\dpage\Downloads\postgresql-16.3\postgr
es.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2054: expected
'(' to follow 'ptr'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2146: syntax
error: missing ')' before identifier 'cmp'
[C:\Users\dpage\Downloads\postgresql-16.3\
postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2061: syntax
error: identifier 'cmp'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2059: syntax
error: ';' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2449: found
'{' at file scope (missing function header?)
[C:\Users\dpage\Downloads\postgresql-16.3
\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2059: syntax
error: '}' [C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2146: syntax
error: missing ')' before identifier 'fr'
[C:\Users\dpage\Downloads\postgresql-16.3\p
ostgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C2061: syntax
error: identifier 'fr'
[C:\Users\dpage\Downloads\postgresql-16.3\postgres.vcxproj]
  C:\build64\openssl\include\openssl\x509v3.h(527,1): error C1003: error
count exceeds 100; stopping compilation
[C:\Users\dpage\Downloads\postgresql-16.3\post
gres.vcxproj]

    4 Warning(s)
    53 Error(s)

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

  2. Re: Windows: openssl & gssapi dislike each other

    Imran Zaheer <imran.zhir@gmail.com> — 2024-06-08T10:22:39Z

    I was able to reproduce the gssapi & openssl error on windows. I tried
on PG16 with msvc build system and on PG17 with meson build system.
The error was reproducible when enabling both openssl and gssapi from
the configurations. Turns out that it was due to the conflicting
macros.


"be-secure-openssl.c" tries to prevent this conflict here [1]. But the
error again appears when gssapi is enabled. The file
"be-secure-openssl.c" fails to compile because it has a similar
scenario as explained here [2]. The header libpq.h is indirectly
including libpq-be.h which has a wrong order of including openssl
headers. Header "gssapi.h" indirectly includes "wincrypt.h" and
openssl header should be defined after gssapi includes.

Now this can either be solved by just just undefine the macro defined
by wincrypt.h as done here [3]
```
#ifdef X509_NAME
#undef X509_NAME
#endif
```

Or we should rearrange our headers. Openssl header should be at the
bottom (after the gssapi includes).


I am attaching the patch here in which I rearranged the openssl header
in libpq-be.h


[1]: https://github.com/postgres/postgres/blob/8ba34c698d19450ccae9a5aea59a6d0bc8b75c0e/src/backend/libpq/be-secure-openssl.c#L46
[2]: https://github.com/openssl/openssl/issues/10307#issuecomment-964155382
[3]: https://github.com/postgres/postgres/blob/00ac25a3c365004821e819653c3307acd3294818/contrib/sslinfo/sslinfo.c#L29


Thanks
Imran Zaheer
Bitnine

  3. Re: Windows: openssl & gssapi dislike each other

    Andrew Dunstan <andrew@dunslane.net> — 2024-06-08T20:40:28Z

    On 2024-06-08 Sa 06:22, Imran Zaheer wrote:
> I was able to reproduce the gssapi & openssl error on windows. I tried
> on PG16 with msvc build system and on PG17 with meson build system.
> The error was reproducible when enabling both openssl and gssapi from
> the configurations. Turns out that it was due to the conflicting
> macros.
>
>
> "be-secure-openssl.c" tries to prevent this conflict here [1]. But the
> error again appears when gssapi is enabled. The file
> "be-secure-openssl.c" fails to compile because it has a similar
> scenario as explained here [2]. The header libpq.h is indirectly
> including libpq-be.h which has a wrong order of including openssl
> headers. Header "gssapi.h" indirectly includes "wincrypt.h" and
> openssl header should be defined after gssapi includes.
>
> Now this can either be solved by just just undefine the macro defined
> by wincrypt.h as done here [3]
> ```
> #ifdef X509_NAME
> #undef X509_NAME
> #endif
> ```
>
> Or we should rearrange our headers. Openssl header should be at the
> bottom (after the gssapi includes).
>
>
> I am attaching the patch here in which I rearranged the openssl header
> in libpq-be.h
>
>
> [1]: https://github.com/postgres/postgres/blob/8ba34c698d19450ccae9a5aea59a6d0bc8b75c0e/src/backend/libpq/be-secure-openssl.c#L46
> [2]: https://github.com/openssl/openssl/issues/10307#issuecomment-964155382
> [3]: https://github.com/postgres/postgres/blob/00ac25a3c365004821e819653c3307acd3294818/contrib/sslinfo/sslinfo.c#L29
>
>

Let's be consistent and use the #undef from [3]. I did find the comment 
in sslinfo.c slightly confusing until I understood that this was a 
#define clashing with a typedef.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  4. Re: Windows: openssl & gssapi dislike each other

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-06-08T22:21:24Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> On 2024-06-08 Sa 06:22, Imran Zaheer wrote:
>> Now this can either be solved by just just undefine the macro defined
>> by wincrypt.h as done here [3]
>> Or we should rearrange our headers. Openssl header should be at the
>> bottom (after the gssapi includes).

> Let's be consistent and use the #undef from [3].

+1.  Depending on header order is not great, especially when you have
to make it depend on an order that is directly contradictory to
project conventions [0].

			regards, tom lane

[0] https://wiki.postgresql.org/wiki/Committing_checklist#Policies

  5. Re: Windows: openssl & gssapi dislike each other

    Imran Zaheer <imran.zhir@gmail.com> — 2024-06-09T07:28:59Z

    Hi

I am submitting two new patches. We can undefine the macro at two locations

1). As be-secure-openssl.c [1] was the actual
file where the conflict happened so I undefined the macro here before
the ssl includes. I changed the comment a little to make it understandable.
I am also attaching the error generated with ninja build.

OR

2). Right after the gssapi includes in libpq-be.h


Thanks
Imran Zaheer
Bitnine

[1]: https://github.com/postgres/postgres/blob/00ac25a3c365004821e819653c3307acd3294818/src/backend/libpq/be-secure-openssl.c#L46

On Sun, Jun 9, 2024 at 7:21 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Andrew Dunstan <andrew@dunslane.net> writes:
> > On 2024-06-08 Sa 06:22, Imran Zaheer wrote:
> >> Now this can either be solved by just just undefine the macro defined
> >> by wincrypt.h as done here [3]
> >> Or we should rearrange our headers. Openssl header should be at the
> >> bottom (after the gssapi includes).
>
> > Let's be consistent and use the #undef from [3].
>
> +1.  Depending on header order is not great, especially when you have
> to make it depend on an order that is directly contradictory to
> project conventions [0].
>
>                         regards, tom lane
>
> [0] https://wiki.postgresql.org/wiki/Committing_checklist#Policies

  6. Re: Windows: openssl & gssapi dislike each other

    Dave Page <dpage@pgadmin.org> — 2024-06-11T09:19:17Z

    Hi

On Sun, 9 Jun 2024 at 08:29, Imran Zaheer <imran.zhir@gmail.com> wrote:

> Hi
>
> I am submitting two new patches. We can undefine the macro at two locations
>
> 1). As be-secure-openssl.c [1] was the actual
> file where the conflict happened so I undefined the macro here before
> the ssl includes. I changed the comment a little to make it understandable.
> I am also attaching the error generated with ninja build.
>
> OR
>
> 2). Right after the gssapi includes in libpq-be.h
>

Thank you for working on this. I can confirm the undef version compiles and
passes tests with 16.3.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

  7. Re: Windows: openssl & gssapi dislike each other

    Andrew Dunstan <andrew@dunslane.net> — 2024-06-11T11:22:18Z

    On 2024-06-11 Tu 05:19, Dave Page wrote:
> Hi
>
> On Sun, 9 Jun 2024 at 08:29, Imran Zaheer <imran.zhir@gmail.com> wrote:
>
>     Hi
>
>     I am submitting two new patches. We can undefine the macro at two
>     locations
>
>     1). As be-secure-openssl.c [1] was the actual
>     file where the conflict happened so I undefined the macro here before
>     the ssl includes. I changed the comment a little to make it
>     understandable.
>     I am also attaching the error generated with ninja build.
>
>     OR
>
>     2). Right after the gssapi includes in libpq-be.h
>
>
> Thank you for working on this. I can confirm the undef version 
> compiles and passes tests with 16.3.
>

Thanks for testing.

I think I prefer approach 2, which should also allow us to remove the 
#undef in sslinfo.c so we only need to do this in one place.


cheers


andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

  8. Re: Windows: openssl & gssapi dislike each other

    Dave Page <dpage@pgadmin.org> — 2024-06-11T15:33:55Z

    On Tue, 11 Jun 2024 at 12:22, Andrew Dunstan <andrew@dunslane.net> wrote:

>
> On 2024-06-11 Tu 05:19, Dave Page wrote:
>
> Hi
>
> On Sun, 9 Jun 2024 at 08:29, Imran Zaheer <imran.zhir@gmail.com> wrote:
>
>> Hi
>>
>> I am submitting two new patches. We can undefine the macro at two
>> locations
>>
>> 1). As be-secure-openssl.c [1] was the actual
>> file where the conflict happened so I undefined the macro here before
>> the ssl includes. I changed the comment a little to make it
>> understandable.
>> I am also attaching the error generated with ninja build.
>>
>> OR
>>
>> 2). Right after the gssapi includes in libpq-be.h
>>
>
> Thank you for working on this. I can confirm the undef version compiles
> and passes tests with 16.3.
>
>
>
> Thanks for testing.
>
> I think I prefer approach 2, which should also allow us to remove the
> #undef in sslinfo.c so we only need to do this in one place.
>
OK, well that version compiles and passes tests as well :-)

Thanks!

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

  9. Re: Windows: openssl & gssapi dislike each other

    Imran Zaheer <imran.zhir@gmail.com> — 2024-06-12T15:12:51Z

    Hi

I removed the macro from the sslinfo.c as suggested by Andrew. Then I
was thinking maybe we can undo some other similar code.

I rearranged the headers to their previous position in
be-secure-openssl.c and in fe-secure-openssl.c. I was able to compile
with gssapi and openssl enabled. You can look into the original commits. [1,
2]
Is it ok if we undo the changes from these commits?

I am attaching two new patches.
One with macro guards removed from ssinfo.c.
Second patch will additionally rearrange headers for
be-secure-openssl.c and in fe-secure-openssl.c to their previous
position.

Thanks
Imran Zaheer
Bitnine

[1]: https://github.com/postgres/postgres/commit/1241fcbd7e649414f09f9858ba73e63975dcff64
[2]: https://github.com/postgres/postgres/commit/568620dfd6912351b4127435eca5309f823abde8

On Wed, Jun 12, 2024 at 12:34 AM Dave Page <dpage@pgadmin.org> wrote:
>
>
>
> On Tue, 11 Jun 2024 at 12:22, Andrew Dunstan <andrew@dunslane.net> wrote:
>>
>>
>> On 2024-06-11 Tu 05:19, Dave Page wrote:
>>
>> Hi
>>
>> On Sun, 9 Jun 2024 at 08:29, Imran Zaheer <imran.zhir@gmail.com> wrote:
>>>
>>> Hi
>>>
>>> I am submitting two new patches. We can undefine the macro at two locations
>>>
>>> 1). As be-secure-openssl.c [1] was the actual
>>> file where the conflict happened so I undefined the macro here before
>>> the ssl includes. I changed the comment a little to make it understandable.
>>> I am also attaching the error generated with ninja build.
>>>
>>> OR
>>>
>>> 2). Right after the gssapi includes in libpq-be.h
>>
>>
>> Thank you for working on this. I can confirm the undef version compiles and passes tests with 16.3.
>>
>>
>>
>> Thanks for testing.
>>
>> I think I prefer approach 2, which should also allow us to remove the #undef in sslinfo.c so we only need to do this in one place.
>
> OK, well that version compiles and passes tests as well :-)
>
> Thanks!
>
> --
> Dave Page
> pgAdmin: https://www.pgadmin.org
> PostgreSQL: https://www.postgresql.org
> EDB: https://www.enterprisedb.com
>

  10. Re: Windows: openssl & gssapi dislike each other

    Andres Freund <andres@anarazel.de> — 2024-07-08T17:32:04Z

    Hi,


On 2024-06-13 00:12:51 +0900, Imran Zaheer wrote:
> I removed the macro from the sslinfo.c as suggested by Andrew. Then I
> was thinking maybe we can undo some other similar code.

What precisely do you mean by that? Just getting rid of the "ordered include"
of openssl headers in {fe,be}-secure-openssl.h?


> I rearranged the headers to their previous position in
> be-secure-openssl.c and in fe-secure-openssl.c. I was able to compile
> with gssapi and openssl enabled. You can look into the original commits. [1,
> 2]
> Is it ok if we undo the changes from these commits?
>
> I am attaching two new patches.
> One with macro guards removed from ssinfo.c.
> Second patch will additionally rearrange headers for
> be-secure-openssl.c and in fe-secure-openssl.c to their previous
> position.

One thing that concerns me with this is that there are other includes of
gssapi/gssapi.h (e.g. in , which haven't been changed here. ISTM we ought to do apply
the same change to all of those, otherwise we're just waiting for the problem
to re-appear.

I wonder if we should add a src/include/libpq/pg-gssapi.h or such, which could
wrap the entire ifdeferry for gss support. Something like


#ifdef ENABLE_GSS

#if defined(HAVE_GSSAPI_H)
#include <gssapi.h>
#include <gssapi_ext.h>
#else
#include <gssapi/gssapi.h>
#include <gssapi/gssapi_ext.h>
#endif

/*
 * On Windows, <wincrypt.h> includes a #define for X509_NAME, which breaks our
 * ability to use OpenSSL's version of that symbol if <wincrypt.h> is pulled
 * in after <openssl/ssl.h> ... and, at least on some builds, it is.  We
 * can't reliably fix that by re-ordering #includes, because libpq/libpq-be.h
 * #includes <openssl/ssl.h>.  Instead, just zap the #define again here.
 */
#ifdef X509_NAME
#undef X509_NAME
#endif

#endif                                                 /* ENABLE_GSS */

Which'd allow the various places using gss (libpq-be.h, be-gssapi-common.h,
libpq-int.h) to just include pg-gssapi.h and get all of the above without
redundancy?


Another thing that concerns me about this approach is that it seems to assume
that the only source of such conflicting includes is gssapi. What if some
other header pulls in wincrypt.h?  But I can't really see a way out of that...

Greetings,

Andres Freund

  11. Re: Windows: openssl & gssapi dislike each other

    Imran Zaheer <imran.zhir@gmail.com> — 2024-07-10T17:06:40Z

    On Tue, Jul 9, 2024 at 2:32 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
>
> On 2024-06-13 00:12:51 +0900, Imran Zaheer wrote:
> > I removed the macro from the sslinfo.c as suggested by Andrew. Then I
> > was thinking maybe we can undo some other similar code.
>
> What precisely do you mean by that? Just getting rid of the "ordered include"
> of openssl headers in {fe,be}-secure-openssl.h?
>
Hi

I reordered the includes in {fe,be}-secure-openssl.h as they were also placed
there to resolve similar errors and also were contradicting the
project conventions [1].
But looks like it's better to not touch those as they were for future proofing.

>
> > I rearranged the headers to their previous position in
> > be-secure-openssl.c and in fe-secure-openssl.c. I was able to compile
> > with gssapi and openssl enabled. You can look into the original commits. [1,
> > 2]
> > Is it ok if we undo the changes from these commits?
> >
> > I am attaching two new patches.
> > One with macro guards removed from ssinfo.c.
> > Second patch will additionally rearrange headers for
> > be-secure-openssl.c and in fe-secure-openssl.c to their previous
> > position.
>
> One thing that concerns me with this is that there are other includes of
> gssapi/gssapi.h (e.g. in , which haven't been changed here. ISTM we ought to do apply
> the same change to all of those, otherwise we're just waiting for the problem
> to re-appear.
>

Yes this should be better.

> I wonder if we should add a src/include/libpq/pg-gssapi.h or such, which could
> wrap the entire ifdeferry for gss support. Something like
>
>
> #ifdef ENABLE_GSS
>
> #if defined(HAVE_GSSAPI_H)
> #include <gssapi.h>
> #include <gssapi_ext.h>
> #else
> #include <gssapi/gssapi.h>
> #include <gssapi/gssapi_ext.h>
> #endif
>
> /*
>  * On Windows, <wincrypt.h> includes a #define for X509_NAME, which breaks our
>  * ability to use OpenSSL's version of that symbol if <wincrypt.h> is pulled
>  * in after <openssl/ssl.h> ... and, at least on some builds, it is.  We
>  * can't reliably fix that by re-ordering #includes, because libpq/libpq-be.h
>  * #includes <openssl/ssl.h>.  Instead, just zap the #define again here.
>  */
> #ifdef X509_NAME
> #undef X509_NAME
> #endif
>
> #endif                                                 /* ENABLE_GSS */
>
> Which'd allow the various places using gss (libpq-be.h, be-gssapi-common.h,
> libpq-int.h) to just include pg-gssapi.h and get all of the above without
> redundancy?
>
>
> Another thing that concerns me about this approach is that it seems to assume
> that the only source of such conflicting includes is gssapi. What if some
> other header pulls in wincrypt.h?  But I can't really see a way out of that...
>
> Greetings,
>
> Andres Freund

Creating src/include/libpq/pg-gssapi.h can be another great way of
handling these includes. I compiled successfully but couldn't do
proper testing as there is something wrong with my windows env.

And you are right, the approach we are going with right now only
assumes that it's due to the
gssapi as the bug also appeared when building with gssapi (openssl &
gssapi build). What if openssl clashes with
some other lib too which indirectly includes wincrypt.h

For now maybe we can do the future proofing for gssapi & openssl includes
and do testing if openssl clashes with some other lib too.

Thanks
Imran Zaheer

[1]: https://wiki.postgresql.org/wiki/Committing_checklist#Policies
(3rd last point)

  12. Re: Windows: openssl & gssapi dislike each other

    Daniel Gustafsson <daniel@yesql.se> — 2025-01-22T09:17:13Z

    > On 10 Jul 2024, at 19:06, Imran Zaheer <imran.zhir@gmail.com> wrote:

(Reviving an old thread to give them a chance to finish before v18)

> For now maybe we can do the future proofing for gssapi & openssl includes
> and do testing if openssl clashes with some other lib too.

Where did this end up, is compilation on Windows with OpenSSL and GSSAPI still
an issue?  AFAICT the fixes from this thread are yet to be applied and it would
be nice to have that done before v18 if still needed.

--
Daniel Gustafsson

  13. Re: Windows: openssl & gssapi dislike each other

    Dave Page <dpage@pgadmin.org> — 2025-01-22T09:25:30Z

    On Wed, 22 Jan 2025 at 09:17, Daniel Gustafsson <daniel@yesql.se> wrote:

> > On 10 Jul 2024, at 19:06, Imran Zaheer <imran.zhir@gmail.com> wrote:
>
> (Reviving an old thread to give them a chance to finish before v18)
>
> > For now maybe we can do the future proofing for gssapi & openssl includes
> > and do testing if openssl clashes with some other lib too.
>
> Where did this end up, is compilation on Windows with OpenSSL and GSSAPI
> still
> an issue?  AFAICT the fixes from this thread are yet to be applied and it
> would
> be nice to have that done before v18 if still needed.
>

I kid you not, 20 seconds ago I hit send on a message on a pgAdmin thread
where I was complaining this bug was outstanding to someone asking about
GSSAPI support on Windows. That's quite the coincidence.

 Anyway, no fix was committed as far as I know. I would suggest it should
be back-patched as well.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
pgEdge: https://www.pgedge.com

  14. Re: Windows: openssl & gssapi dislike each other

    Andrew Dunstan <andrew@dunslane.net> — 2025-01-24T20:07:42Z

    On 2025-01-22 We 4:25 AM, Dave Page wrote:
>
>
> On Wed, 22 Jan 2025 at 09:17, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>     > On 10 Jul 2024, at 19:06, Imran Zaheer <imran.zhir@gmail.com> wrote:
>
>     (Reviving an old thread to give them a chance to finish before v18)
>
>     > For now maybe we can do the future proofing for gssapi & openssl
>     includes
>     > and do testing if openssl clashes with some other lib too.
>
>     Where did this end up, is compilation on Windows with OpenSSL and
>     GSSAPI still
>     an issue?  AFAICT the fixes from this thread are yet to be applied
>     and it would
>     be nice to have that done before v18 if still needed.
>
>
> I kid you not, 20 seconds ago I hit send on a message on a pgAdmin 
> thread where I was complaining this bug was outstanding to someone 
> asking about GSSAPI support on Windows. That's quite the coincidence.
>
>  Anyway, no fix was committed as far as I know. I would suggest it 
> should be back-patched as well.
>
>

I'm quite partial to the approach suggested upthread by Andres (a 
separate pg_gssapi.h file). If there's agreement on that I'm prepared to 
go and make it happen, unless Daniel beats me to it. Backpatching also 
seems reasonable.


cheers


andrew



--
Andrew Dunstan
EDB:https://www.enterprisedb.com

  15. Re: Windows: openssl & gssapi dislike each other

    Dave Page <dpage@pgadmin.org> — 2025-01-24T20:24:31Z

    On Fri, 24 Jan 2025 at 20:07, Andrew Dunstan <andrew@dunslane.net> wrote:

>
> On 2025-01-22 We 4:25 AM, Dave Page wrote:
>
>
>
> On Wed, 22 Jan 2025 at 09:17, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> > On 10 Jul 2024, at 19:06, Imran Zaheer <imran.zhir@gmail.com> wrote:
>>
>> (Reviving an old thread to give them a chance to finish before v18)
>>
>> > For now maybe we can do the future proofing for gssapi & openssl
>> includes
>> > and do testing if openssl clashes with some other lib too.
>>
>> Where did this end up, is compilation on Windows with OpenSSL and GSSAPI
>> still
>> an issue?  AFAICT the fixes from this thread are yet to be applied and it
>> would
>> be nice to have that done before v18 if still needed.
>>
>
> I kid you not, 20 seconds ago I hit send on a message on a pgAdmin thread
> where I was complaining this bug was outstanding to someone asking about
> GSSAPI support on Windows. That's quite the coincidence.
>
>  Anyway, no fix was committed as far as I know. I would suggest it should
> be back-patched as well.
>
>
>
> I'm quite partial to the approach suggested upthread by Andres (a separate
> pg_gssapi.h file). If there's agreement on that I'm prepared to go and make
> it happen, unless Daniel beats me to it. Backpatching also seems reasonable
>

That solution seems quite elegant to me.

Thanks.


—
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
pgEdge: https://www.pgedge.com

  16. Re: Windows: openssl & gssapi dislike each other

    Daniel Gustafsson <daniel@yesql.se> — 2025-01-24T21:45:23Z

    > On 24 Jan 2025, at 21:07, Andrew Dunstan <andrew@dunslane.net> wrote:
> On 2025-01-22 We 4:25 AM, Dave Page wrote:

>>  Anyway, no fix was committed as far as I know. I would suggest it should be back-patched as well.
> 
> I'm quite partial to the approach suggested upthread by Andres (a separate pg_gssapi.h file). If there's agreement on that I'm prepared to go and make it happen, unless Daniel beats me to it. Backpatching also seems reasonable.

Thanks for the reminder, I also agree that Andres' suggestion is the best
option.  I hacked up a patch but got distracted by the pgcrypto GUC patch for a
bit.  I'll share what I have once I've done a little testing.

--
Daniel Gustafsson

  17. Re: Windows: openssl & gssapi dislike each other

    Daniel Gustafsson <daniel@yesql.se> — 2025-01-31T14:54:45Z

    > On 24 Jan 2025, at 22:45, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
>> On 24 Jan 2025, at 21:07, Andrew Dunstan <andrew@dunslane.net> wrote:
>> On 2025-01-22 We 4:25 AM, Dave Page wrote:
> 
>>> Anyway, no fix was committed as far as I know. I would suggest it should be back-patched as well.
>> 
>> I'm quite partial to the approach suggested upthread by Andres (a separate pg_gssapi.h file). If there's agreement on that I'm prepared to go and make it happen, unless Daniel beats me to it. Backpatching also seems reasonable.
> 
> Thanks for the reminder, I also agree that Andres' suggestion is the best
> option.  I hacked up a patch but got distracted by the pgcrypto GUC patch for a
> bit.  I'll share what I have once I've done a little testing.

After another (conference induced) distraction I remembered this thread again
and tested to build/test the patch against a GSSAPI enabled tree.  I think this
is along the right lines.

--
Daniel Gustafsson

  18. Re: Windows: openssl & gssapi dislike each other

    Andres Freund <andres@anarazel.de> — 2025-01-31T15:29:23Z

    Hi,

On 2025-01-31 15:54:45 +0100, Daniel Gustafsson wrote:
> > On 24 Jan 2025, at 22:45, Daniel Gustafsson <daniel@yesql.se> wrote:
> > 
> >> On 24 Jan 2025, at 21:07, Andrew Dunstan <andrew@dunslane.net> wrote:
> >> On 2025-01-22 We 4:25 AM, Dave Page wrote:
> > 
> >>> Anyway, no fix was committed as far as I know. I would suggest it should be back-patched as well.
> >> 
> >> I'm quite partial to the approach suggested upthread by Andres (a separate pg_gssapi.h file). If there's agreement on that I'm prepared to go and make it happen, unless Daniel beats me to it. Backpatching also seems reasonable.
> > 
> > Thanks for the reminder, I also agree that Andres' suggestion is the best
> > option.  I hacked up a patch but got distracted by the pgcrypto GUC patch for a
> > bit.  I'll share what I have once I've done a little testing.
> 
> After another (conference induced) distraction I remembered this thread again
> and tested to build/test the patch against a GSSAPI enabled tree.  I think this
> is along the right lines.

Largely looks good to me, thanks for tackling this!


> diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
> index 2f6c29200ba..f6d8f938c83 100644
> --- a/src/include/libpq/libpq-be.h
> +++ b/src/include/libpq/libpq-be.h
> @@ -28,11 +28,7 @@
>  #include <netinet/tcp.h>
>  
>  #ifdef ENABLE_GSS
> -#if defined(HAVE_GSSAPI_H)
> -#include <gssapi.h>
> -#else
> -#include <gssapi/gssapi.h>
> -#endif							/* HAVE_GSSAPI_H */
> +#include "libpq/pg-gssapi.h"
>  #endif							/* ENABLE_GSS */

This #ifdef ENABLE_GSS probably isn't necessary anymore.


Greetings,

Andres Freund

  19. Re: Windows: openssl & gssapi dislike each other

    Daniel Gustafsson <daniel@yesql.se> — 2025-01-31T15:34:51Z

    > On 31 Jan 2025, at 16:29, Andres Freund <andres@anarazel.de> wrote:

>> #ifdef ENABLE_GSS
>> -#if defined(HAVE_GSSAPI_H)
>> -#include <gssapi.h>
>> -#else
>> -#include <gssapi/gssapi.h>
>> -#endif /* HAVE_GSSAPI_H */
>> +#include "libpq/pg-gssapi.h"
>> #endif /* ENABLE_GSS */
> 
> This #ifdef ENABLE_GSS probably isn't necessary anymore.

Yeah, I only left it for code documentation reasons to keep readers from
thinking the ifdef was missing and had to go chase it in the new file.  It's
definitely not required though I for sure don't mind removing it if others feel
it's pointless.

--
Daniel Gustafsson

  20. Re: Windows: openssl & gssapi dislike each other

    vignesh C <vignesh21@gmail.com> — 2025-03-26T08:53:55Z

    On Fri, 31 Jan 2025 at 21:05, Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 31 Jan 2025, at 16:29, Andres Freund <andres@anarazel.de> wrote:
>
> >> #ifdef ENABLE_GSS
> >> -#if defined(HAVE_GSSAPI_H)
> >> -#include <gssapi.h>
> >> -#else
> >> -#include <gssapi/gssapi.h>
> >> -#endif /* HAVE_GSSAPI_H */
> >> +#include "libpq/pg-gssapi.h"
> >> #endif /* ENABLE_GSS */
> >
> > This #ifdef ENABLE_GSS probably isn't necessary anymore.
>
> Yeah, I only left it for code documentation reasons to keep readers from
> thinking the ifdef was missing and had to go chase it in the new file.  It's
> definitely not required though I for sure don't mind removing it if others feel
> it's pointless.

Few thoughts:
1) I also felt that this could be removed.

2) Was the copyright year retained as 1996 intentionally for the new
"pg-gssapi.h" file added because the contents were copied from other
files?
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California

I see in few other places were new file was created, it was mentioned
as "Copyright (c) 2025, PostgreSQL Global Development Group"

3) Apart from that, there was a small whitespace issue while applying the patch:
git am v1-0001-Move-GSSAPI-includes-into-its-own-header.patch
Applying: Move GSSAPI includes into its own header
.git/rebase-apply/patch:116: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

Overall patch looks good to me.

Regards,
Vignesh

  21. Re: Windows: openssl & gssapi dislike each other

    Andrew Dunstan <andrew@dunslane.net> — 2025-03-26T11:11:11Z

    On 2025-03-26 We 4:53 AM, vignesh C wrote:
> On Fri, 31 Jan 2025 at 21:05, Daniel Gustafsson <daniel@yesql.se> wrote:
>>> On 31 Jan 2025, at 16:29, Andres Freund <andres@anarazel.de> wrote:
>>>> #ifdef ENABLE_GSS
>>>> -#if defined(HAVE_GSSAPI_H)
>>>> -#include <gssapi.h>
>>>> -#else
>>>> -#include <gssapi/gssapi.h>
>>>> -#endif /* HAVE_GSSAPI_H */
>>>> +#include "libpq/pg-gssapi.h"
>>>> #endif /* ENABLE_GSS */
>>> This #ifdef ENABLE_GSS probably isn't necessary anymore.
>> Yeah, I only left it for code documentation reasons to keep readers from
>> thinking the ifdef was missing and had to go chase it in the new file.  It's
>> definitely not required though I for sure don't mind removing it if others feel
>> it's pointless.
> Few thoughts:
> 1) I also felt that this could be removed.
>
> 2) Was the copyright year retained as 1996 intentionally for the new
> "pg-gssapi.h" file added because the contents were copied from other
> files?
> + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
> + * Portions Copyright (c) 1994, Regents of the University of California
>
> I see in few other places were new file was created, it was mentioned
> as "Copyright (c) 2025, PostgreSQL Global Development Group"
>
> 3) Apart from that, there was a small whitespace issue while applying the patch:
> git am v1-0001-Move-GSSAPI-includes-into-its-own-header.patch
> Applying: Move GSSAPI includes into its own header
> .git/rebase-apply/patch:116: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
>
> Overall patch looks good to me.
>


LGTM too.

I was hoping to test it but I haven't been able to set up an environment 
for testing. But I don't think that should hold it up.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  22. Re: Windows: openssl & gssapi dislike each other

    Daniel Gustafsson <daniel@yesql.se> — 2025-03-26T14:47:21Z

    > On 26 Mar 2025, at 09:53, vignesh C <vignesh21@gmail.com> wrote:
> On Fri, 31 Jan 2025 at 21:05, Daniel Gustafsson <daniel@yesql.se> wrote:
>> 
>>> On 31 Jan 2025, at 16:29, Andres Freund <andres@anarazel.de> wrote:

>>> This #ifdef ENABLE_GSS probably isn't necessary anymore.
>> 
>> Yeah, I only left it for code documentation reasons to keep readers from
>> thinking the ifdef was missing and had to go chase it in the new file.  It's
>> definitely not required though I for sure don't mind removing it if others feel
>> it's pointless.
> 
> Few thoughts:
> 1) I also felt that this could be removed.

Fixed.

> 2) Was the copyright year retained as 1996 intentionally for the new
> "pg-gssapi.h" file added because the contents were copied from other
> files?
> + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
> + * Portions Copyright (c) 1994, Regents of the University of California
> 
> I see in few other places were new file was created, it was mentioned
> as "Copyright (c) 2025, PostgreSQL Global Development Group"

Right, this is new file but not net-new content so I retained the copyright
from the original location. We are standing on the shoulders of giants.

> 3) Apart from that, there was a small whitespace issue while applying the patch:
> git am v1-0001-Move-GSSAPI-includes-into-its-own-header.patch
> Applying: Move GSSAPI includes into its own header
> .git/rebase-apply/patch:116: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.

Fixed.

> Overall patch looks good to me.

Thanks for review!  Pushed after making the above changes and taking it for
another CI run.

--
Daniel Gustafsson

  23. Re: Windows: openssl & gssapi dislike each other

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-03-26T16:15:13Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> Thanks for review!  Pushed after making the above changes and taking it for
> another CI run.

CF entry should be marked closed no?

https://commitfest.postgresql.org/patch/5060/

			regards, tom lane

  24. Re: Windows: openssl & gssapi dislike each other

    Daniel Gustafsson <daniel@yesql.se> — 2025-03-26T16:32:17Z

    > On 26 Mar 2025, at 17:15, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Daniel Gustafsson <daniel@yesql.se> writes:
>> Thanks for review!  Pushed after making the above changes and taking it for
>> another CI run.
> 
> CF entry should be marked closed no?

Yep, just wanted to allow some time to see how the buildfarm liked it before
closing.  Done now, thanks for the reminder.

--
Daniel Gustafsson

  25. Re: Windows: openssl & gssapi dislike each other

    Dave Page <dpage@pgadmin.org> — 2025-03-31T10:38:37Z

    On Wed, 26 Mar 2025 at 16:32, Daniel Gustafsson <daniel@yesql.se> wrote:

> > On 26 Mar 2025, at 17:15, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Daniel Gustafsson <daniel@yesql.se> writes:
> >> Thanks for review!  Pushed after making the above changes and taking it
> for
> >> another CI run.
> >
> > CF entry should be marked closed no?
>
> Yep, just wanted to allow some time to see how the buildfarm liked it
> before
> closing.  Done now, thanks for the reminder.
>

Thank you for dealing with this!

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
pgEdge: https://www.pgedge.com