Skip to content

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Nov 21, 2025

  • _wopen -> _wsopen_s
  • _open, open -> _sopen_s
  • _wfopen -> _wfopen_s
  • fopen -> fopen_s
  • _wfreopen -> _wfreopen_s
  • freopen -> freopen_s

For better error handling and for using the CRT functions recommended
via warnings suppressed by _CRT_SECURE_NO_WARNINGS.

Also:

Refs:
https://learn.microsoft.com/cpp/c-runtime-library/reference/open-wopen
https://learn.microsoft.com/cpp/c-runtime-library/reference/sopen-s-wsopen-s
https://learn.microsoft.com/cpp/c-runtime-library/reference/freopen-wfreopen
https://learn.microsoft.com/cpp/c-runtime-library/reference/fopen-wfopen
https://learn.microsoft.com/cpp/c-runtime-library/reference/fopen-s-wfopen-s
https://learn.microsoft.com/cpp/c-runtime-library/reference/freopen-s-wfreopen-s


@vszakats vszakats marked this pull request as draft November 21, 2025 19:16
@github-actions github-actions bot added the tests label Nov 21, 2025
@vszakats
Copy link
Member Author

vszakats commented Nov 21, 2025

Issues:

  • freopen_s requires mingw-w64 4+.
  • #include <share.h> includes lib/share.h instead of share.h from the CRT.
    Ref: lib: include files using known path #16991 that seemed fixing this issue reported in share.h conflicts with windows ucrt header #16949, but apparently this
    use-case still hits it. Avoiding the name collision by renaming would be best?
  • test failures. I still got something wrong about the update that makes some tests
    fail, some others time out.
    → Caused by passing invalid pmode flags to open on Windows
  • after the above: TESTFAIL: These test cases failed: 204 1490 (file:// protocol)

@testclutch

This comment was marked as off-topic.

@vszakats
Copy link
Member Author

vszakats commented Nov 21, 2025

Fine, pytests broken (test_17_ssl_use, 20 variants failed) after Homebrew bumped wolfSSL to 5.8.4 (from previous release 5.8.2):
BAD: https://github.com/curl/curl/actions/runs/19582773452/job/56084526466
GOOD: https://github.com/curl/curl/actions/runs/19575732267

SSL_connect failed with error -311: unknown type in record hdr

https://github.com/wolfSSL/wolfssl/releases/tag/v5.8.4-stable

#19644

vszakats added a commit to vszakats/curl that referenced this pull request Nov 22, 2025
vszakats added a commit to vszakats/curl that referenced this pull request Nov 24, 2025
vszakats added a commit that referenced this pull request Nov 24, 2025
The safe (`_s`) variants of the Windows `open()` reject these flags,
while the classic ones silently accepted them.

Also:
- also drop the now unused `stat()` call on Windows.
- replace magic number with their equivalent Windows and Unix-specific
  `S_*` macros.

Refs:
https://learn.microsoft.com/cpp/c-runtime-library/reference/open-wopen
https://learn.microsoft.com/cpp/c-runtime-library/reference/fstat-fstat32-fstat64-fstati64-fstat32i64-fstat64i32

Cherry-picked from #19643
Closes #19645
vszakats added a commit that referenced this pull request Nov 24, 2025
…ision

Windows CRTs have a `share.h`. Before this patch when trying to
`#include <share.h>` it, the compiler picked up curl's internal
`lib/share.h` instead. Rename it to avoid this issue.

CRT `share.h` has constants necessary for using safe open CRT functions.

Also rename `lib/share.c` to keep matching the header.

Ref: https://learn.microsoft.com/cpp/c-runtime-library/sharing-constants
Ref: 625f2c1 #16949 #16991
Cherry-picked from #19643
Closes #19676
vszakats added a commit that referenced this pull request Nov 25, 2025
Replace:
- `open()` with `curlx_open()` (1 call).
- `fopen()` with `curlx_fopen()`.
- `fclose()` with `curlx_fclose()`.

To centralize interacting with the CRT in preparation for using "safe"
alternatives on Windows. This also adds long-filename and Unicode
support for these operations on Windows.

Keep using `open()` in the signal handler to avoid any issues with
calling code not allowed in signal handlers.

Cherry-picked from #19643
Closes #19679
@vszakats vszakats marked this pull request as ready for review November 25, 2025 00:58
@vszakats
Copy link
Member Author

TL;DR the reason for the CI failures seen earlier in #19581 (comment) were:

  • missed passing _SH_DENYNO (or other valid values) to open().
  • pre-existing code passing invalid mode flags to open().

These were addressed in this PR and a series of separate commits today.

@vszakats vszakats added the Windows Windows-specific label Nov 25, 2025
@vszakats
Copy link
Member Author

vszakats commented Nov 25, 2025

CRT functions flagged by _CRT_SECURE_NO_WARNINGS after this patch:

  • 39 'strcpy' (lib, src, curlx, libtests, servers) → strcpy_s
  • 21 'getenv' (lib, src, libtests) → _dupenv_s
  • 18 'sscanf' (1x libtests, servers) → sscanf_s
  • 3 'localtime' (src, libtests, servers) → localtime_s
  • 2 'gmtime' (lib, src) → gmtime_s
  • 1 'sprintf' (curlx) → sprintf_s → curlx: replace sprintf with snprintf #19681

@vszakats vszakats closed this in 1e7d0ba Nov 25, 2025
@vszakats vszakats deleted the wopen branch November 25, 2025 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

2 participants