Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cmake] Can't use feature ldaps on Windows #6284

Closed
JackBoosY opened this issue Dec 7, 2020 · 2 comments
Closed

[cmake] Can't use feature ldaps on Windows #6284

JackBoosY opened this issue Dec 7, 2020 · 2 comments

Comments

@JackBoosY
Copy link

JackBoosY commented Dec 7, 2020

Hi guys:

Recently, we received an issue about curl (microsoft/vcpkg#14030), that reported that the feature ldaps could not be enabled on Windows.
Since curl in vcpkg uses cmake to build, I checked the cmake configuration in curl and found something strange:

curl/CMakeLists.txt

Lines 511 to 520 in abd846c

if(NOT CURL_DISABLE_LDAP)
if(WIN32)
option(USE_WIN32_LDAP "Use Windows LDAP implementation" ON)
if(USE_WIN32_LDAP)
check_library_exists_concat("wldap32" cldap_open HAVE_WLDAP32)
if(NOT HAVE_WLDAP32)
set(USE_WIN32_LDAP OFF)
endif()
endif()
endif()

Here we can use the system's ldaps on windows, but openldap is not disabled here, and:

curl/CMakeLists.txt

Lines 1398 to 1400 in abd846c

_add_if("LDAPS" NOT CURL_DISABLE_LDAPS AND
((USE_OPENLDAP AND SSL_ENABLED) OR
(NOT USE_OPENLDAP AND HAVE_LDAP_SSL)))

There is no judgment variable USE_WIN32_LDAP when judging whether to enable ldaps.

I think that's caused this issue.
But even if I use USE_WIN32_LDAP and explicitly enable ldaps in the configuration log, using curl -V does not explicitly enable ldaps.
Configure log:

-- Enabled protocols: DICT FILE FTP FTPS GOPHER HTTP HTTPS IMAP IMAPS LDAP LDAPS MQTT POP3 POP3S RTSP SMB SMBS SMTP SMTPS TELNET TFTP

Result:

curl 7.73.0-DEV (Windows) libcurl/7.73.0-DEV Schannel zlib/1.2.11
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI TrackMemory libz

I expected the following

ldaps should be enabled on Windows.

curl/libcurl version

7.73.0

[curl -V output]

curl 7.73.0-DEV (Windows) libcurl/7.73.0-DEV Schannel zlib/1.2.11
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI TrackMemory libz

operating system

Windows 10, Visual Studio 2017, cmake 3.18.2

@bagder
Copy link
Member

bagder commented Feb 13, 2021

It seems nobody is going to work on this. I'll add it to KNOWN_BUGS.

@bagder bagder closed this as completed in 835c263 Feb 13, 2021
vszakats added a commit to vszakats/curl that referenced this issue Mar 4, 2023
Before this patch, enabling LDAPS required a manual C flag:
https://github.com/curl/curl-for-win/blob/c1cfc31cfc04f24f7a4f946564d6f0e1b4d7dd36/curl-cmake.sh#L105

Fix this and enable LDAPS automatically when using `wldap32` (and when
not explicitly disabled). This matches autotools and `Makefile.mk`
behavior. Also remove issue from KNOWN_BUGS.

Reported-by: JackBoosY on github
Fixes curl#6284
@vszakats
Copy link
Member

vszakats commented Mar 4, 2023

Proposing a fix for this in PR #10674.

vszakats added a commit that referenced this issue Mar 5, 2023
Before this patch, enabling LDAPS required a manual C flag:
https://github.com/curl/curl-for-win/blob/c1cfc31cfc04f24f7a4f946564d6f0e1b4d7dd36/curl-cmake.sh#L105

Fix this and enable LDAPS automatically when using `wldap32` (and
when not explicitly disabled). This matches autotools and `Makefile.mk`
behavior. Also remove issue from KNOWN_BUGS.

Add workaround for MSVS 2010 warning triggered by LDAPS now enabled
in more CI tests:
`ldap.c(360): warning C4306: 'type cast' : conversion from 'int' to 'void *' of greater size`
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/46408284/job/v8mwl9yfbmoeqwlr#L312

Reported-by: JackBoosY on github
Reviewed-by: Jay Satiro
Reviewed-by: Marcel Raad
Fixes #6284
Closes #10674
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
Before this patch, enabling LDAPS required a manual C flag:
https://github.com/curl/curl-for-win/blob/c1cfc31cfc04f24f7a4f946564d6f0e1b4d7dd36/curl-cmake.sh#L105

Fix this and enable LDAPS automatically when using `wldap32` (and
when not explicitly disabled). This matches autotools and `Makefile.mk`
behavior. Also remove issue from KNOWN_BUGS.

Add workaround for MSVS 2010 warning triggered by LDAPS now enabled
in more CI tests:
`ldap.c(360): warning C4306: 'type cast' : conversion from 'int' to 'void *' of greater size`
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/46408284/job/v8mwl9yfbmoeqwlr#L312

Reported-by: JackBoosY on github
Reviewed-by: Jay Satiro
Reviewed-by: Marcel Raad
Fixes curl#6284
Closes curl#10674
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants