Skip to content

connection reuse: starttls#21665

Closed
icing wants to merge 3 commits into
curl:masterfrom
icing:conn-reuse-starttls
Closed

connection reuse: starttls#21665
icing wants to merge 3 commits into
curl:masterfrom
icing:conn-reuse-starttls

Conversation

@icing
Copy link
Copy Markdown
Contributor

@icing icing commented May 19, 2026

Add test_31_13 to check connection reuse on mixed --ssl-reqd setting. For that add debug env var CURL_DBG_NO_USE_SSL_ON_FIRST to disable --ssl-reqd for the first url. Check that the connection without SSL from the first url is not reused on the second URL that requires it.

Tweak special ftp: protocol check to fail a DEBUGASSERT on mismatched use_ssl settings as that should have been caught before in the connection reuse matching (imap/smtp etc. do not have this extra check and rely on the general part doing its job).

Add test_31_13 to check connection reuse on mixed --ssl-reqd setting.
For that add debug env var CURL_DBG_NO_USE_SSL_ON_FIRST to disable
--ssl-reqd for the first url. Check that the connection without SSL
from the first url is not reused on the second URL that requires it.

Tweak special ftp: protocol check to fail a DEBUGASSERT on mismatched
`use_ssl` settings as that should have been caught before in the
connection reuse matching (imap/smtp etc. do not have this extra check
and rely on the general part doing its job).
@icing icing requested a review from bagder May 19, 2026 09:44
@icing
Copy link
Copy Markdown
Contributor Author

icing commented May 19, 2026

@bagder this is related to the report you mentioned yesterday.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a test for FTP connection reuse with mixed --ssl-reqd settings, and refactors where the SSL-use match is enforced during connection reuse so the check now applies regardless of scheme match. A debug-only env var is introduced to coerce the first URL to skip --ssl-reqd, enabling the new test scenario. The ftp code switches to reading use_ssl from ftp_conn (per-connection) instead of data->set, and the FTP connection-match function turns a previous symmetric use_ssl mismatch into a DEBUGASSERT-guarded sanity check, relying on the centralized url_match_ssl_use() to do the real filtering.

Changes:

  • Move the url_match_ssl_use() call out of url_match_destination() into url_match_conn() so STARTTLS reuse mismatches are caught for all schemes.
  • Use per-connection ftpc->use_ssl (instead of data->set.use_ssl) in FTP control/PROT/PBSZ logic and drop the symmetric use_ssl equality in ftp_conns_match() in favor of a DEBUGASSERT for the unsafe direction.
  • Add CURL_DBG_NO_USE_SSL_ON_FIRST debug env var (set via config2setopts) plus docs and a new test_31_13_starttls_reuse Python test.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/url.c Relocates the SSL-use match from destination matching to the main per-connection matcher.
lib/ftp.c Reads use_ssl from ftp_conn and replaces strict equality with a one-sided DEBUGASSERT.
src/config2setopts.c Adds debug-only override that clears CURLOPT_USE_SSL on the first URL when env var set.
docs/libcurl/libcurl-env-dbg.md Documents the new CURL_DBG_NO_USE_SSL_ON_FIRST debug env var.
tests/http/test_31_vsftpds.py Adds test_31_13_starttls_reuse to verify no reuse across mixed STARTTLS requirements.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@icing icing requested a review from bagder May 26, 2026 14:30
@bagder bagder closed this in 4ff212f May 31, 2026
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
Add test_31_13 to check connection reuse on mixed --ssl-reqd setting.
For that add debug env var CURL_DBG_NO_USE_SSL_ON_FIRST to disable
--ssl-reqd for the first url. Check that the connection without SSL
from the first url is not reused on the second URL that requires it.

Tweak special ftp: protocol check to fail a DEBUGASSERT on mismatched
`use_ssl` settings as that should have been caught before in the
connection reuse matching (imap/smtp etc. do not have this extra check
and rely on the general part doing its job).

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

Development

Successfully merging this pull request may close these issues.

3 participants