Skip to content

Conversation

@markhermeling
Copy link

No description provided.

vszakats and others added 30 commits November 28, 2024 11:55
The ECH feature cannot be built without HTTPS RR.

ECH automatically implied HTTPS RR in `./configure` but not in CMake,
winbuild, documentation.

Also update documentation and CI configs.

Follow-up to a362962 curl#11922
Closes curl#15648
The MSVC UWP job in CI did not actually enable UWP. Fix this and
the fallouts discovered after enabling it.

- GHA/windows: make sure to enable UWP in MSVC vcpkg UWP job.
  Use the CMake options and C flags already used for mingw-w64, but use
  `WINAPI_FAMILY_PC_APP` instead of the deprecated `WINAPI_FAMILY_APP`.
  (The former is not supported by mingw-w64, so leave it there as-is.)
  Follow-up to cb22cfc curl#14077

- GHA/windows: by default the MSVC UWP job became 2x-3x slower than
  others after actually enabling UWP. Most of it is caused by
  CMake/MSBuild automatically building full APPX containers for each
  `.exe` target. This includes 21 CMake feature detections. Each
  detection app is built into a 15MB APPX project, with code signing,
  logos, etc. Example:
    https://github.com/curl/curl/actions/runs/12056968170/job/33620610958
  Disable this overhead for curl build targets via custom
  `CMAKE_VS_GLOBALS` options. I've found no way to apply them to feature
  detection targets, so those remain slow.

- cmake: automatically enable Unicode for UWP builds. It's required.
  Also stop enabling it manually in the existing CI job.

- tests: fix `getpid()` use for Windows UWP:
  ```
  tests\server\util.c(281,21): warning C4013: 'getpid' undefined; assuming extern returning int
  ```
  Ref: https://github.com/curl/curl/actions/runs/12061215311/job/33632904249#step:11:38

- src/tool_doswin: disable `GetLoadedModulePaths()` for UWP.
  mingw-w64 UWP was okay with this, but MS SDK headers are not.
  This makes `--dump-module-paths` return empty for UWP builds.
  ```
  src\tool_doswin.c(620,3): error C2065: 'MODULEENTRY32': undeclared identifier
  src\tool_doswin.c(626,11): warning C4013: 'CreateToolhelp32Snapshot' undefined; assuming extern returning int
  src\tool_doswin.c(626,36): error C2065: 'TH32CS_SNAPMODULE': undeclared identifier
  src\tool_doswin.c(632,7): warning C4013: 'Module32First' undefined; assuming extern returning int
  ```
  Ref: https://github.com/curl/curl/actions/runs/12055081933/job/33614629930#step:9:35

- examples: fix `websocket.c` to include `winsock2.h` before `windows.h`
  to make it build with MSVC UWP:
  ```
  include\curl\curl.h(143,16): error C2061: syntax error: identifier 'curl_socket_t'
  include\curl\curl.h(143,16): error C2059: syntax error: ';'
  include\curl\curl.h(417,52): error C2146: syntax error: missing ')' before identifier 'curlfd'
  include\curl\curl.h(417,38): error C2081: 'curl_socket_t': name in formal parameter list illegal
  ```
  Ref: https://github.com/curl/curl/actions/runs/12055317910/job/33615644427#step:14:126

- GHA/windows: silence linker warning with MSVC UWP builds:
  ```
  LINK : warning LNK4075: ignoring '/INCREMENTAL' due to '/OPT:ICF' specification
  ```
  Ref: https://github.com/curl/curl/actions/runs/12055696808/job/33616629610#step:11:38

- GHA/windows: set `/INCREMENTAL:NO` for all MSVC jobs to improve
  performance a little.

- cmake: show `UWP` platform flag.

Ref: curl#15652
Closes curl#15657
This was once supported in CMake 2.x and in current 3.x versions is
ignored.

Closes curl#15661
This makes `runtests.pl` run the final executables directly.
Before this patch it called the autotools/libtool wrapper tool, which
then called the final executables.

This solution was already used for `curl.exe`.

Applies to tests run in the `mingw, AM x86_64 c-ares U` job, which still
shows unexplained flakiness.

Also makes tests finish 45 seconds faster.

Ref: curl#14854
Follow-up to 1a2d38c curl#15437
Closes curl#15662
This provides better information on the length of the job and when it
completed.
It was already done in cmake jobs, but not in autotools ones.

Follow-up to 1a2d38c curl#15437
Follow-up to 04184d4 curl#15662

Closes curl#15663
- Restore some necessary options for builds without HTTP and MQTT.

The logic to turn off a segment of options in builds without HTTP and
MQTT was too expansive. Those builds (such as FTP-only builds) could not
use options such as CURLOPT_URL or CURLOPT_USERNAME etc.

Prior to this change 30da1f5 (precedes 8.11.0) refactored the options
processing and caused this issue.

Reported-by: Yoshimasa Ohno

Fixes curl#15634
Closes curl#15640
Reported-by: SuperStormer on github
Fixes curl#15658
Closes curl#15660
Allowing both just creates a transfer with behaviors no user can
properly anticipate so better just deny the combo.

Fixes curl#15646
Reported-by: Harry Sintonen
Closes curl#15666
Add test_02_33 to run with various values for the multi option
CURLMOPT_MAX_HOST_CONNECTIONS and CURLOPT_FRESH_CONNECT to trigger
connection pool limit handling code.

Closes curl#15494
Fix regression that no longer printed the error messages about expired
certificates in openssl. Add test case for openssl/gnutls/wolfssl.

Fixes curl#15612
Reported-by: hiimmat on github
Closes curl#15613
Timestamps in trace logs used a mix of realtime and monotonic time
sources, leading to fractional seconds carrying wrong values. Use
realtime only, so the correct nanoseconds are printed.

Fixes curl#15614
Reported-by: jethrogb on github
Closes curl#15641
Test 481 verifies

Fixes curl#15645
Reported-by: Harry Sintonen
Closes curl#15668
Test 482 verifies

Fixes curl#15645
Reported-by: Harry Sintonen
Closes curl#15668
Support async sftp upload for curl built with libssh.

Closes curl#15625
- ngtcp2/ngtcp2 to v1.9.1
- github/codeql-action digest to f09c1c0
- rustls/rustls-ffi to v0.14.1
- awslabs/aws-lc to v1.40.0

Closes curl#15616
Closes curl#15619
Closes curl#15629
Closes curl#15651
It is a prefix already taken and is used by OpenSSL

Closes curl#15673
... and make it static. As it is not used anywhere else.

Closes curl#15672
For MD4, MD5, and DES

Assisted-by: Viktor Szakats
Closes curl#15650
This moves argument parsing logic for a number of options into sub
functions to reduce the overall complexity of the single getparameter()
function. pmccabe says it takes complexity down from 234 to 147.

The command line options that now has dedicated parser funtions are:

 --continue-at, --ech, --header, --localport, --output, --quote, --range
 --remote-name, --time-cond, --upload-file, --url, --verbose, --writeout

These parsers were selected for thise because they had more than 15
lines of logic in the main switch(). Detected like this:

 git grep -hn 'case C_' tool_getparam.c |
  cut -d: -f1 |
  awk '{if(($1 - prev) > 15) { printf "%d\n", prev;} prev = $1;}'

Closes curl#15680
Other programs (Postman, Chrome, Python request) use a 16 byte cnonce
and there are instances of server-side implementations that don't
support the larger lengths curl used previously.

Fixes curl#15653
Reported-by: Florian Eckert
Closes curl#15670
A step towards a future without sscanf() calls.

Closes curl#15682
In the function for handling 'type=' in the -F command line arguments,
we make the code more lax to accept more strings and thereby also avoid
the use of sscanf().

Closes curl#15683
and switch to str2unummax() for the number parsings

Closes curl#15681
bagder and others added 28 commits January 6, 2025 14:11
Count connections to a host against a possibly configured destination
limit. Trigger multi `connchange` when a connection has been shutdown,
so pending transfers can try to get a connection once again.

