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

Add Windows header for CRYPT_E_REVOKED macro #411

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@vszakats
Member

vszakats commented Sep 4, 2015

Starting with curl 7.44.0 [1], compiling source file lib/strerror.c
with mingw and SSPI enabled, may fail with error:

strerror.c: In function 'Curl_sspi_strerror':
strerror.c:827:10: error: 'CRYPT_E_REVOKED' undeclared (first use in this function)
     case CRYPT_E_REVOKED:
          ^

This happens when using mingw distros that ship with Windows headers
via the w32api package. Such distro is the canonical one, which is
also offered by certain [2] CI platforms. w32api developers implemented
[3] this macro strictly per MSDN [4], solely adding it to wincrypt.h,
This made it incompatible with original Windows SDK headers which define
it in winerror.h. Other mingw distros (f.e. [5]) that use the WINE
headers, work fine.

One solution is to comply with MSDN by #include-ing <wincrypt.h>
via lib/curl_setup.h.

This patch also fixes a TAB indentation.

[1] de74e85
[2] http://www.appveyor.com/docs/installed-software#mingw-msys-cygwin
[3] https://sourceforge.net/u/cstrauss/w32api/ci/3dd1f4b7cb6ab8da80e4c2bbcdb629c8c43b0d6b/
[4] https://msdn.microsoft.com/en-us/library/windows/desktop/aa377167.aspx
[5] https://sourceforge.net/projects/mingw-w64

Add Windows header for CRYPT_E_REVOKED macro
Starting with curl 7.44.0 [1], compiling source file `lib/strerror.c`
with mingw and SSPI enabled, may fails with error:

```
strerror.c: In function 'Curl_sspi_strerror':
strerror.c:827:10: error: 'CRYPT_E_REVOKED' undeclared (first use in this function)
     case CRYPT_E_REVOKED:
          ^
```

This happens when using mingw distros that ship with Windows headers
via the `w32api` package. Such distro is the canonical one, which is
also used by certain [2] CI platforms. `w32api` developers implemented
[3] this macro strictly per MSDN [4], solely adding it to `wincrypt.h`,
This made it incompatible with official Windows headers which define
it in `winerror.h`. Other mingw distros (f.e. [5]) that use the WINE
headers, work fine.

One solution is to comply with MSDN by #include-ing `<wincrypt.h>`
via `lib/curl_setup.h`.

This patch also fixes a TAB indentation.

[1] de74e85
[2] http://www.appveyor.com/docs/installed-software#mingw-msys-cygwin
[3] https://sourceforge.net/u/cstrauss/w32api/ci/3dd1f4b7cb6ab8da80e4c2bbcdb629c8c43b0d6b/
[4] https://msdn.microsoft.com/en-us/library/windows/desktop/aa377167.aspx
[5] https://sourceforge.net/projects/mingw-w64
@vszakats

This comment has been minimized.

Member

vszakats commented Sep 4, 2015

Not sure why the CI failure, it doesn't seem to be related to this patch, given that it compiles strerror.c fine.

Anyhow an extra guard of defined(WIN32) may be added, though I'd assume USE_WINDOWS_SSPI is never defined on non-Windows platforms.

Or, for extra safety a HAVE_WINCRYPT_H could be defined and used, but (maybe besides certain versions of experimental Tiny C compiler and possibly other very old ones), this header should be present. My experience with autoconf tools is non-existent unfortunately.

@bagder

This comment has been minimized.

Member

bagder commented Sep 6, 2015

Any objections or comments from @jay or @mback2k ?

@vszakats

This comment has been minimized.

Member

vszakats commented Sep 8, 2015

@jay

This comment has been minimized.

Member

jay commented Sep 9, 2015

It may be better to just define CRYPT_E_REVOKED ourselves. It is the only CRYPT_ code that schannel will return afaict. @vszakats can you try
https://github.com/bagder/curl/compare/master...jay:fix_missing_crypt_e_revoked?expand=1

@vszakats

This comment has been minimized.

Member

vszakats commented Sep 9, 2015

If such solution is okay in curl and it's used already, it's definitely much simpler, in fact this was how I fixed it first, but went the "official" way instead.

Build is okay with the new patch:
https://ci.appveyor.com/project/vszakats/harbour-deps/build/1.0.86#L2069

jay added a commit that referenced this pull request Sep 10, 2015

@jay

This comment has been minimized.

Member

jay commented Sep 10, 2015

Thanks, landed in df6a4d3.

@jay jay closed this Sep 10, 2015

@vszakats vszakats deleted the vszakats:patch-3 branch Sep 10, 2015

@vszakats

This comment has been minimized.

Member

vszakats commented Sep 10, 2015

Thank you @jay!

jgsogo added a commit to jgsogo/curl that referenced this pull request Oct 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment