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

Jump to bottom

libcurl from a Windows dll sometimes incorrectly identifies presence of ALPN #7742

Closed
wyattoday opened this issue Sep 17, 2021 · 1 comment
Closed

libcurl from a Windows dll sometimes incorrectly identifies presence of ALPN #7742

wyattoday opened this issue Sep 17, 2021 · 1 comment
Labels
build Windows Windows-specific

Comments

@wyattoday
Copy link
Contributor

wyattoday commented Sep 17, 2021 •
edited

I did this

Compile latest libcurl on Windows with SChannel (either static or dynamic library). Then include either static or dynamic libcurl from inside a dll that will be run in another host application. ALPN detection will now sometimes fail depending on whether the host application was compiled with the correctly configured Application Manifest file.

In this scenario we control the dll (which eventually calls libCurl functionality). However, we do not control the "host application". Meaning the functionality we call in curl could work fine or break depending on whether the host application was compiled and packaged correctly.

Also, this is not a hypothetical situation -- we've seen this in the field.

I expected the following

libCurl should work the same regardless of whether the host application has a manifest configured correctly.

curl/libcurl version

7.79.0

operating system

Windows 8.1 or newer.

Why does it fail?

Incorrect Windows Version detection is used. Or, rather, flaky version detection is used. This was a problem of Microsoft's making (they decided to make some version detection functions flaky).

Solution

Use alternative version detection functions that always returns the correct version. I've already tested and made a PR with the solution. See #7727
@jay
Copy link
Member

jay commented Sep 17, 2021

I agree we should do something about this. I think libcurl should know the actual version and I don't know what disadvantage there would be to that.

@jay jay added build Windows Windows-specific labels Sep 17, 2021
@wyattoday wyattoday mentioned this issue Oct 4, 2021
Closed
jay added a commit to jay/curl that referenced this issue Oct 4, 2021
@jay
version_win32: use actual version instead of manifested version
8e2579e 
- Use RtlVerifyVersionInfo instead of VerifyVersionInfo, when possible.

Later versions of Windows have normal version functions that compare and
return versions based on the way the application is manifested, instead
of the actual version of Windows the application is running on. We
prefer the actual version of Windows so we'll now call the Rtl variant
of version functions (RtlVerifyVersionInfo) which does a proper
comparison of the actual version.

Reported-by: Wyatt O'Day

Ref: curl#7727

Fixes curl#7742
Closes #xxxx
@jay jay mentioned this issue Oct 4, 2021
Closed
jay added a commit to jay/curl that referenced this issue Oct 4, 2021
@jay
version_win32: use actual version instead of manifested version
61bd2e1 
- Use RtlVerifyVersionInfo instead of VerifyVersionInfo, when possible.

Later versions of Windows have normal version functions that compare and
return versions based on the way the application is manifested, instead
of the actual version of Windows the application is running on. We
prefer the actual version of Windows so we'll now call the Rtl variant
of version functions (RtlVerifyVersionInfo) which does a proper
comparison of the actual version.

Reported-by: Wyatt O'Day

Ref: curl#7727

Fixes curl#7742
Closes #xxxx
@jay jay closed this as completed in 5044909 Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Windows Windows-specific
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

version_win32: use actual version instead of manifested version jay/curl
2 participants
@jay @wyattoday