Reported-by: baranyaib90 on github
Fixes curl#15857
Closes curl#15879
Bumps [cygwin/cygwin-install-action](https://github.com/cygwin/cygwin-install-action) from 4 to 5.
- [Release notes](https://github.com/cygwin/cygwin-install-action/releases)
- [Commits](cygwin/cygwin-install-action@006ad0b...f61179d)

---
updated-dependencies:
- dependency-name: cygwin/cygwin-install-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Closes curl#15918
MSVC 1900 and older is missing a `const` specifier in the `inet_ntop()`
declaration for the second argument. A workaround was in place for it
in cmake, but it didn't cover all necessary versions.

Replace the workaround with a different one, move it to `lib/inet_ntop.c`
and extend to all necessary MSVC versions.

Also add CI jobs for the older MSVC versions: 2013, 2015, 2017.

Closes curl#15923
- bump cookie counter and "creation time" to use 'unsigned int'
- use BIT() for single-bit struct field
- make invalid_octets() return bool properly

Closes curl#15921
To avoid collision with a 3rd-party `RESERVED` symbols.

This symbol is used in the public header of MSH3 0.7.0.

Closes curl#15929
Starting GHA runner image 20250105.1.0.

As seen on Linux with 0.7.0:
```
/home/runner/msh3/include/msh3.h:377:18: error: width of ‘RESERVED’ exceeds its type
  377 |             bool RESERVED                 : 5;
      |                  ^~~~~~~~
/home/runner/msh3/include/msh3.h:490:18: error: width of ‘RESERVED’ exceeds its type
  490 |             bool RESERVED            : 7;
      |                  ^~~~~~~~
```
https://github.com/curl/curl/actions/runs/12655717818/job/35266716846#step:35:195

Bug: curl#15924 (comment)
Bug: curl#15930 (comment)

Closes curl#15927
Also to align with existing VS2010. Keeping the VS2008 job first to give
a quick sniff test for MSVC builds.

Follow-up to 08ff33e curl#15923
Follow-up to 50f6a6b curl#15926

Closes curl#15932
The `Win64` generator suffix alternative was required by old CMake
versions (<3.1) only:
https://cmake.org/cmake/help/v3.22/generator/Visual%20Studio%2010%202010.html

Closes curl#15935
VS2008 has been partly broken for a while with its shared-debug builds
crashing on startup. Its compiler output (UTF-16 HTML) was also barely
readable even after conversion. It's also the only platform in CI
missing `stdint.h`.

This patch migrates a VS2008 job to VS2010 and drops another that
already had a VS2010 equivalent.

We recommend switching to VS2010 or newer when using MSVC to build curl.

Ref: curl#15907
Closes curl#15934
Adds the experimental feature `ssls-export` to libcurl and curl for
importing and exporting SSL sessions from/to a file.

* add functions to libcurl API
* add command line option `--ssl-sessions <filename>` to curl
* add documenation
* add support in configure
* add support in cmake
+ add pytest case

Closes curl#15924
The TE request header field is invalid in HTTP/2. Since clients may not
know in advance if a connection negotiates HTTP/2, automatically strip
such a header when h2 is in play.

Add test_01_10 to verify.

Reported-by: Jiri Stary
Fixes curl#15941
Closes curl#15943
This discussion:
openssl/openssl#23339 (comment)

Specifically item number 2 (Send Blocking) was raised by the curl team,
noting that SSL_want_write returning false was not a good indicator of
when a stream is writeable. The suggestion in that discussion was to use
SSL_poll with an SSL_POLL_EVENT_W flag instead, as that is a proper
indication of when an SSL_object will allow writing without blocking.

While ssl_want_write updates its state based on the last error
encountered (implying a need to retry an operation to update the
last_error state again), SSL_poll checks stream buffer status during the
call, giving it more up to date information on request. This is the
method used by our guide demos (quic-hq-interop specifically), and it
works well.

This change has been run through the curl test suite, and shown to pass
all tests. However, given the initial problem description I'm not sure
if there is a test case that explicitly checks for blocking and
unblocking of streams. As such some additional testing may be warranted.

Closes curl#15909
The msh3 backed for QUIC and HTTP/3 was introduced in April 2022 but has
never been made to work properly. It has seen no visible traction or
developer activity from the msh3 main author (or anyone else seemingly
interested) in two years. As a non-functional backend, it only adds
friction and "weight" to the development and maintenance.

Meanwhile, we have a fully working backend in the ngtcp2 one and we have
two fully working backends in OpenSSL-QUIC and quiche well on their way
of ending their experimental status in a future.

We remove msh3 support from the curl source tree in July 2025.

Closes curl#15931
We decided last year not to pursue avoiding this warning, because it
adds noise and friction, while in most cases not revealing actual code
issues. We fixed the interesting portion of them throughout mid-2024.

Conclude this effort by deleting related FIXMEs and temporary comments.

Follow-up to 3829759 curl#12489
Closes curl#15939
- autotools: delete stray `VC14_LIB*` references.
- autotools: delete (now) empty `CLEANFILES`.
- autotools: delete no longer used lib/src .inc includes in root makefile.
- autotools: delete stray `cygwinbin` target.
- autotools: delete stray `pkgadd` target (Solaris).
- lib, src: delete stray files from `.gitignore`.
- INSTALL.md: delete reference to non-existing `src/config-win32.h`.
- lib/config-win32ce.h: whitespace.
- lib/config-win32ce.h: sync comments with `config-win32.h`.

Closes curl#15944
- moved the dummy functions into the C file, made them non-static
- added a Curl_trc_ssls dummy

Closes curl#15951
- drop version guard for `__inline`.
  Supported since `_MSC_VER` 1000.
  Visual C++, 32-bit, version 4.0 (1996)

- drop version guard for `__declspec(noreturn)` and `__forceinline`.
  Supported since `_MSC_VER` 1200.
  Visual C++, 32-bit, version 6.0 (1998)

For ancient versions, it's possible to override the default behaviour
by setting these macros via `CPPFLAGS`: `CURL_NORETURN`, `CURL_INLINE`,
`CURL_FORCEINLINE`

Closes curl#15946
It's Visual C++, 32-bit, version 2.0, released in 1993. Used to verify
if `_INTEGRAL_MAX_BITS` is available.

After this patch we assume `_INTEGRAL_MAX_BITS` is always available in
MSVC.

Closes curl#15952
@github-actions
Copy link

CodeSonar analysis

Analysis results are available on the CodeSonar hub

Severity Count
High 2
Medium 1
Low 2
Warning Class Count
File System Race Condition 2
Ignored Return Value 1
Null Pointer Dereference 1
Redundant Condition 1

Generated by CodeSonar

struct ssl_config_data *ssl_config = Curl_ssl_cf_get_config(cf, data);
CURLcode result;

if(!ssl_config || !scache || !ssl_config->primary.cache_session) {

Check warning

Code scanning / CodeSonar

Redundant Condition Warning

ssl_config always evaluates to true. This may be because: - There is a constant assignment to one or more of the variables involved. - An earlier conditional statement has already ensured that ssl_config cannot be false. - A crashing bug occurs on every path where ssl_config could have evaluated to false. Look for a preceding Null Pointer Dereference or Division By Zero warning.
@@ -72,7 +66,7 @@
else {
int fd;
do {
fd = open(fname, O_CREAT | O_WRONLY | O_EXCL | O_BINARY, OPENMODE);
fd = open(fname, O_CREAT | O_WRONLY | O_EXCL | CURL_O_BINARY, OPENMODE);

Check failure

Code scanning / CodeSonar

File System Race Condition Error

The file named fname is accessed again. Another process may have changed the file since the access at tool_cb_wrt.c:68. For example, an attacker could replace the original file with a link to a file containing important or confidential data. - fname evaluates to outs->filename.The issue can occur if the highlighted code executes.
@@ -99,7 +93,8 @@
msnprintf(newname + len + 1, 12, "%d", next_num);
next_num++;
do {
fd = open(newname, O_CREAT | O_WRONLY | O_EXCL | O_BINARY, OPENMODE);
fd = open(newname, O_CREAT | O_WRONLY | O_EXCL | CURL_O_BINARY,

Check failure

Code scanning / CodeSonar

File System Race Condition Error

The file named newname is accessed again. Another process may have changed the file since the access at tool_cb_wrt.c:95. For example, an attacker could replace the original file with a link to a file containing important or confidential data. - newname evaluates to malloc(newlen)[[tool_cb_wrt.c:82]].The issue can occur if the highlighted code executes.
if(!*peasy)
return CURLE_OUT_OF_MEMORY;

result = curl_easy_setopt(*peasy, CURLOPT_SHARE, share);

Check failure

Code scanning / CodeSonar

Ignored Return Value Error

The return value of curl_easy_setopt() is not checked or used in the highlighted execution scenario, although it is in other scenarios. In some situations it may be possible to move the call closer to the first use, past an intervening conditional. - If the return value can indicate an error, the error will be ignored if the highlighted code executes. - The return value of curl_easy_setopt() is checked or used 98% of the time in this project. CodeSonar is configured to enforce Ignored Return Value checks for any function whose return value is checked at least 90% of the time. (To modify the threshold, use configuration file parameters RETURN_CHECKER_RATIO and RETURN_CHECKER_CONFIDENCE. To exempt curl_easy_setopt() from the Ignored Return Value check, use configuration file parameter RETURN_CHECKER_IGNORED_FUNCS).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.