Skip to content

lib: separate scheme info from protocol implementation#20351

Closed
bagder wants to merge 4 commits intomasterfrom
bagder/split-handler-protocol
Closed

lib: separate scheme info from protocol implementation#20351
bagder wants to merge 4 commits intomasterfrom
bagder/split-handler-protocol

Conversation

@bagder
Copy link
Member

@bagder bagder commented Jan 19, 2026

This allows builds know about all schemes - but only have the protocol
implementations for those actually built-in.

It further allows multiple protocols to reuse the same protocol setup
and functions for both TLS and non-TLS implementations instead of
needing two (or more) structs.

The scheme information is now in 'struct Curl_scheme' and all the
function pointers for each scheme/protocol implementation are in struct
Curl_protocol.

The URL API now always work with all known protocols.

@bagder bagder force-pushed the bagder/split-handler-protocol branch 2 times, most recently from 9dc3b82 to b488dee Compare January 19, 2026 08:16
@icing
Copy link
Contributor

icing commented Jan 19, 2026

Sounds like a good idea.

@bagder bagder force-pushed the bagder/split-handler-protocol branch 2 times, most recently from b509f8e to d35e447 Compare January 19, 2026 09:32
bagder added a commit that referenced this pull request Jan 19, 2026
This allows builds know about all schemes - but only have the protocol
implementations for those actually built-in. In a next step this should
allow the URL API to always work with all known protocols.

It further allows multiple protocols to reuse the same protocol setup
and functions for both TLS and non-TLS implementations instead of
needing two (or more) structs.

The scheme information is now in 'struct Curl_scheme' and all the
function pointers for each scheme/protocol implementation are in struct
Curl_protocol.

Closes #20351
@bagder bagder force-pushed the bagder/split-handler-protocol branch from 1b51506 to 120dbea Compare January 19, 2026 09:51
@bagder bagder marked this pull request as ready for review January 19, 2026 09:51
@bagder bagder requested a review from Copilot January 19, 2026 09:51
Copy link

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

This PR refactors the cURL library's protocol handling architecture by splitting the Curl_handler struct into two separate structs: Curl_scheme (containing scheme metadata) and Curl_protocol (containing protocol implementation function pointers). This separation allows builds to know about all schemes even when specific protocol implementations are disabled, and enables multiple schemes (like HTTP/HTTPS or WS/WSS) to share the same protocol implementation.

Changes:

  • Split Curl_handler into Curl_scheme and Curl_protocol structs in urldata.h
  • Updated all protocol files to define separate scheme and protocol structs
  • Changed references from conn->handler to conn->scheme and handler->field to scheme->field or scheme->run->field throughout the codebase
  • Simplified scripts/schemetable.c by removing conditional compilation from the scheme table
  • Fixed header guards in vssh files

Reviewed changes

