-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix compilation with BoringSSL on Windows with MSVC #3456
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
base: main
Are you sure you want to change the base?
Conversation
Co-developed-by: Gemini 2.5 Pro
This commit contains fixes for two separate issues:
1. Deaddrop upload issues:
- The UI spinner would hang after a successful upload.
- The initial fix for this also addresses 302/404 redirects seen
on some platforms.
The root cause was incorrect HTTP response handling for the upload
POST request. This is fixed by simplifying the response handler to
send a standard, headers-only 200 OK response, and by ensuring
the request doesn't fall through to the dummy http handler.
2. Windows spawn build errors:
- The file `lib/plat/windows/windows-spawn.c` failed to build due
to undeclared identifiers and incomplete struct definitions.
This is fixed by including the necessary `Psapi.h` header and by
adding the missing `ft_create` and `ft_exit` members to the
`lws_spawn_piped` struct definition for WIN32 builds.
…fix compilation with MSVC
a0c73c1 to
7c9d4bc
Compare
|
Good job.In my case ,there should some more fix to pass compiling. diff --git a/packages/libwebsockets-fix.windows_boringssl_compat/lib/tls/openssl/lws-genec.c b/packages/libwebsockets-fix.windows_boringssl_compat/lib/tls/openssl/lws-genec.c
index 2cd29e4..91f387a 100644
--- a/packages/libwebsockets-fix.windows_boringssl_compat/lib/tls/openssl/lws-genec.c
+++ b/packages/libwebsockets-fix.windows_boringssl_compat/lib/tls/openssl/lws-genec.c
@@ -77,11 +77,13 @@ ECDSA_SIG_set0(ECDSA_SIG *sig, BIGNUM *r, BIGNUM *s)
int BN_bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
{
int i;
-#if !defined(USE_WOLFSSL)
+#if !defined(USE_WOLFSSL) && !defined(LWS_WITH_BORINGSSL)
BN_ULONG l;
#endif
-#if !defined(LIBRESSL_VERSION_NUMBER) && !defined(USE_WOLFSSL)
+
+/* BoringSSL does not expose BIGNUM internal structure, so we disable bn_check_top */
+#if !defined(LIBRESSL_VERSION_NUMBER) && !defined(USE_WOLFSSL) && !defined(LWS_WITH_BORINGSSL)
bn_check_top(a);
#endif
i = BN_num_bytes(a);
@@ -91,7 +93,7 @@ int BN_bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
memset(to, 0, (size_t)(tolen - i));
to += tolen - i;
}
-#if defined(USE_WOLFSSL)
+#if defined(USE_WOLFSSL) || defined(LWS_WITH_BORINGSSL)
BN_bn2bin(a, to);
#else
while (i--) {
diff --git a/packages/libwebsockets-fix.windows_boringssl_compat/test-apps/test-client.c b/packages/libwebsockets-fix.windows_boringssl_compat/test-apps/test-client.c
index 1cfdd35..303c84f 100644
--- a/packages/libwebsockets-fix.windows_boringssl_compat/test-apps/test-client.c
+++ b/packages/libwebsockets-fix.windows_boringssl_compat/test-apps/test-client.c
@@ -22,7 +22,7 @@
#include <stdio.h>
#include <stdlib.h>
-#if defined(LWS_HAS_GETOPT_LONG) || defined(WIN32)
+#if defined(LWS_HAS_GETOPT_LONG) || defined(_WIN32)
#include <getopt.h>
#endif
#include <string.h> |
|
@followtheart Thank you for the kind words and your interest in this PR. My understanding is that you are suggesting changes that don't seem to modify the parts of the code base that were modified by this PR (or, at least, my unique commit 7e869a6 prior to the ones by @lws-team). If that's indeed the case, could you please submit a new PR that would only deal with your suggested changes? I'm not a security/SSL/etc. expert and, therefore, I'm unable to properly review and/or validate your suggested patch for potential inclusion in libwebsockets. Thank you in advance for understanding 🙂 |
|
Don't worry about a new PR, I an just too busy to deal with the other patch today. I'll try it and deal with it as its own patch separately over the weekend. |
217e720 to
4f15c21
Compare
|
I don't get it why there are two kinds of behaviour here... I don't need the patch and presumably @maddouri didn't either. @followtheart seems to need it. What version of boringssl is everyone using? I am using aef0d16884e35f6f5f07d5a6432ce3777b6221e5 from May 2. |
|
I use the following in order to build libwebsockets @
# Prerequisite Tools ###############################################################################
param (
[Parameter(Mandatory = $true)]
[string] $WorkDir,
[string] $BuildType = "Debug",
[string] $NasmExe = "C:/code/nasm/nasm.exe",
[string] $GoExe = "C:/code/go/1.25.0/bin/go.exe",
[string] $OpensslExe = "C:/Program Files/Git/mingw64/bin/openssl.exe",
[string] $CMakeExe = "C:/Program Files/CMake/bin/cmake.exe",
[string] $GitExe = "C:/Program Files/Git/cmd/git.exe",
[string] $LwsTag = "v4.4.1",
[string] $BsslTag = "0.20250818.0",
[string] $CMakeGenerator = "Visual Studio 17 2022",
[string] $CMakeArch = "x64",
[switch] $BuildTests,
[switch] $ApplyLwsPatch
)
if ($PSVersionTable.PSVersion -lt [version]"7.5") {
throw "This script requires PowerShell >= v7.5. Remove this test if you want to try on PowerShell 5"
}
if (Test-Path "$WorkDir") {
throw "WorkDir [$WorkDir] already exists. Either remove it, remove this check or specify a new WorkDir"
}
$scriptDir = Split-Path -Path $MyInvocation.MyCommand.Definition -Parent
#$WorkDir = Join-Path "$scriptDir" "WorkDir"
New-Item -ItemType Directory -Path "$WorkDir"
$WorkDir = $WorkDir | Resolve-Path
Write-Host "WorkDir: $WorkDir"
$CMakeBuildTests = if ($BuildTests) {"ON"} else {"OFF"}
# BoringSSL ########################################################################################
$bsslSrcDir = (Join-Path "$WorkDir" "boringssl-src-$BuildType") -replace "\\","/"
$bsslBuildDir = (Join-Path "$WorkDir" "boringssl-build-$BuildType") -replace "\\","/"
$bsslInstallDir = (Join-Path "$WorkDir" "boringssl-install-$BuildType") -replace "\\","/"
& "$GitExe" clone --depth 1 --branch "$BsslTag" "https://boringssl.googlesource.com/boringssl" "$bsslSrcDir"
& "$CMakeExe" -G"$CMakeGenerator" -A"$CMakeArch" `
"-S$bsslSrcDir" `
"-B$bsslBuildDir" `
"-DCMAKE_INSTALL_PREFIX=$bsslInstallDir" `
"-DCMAKE_BUILD_TYPE=$BuildType" `
"-DCMAKE_ASM_NASM_COMPILER=$NasmExe" `
"-DGO_EXECUTABLE=$GoExe" `
"-DBUILD_TESTING=$CMakeBuildTests" `
"-DBUILD_SHARED_LIBS=OFF"
& "$CMakeExe" --build "$bsslBuildDir" --config "$BuildType" --target "install" --parallel
# LWS ##############################################################################################
$lwsSrcDir = (Join-Path "$WorkDir" "libwebsockets-src-$BuildType") -replace "\\","/"
$lwsBuildDir = (Join-Path "$WorkDir" "libwebsockets-build-$BuildType") -replace "\\","/"
$lwsInstallDir = (Join-Path "$WorkDir" "libwebsockets-install-$BuildType") -replace "\\","/"
& "$GitExe" clone --depth 1 --branch "$LwsTag" "https://github.com/warmcat/libwebsockets.git" "$lwsSrcDir"
# This patch fixes libwebsockets incompatibility with BoringSSL on Windows
if ($ApplyLwsPatch) {
& "$GitExe" -C "$lwsSrcDir" apply (Join-Path "$scriptDir" "libwebsockets_${LwsTag}_boringssl_fix.patch")
}
else {
Write-Host "Skipping libwebsockets patch"
}
& "$CMakeExe" -G"$CMakeGenerator" -A"$CMakeArch" `
"-S$lwsSrcDir" `
"-B$lwsBuildDir" `
"-DCMAKE_INSTALL_PREFIX=$lwsInstallDir" `
"-DCMAKE_BUILD_TYPE=$BuildType" `
"-DOPENSSL_EXECUTABLE=$OpensslExe" `
"-DLWS_WITH_SSL=ON" `
"-DLWS_WITH_BORINGSSL=ON" `
"-DOPENSSL_ROOT_DIR=$bsslInstallDir" `
"-DOPENSSL_INCLUDE_DIRS=$bsslInstallDir/include" `
"-DOPENSSL_LIBRARIES=$bsslInstallDir/lib/ssl.lib;$bsslInstallDir/lib/crypto.lib" `
"-DLWS_WITH_MINIMAL_EXAMPLES=$CMakeBuildTests" `
"-DBUILD_TESTING=$CMakeBuildTests" `
"-DLWS_WITHOUT_CLIENT=OFF" `
"-DLWS_WITHOUT_SERVER=OFF" `
"-DLWS_WITH_HTTP2=OFF" `
"-DLWS_IPV6=ON" `
"-DLWS_ROLE_WS=ON" `
"-DLWS_ROLE_MQTT=OFF" `
"-DLWS_WITH_SECURE_STREAMS=ON" `
"-DLWS_WITH_SECURE_STREAMS_CPP=OFF" `
"-DLWS_WITH_SHARED=OFF" `
"-DLWS_WITH_STATIC=ON" `
"-DLWS_STATIC_PIC=ON"
& "$CMakeExe" --build "$lwsBuildDir" --config "$BuildType" --target "install" --parallel
diff --git a/lib/plat/windows/private-lib-plat-windows.h b/lib/plat/windows/private-lib-plat-windows.h
index ef9a8d1..bb8778a 100644
--- a/lib/plat/windows/private-lib-plat-windows.h
+++ b/lib/plat/windows/private-lib-plat-windows.h
@@ -70,6 +70,12 @@
#if defined(LWS_WITH_TLS)
#include <wincrypt.h>
+#if defined(LWS_WITH_BORINGSSL)
+ /* Undefine wincrypt.h symbols that conflict with BoringSSL */
+ #undef X509_NAME
+ #undef X509_EXTENSIONS
+ #undef PKCS7_SIGNER_INFO
+#endif
#endif
#if defined(LWS_HAVE_PTHREAD_H)
Feel free to adapt the PowerShell script for your own testing (e.g. by changing the default values of the Edit: by the way, it seems that my patch has already been integrated into main somehow (see 772e7ca). Therefore, if you're building libwebsockets @ |
721f91f to
49de3d2
Compare
I'm using @maddouri 's libwebsockets branch and BoringSSL @ 0.20250818.0 too. set(LWS_WITH_SSL ON CACHE BOOL "" FORCE)
set(LCCSCF_USE_SSL ON CACHE BOOL "" FORCE)
set(LWS_WITH_STATIC ON CACHE BOOL "" FORCE)
set(LWS_WITH_BORINGSSL ON CACHE BOOL "" FORCE)
set(LWS_HAVE_RSA_SET0_KEY ON CACHE BOOL "" FORCE)
set(LWS_HAVE_X509_VERIFY_PARAM_set1_host ON CACHE BOOL "" FORCE)and for boringssl's cmake configuration: set(BUILD_SHARED_LIBS OFF CACHE BOOL "" FORCE)
set(OPENSSL_NO_ASM ON CACHE BOOL "" FORCE)
set(CMAKE_POSITION_INDEPENDENT_CODE ON CACHE BOOL "" FORCE) |
05bd68c to
0920c0d
Compare
a800d4a to
7f2f518
Compare
baf4897 to
0c7fa23
Compare
2b74af4 to
5f77374
Compare
a677221 to
0c67054
Compare
As discussed in #3455, there seems to be an issue when building libwebsockets with BoringSSL on Windows using MSVC.
The following patch works around that issue by
#undefiningwincrypt.hsymbols that conflict with BoringSSL. It is adapted from curl/curl#5857 which addresses curl/curl#5669 which was pointed out to me by @lws-team in #3455In my testing, this patch fixes #3455
Thanks in advance for considering this PR.