Copilot reviewed 59 out of 59 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/urldata.h Defines new Curl_protocol and Curl_scheme structs, replacing Curl_handler
lib/url.c Updates scheme lookup and connection handling to use new structs
lib/multi.c Updates protocol handler function calls to use conn->scheme->run->*
lib/transfer.c Updates protocol checks to use conn->scheme->protocol
lib/ws.c Splits WebSocket handler into protocol and scheme structs
lib/http.c Splits HTTP/HTTPS handlers, sharing Curl_protocol_http implementation
lib/ftp.c Splits FTP/FTPS handlers, sharing Curl_protocol_ftp implementation
lib/smtp.c, pop3.c, imap.c Email protocols split handlers, share protocol implementations
lib/ldap.c, openldap.c LDAP protocol split across files with shared implementation
lib/vssh/* SSH protocols updated with scheme/protocol split and header guard fixes
scripts/schemetable.c Simplified scheme table generation without conditional compilation

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

@bagder
Copy link
Member Author

bagder commented Jan 19, 2026

augment review

@augmentcode
Copy link

augmentcode bot commented Jan 19, 2026

🤖 Augment PR Summary

Summary: This PR refactors libcurl’s protocol metadata so URL schemes can be known independently of whether the corresponding protocol implementation is compiled in.

Changes:

  • Split the former struct Curl_handler into struct Curl_scheme (name/protocol/family/flags/defport) and struct Curl_protocol (function-pointer vtable).
  • Update struct connectdata to store conn->scheme/conn->given instead of conn->handler.
  • Convert many protocol modules (HTTP, FTP, IMAP/POP3/SMTP, RTSP, TFTP, FILE, DICT, MQTT, SMB, RTMP, WS, LDAP, SSH) to define a protocol vtable plus one-or-more scheme descriptors (e.g., TLS/non-TLS variants reusing the same vtable).
  • Adjust core connection/transfer state machines (multi, transfer, connect, cookie, etc.) to read flags/protocol IDs from the scheme and call implementations via scheme->run.
  • Rename scheme lookup APIs to Curl_get_scheme()/Curl_getn_scheme() and update URL parsing / URL API call sites accordingly.
  • Simplify the generated scheme hash table in url.c to return scheme descriptors.

Technical Notes: This lays groundwork for always recognizing all known schemes (even when implementations are disabled) and for sharing a single protocol implementation across multiple schemes (e.g., TLS/non-TLS variants).

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 3 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

bagder added a commit that referenced this pull request Jan 19, 2026
This allows builds know about all schemes - but only have the protocol
implementations for those actually built-in.

It further allows multiple protocols to reuse the same protocol setup
and functions for both TLS and non-TLS implementations instead of
needing two (or more) structs.

The scheme information is now in 'struct Curl_scheme' and all the
function pointers for each scheme/protocol implementation are in struct
Curl_protocol.

The URL API now always work with all known protocols.

Closes #20351
@bagder bagder force-pushed the bagder/split-handler-protocol branch from 7d12094 to 57b0123 Compare January 19, 2026 10:13
bagder added a commit that referenced this pull request Jan 19, 2026
This allows builds know about all schemes - but only have the protocol
implementations for those actually built-in.

It further allows multiple protocols to reuse the same protocol setup
and functions for both TLS and non-TLS implementations instead of
needing two (or more) structs.

The scheme information is now in 'struct Curl_scheme' and all the
function pointers for each scheme/protocol implementation are in struct
Curl_protocol.

The URL API now always work with all known protocols.

test1560: run without requiring all the protocols

Closes #20351
@bagder bagder force-pushed the bagder/split-handler-protocol branch from 3c78c52 to 0026f1f Compare January 19, 2026 11:55
@github-actions github-actions bot added the tests label Jan 19, 2026
This allows builds know about all schemes - but only have the protocol
implementations for those actually built-in.

It further allows multiple protocols to reuse the same protocol setup
and functions for both TLS and non-TLS implementations instead of
needing two (or more) structs.

The scheme information is now in 'struct Curl_scheme' and all the
function pointers for each scheme/protocol implementation are in struct
Curl_protocol.

The URL API now always work with all known protocols.

test1560: run without requiring all the protocols

Closes #20351
It was a little more complicated than that...
@bagder bagder force-pushed the bagder/split-handler-protocol branch from 0026f1f to 8c4aa33 Compare January 19, 2026 12:08
@bagder bagder closed this in 8edc033 Jan 19, 2026
@bagder bagder deleted the bagder/split-handler-protocol branch January 19, 2026 22:15
vszakats added a commit to vszakats/trurl that referenced this pull request Jan 20, 2026
libcurl 8.19.0 supports all URL schemes even when the protocol itself
is disabled.

Refs:
curl/curl@8edc033
curl/curl#20351
vszakats added a commit to vszakats/curl that referenced this pull request Jan 20, 2026
```
curl/lib/rtsp.c:1073:26: error: no previous extern declaration for non-static variable 'Curl_scheme_rtsp' [-Werror,-Wmissing-variable-declarations]
 1073 | const struct Curl_scheme Curl_scheme_rtsp = {
      |                          ^
curl/lib/rtsp.c:1073:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
 1073 | const struct Curl_scheme Curl_scheme_rtsp = {
      |       ^
```

Follow-up to 8edc033 curl#20351
vszakats added a commit that referenced this pull request Jan 20, 2026
```
lib/rtsp.c:1073:26: error: no previous extern declaration for non-static variable 'Curl_scheme_rtsp' [-Werror,-Wmissing-variable-declarations]
 1073 | const struct Curl_scheme Curl_scheme_rtsp = {
      |                          ^
lib/rtsp.c:1073:7: note: declare 'static' if the variable is not intended to be used outside of this translation unit
 1073 | const struct Curl_scheme Curl_scheme_rtsp = {
      |       ^
```
Ref: https://github.com/curl/trurl/actions/runs/21157411659/job/60844860592?pr=425#step:3:3036

Follow-up to 8edc033 #20351

Closes #20365
vszakats added a commit to curl/trurl that referenced this pull request Jan 20, 2026
libcurl 8.19.0 supports all URL schemes even when the protocol itself
is disabled.

Refs:

curl/curl@8edc033
curl/curl#20351
vszakats added a commit to vszakats/curl that referenced this pull request Feb 5, 2026
vszakats added a commit that referenced this pull request Feb 5, 2026
To prevent inconsistent `CURL_DISABLE_WEBSOCKETS` states between source
files.

Follow-up to 8edc033 #20351

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

Labels

Development

Successfully merging this pull request may close these issues.

4 participants