HeyWe got this bug reported with a pull request, but I didn't like how the fix was made so I wrote up a different take on the same problem.
It changes how we store protocols in the protocol handlers so I figured I give everyone a chance to read and comment before I proceed and push.
Comments? -- / daniel.haxx.se
From 9e08444aa03b7bff2c487bf14af372e7db03b837 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg <[email protected]> Date: Sun, 20 Apr 2014 19:37:54 +0200 Subject: [PATCH] handler: make 'protocol' always specified as a single bit This makes the findprotocol() function work as intended so that libcurl can properly be restricted to not support HTTP while still supporting HTTPS - since the HTTPS handler previously set both the HTTP and HTTPS bits in the protocol field. This fixes --proto and --proto-redir for most SSL protocols. This is done by adding a few new convenience defines that groups HTTP and HTTPS, FTP andFTPS etc that should then be used when the code wants to check for both protocols at once. Bug: https://github.com/bagder/curl/pull/97 Reported-by: drizzt --- lib/ftp.c | 2 +- lib/http.c | 8 ++++---- lib/http2.c | 2 +- lib/imap.c | 2 +- lib/ldap.c | 4 ++-- lib/pop3.c | 2 +- lib/sendf.c | 4 ++-- lib/smtp.c | 2 +- lib/transfer.c | 18 +++++++++--------- lib/url.c | 6 +++--- lib/urldata.h | 11 ++++++++++- 11 files changed, 35 insertions(+), 26 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index 8ce57a6..e8ac363 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -206,11 +206,11 @@ const struct Curl_handler Curl_handler_ftps = { ftp_domore_getsock, /* domore_getsock */ ZERO_NULL, /* perform_getsock */ ftp_disconnect, /* disconnect */ ZERO_NULL, /* readwrite */ PORT_FTPS, /* defport */ - CURLPROTO_FTP | CURLPROTO_FTPS, /* protocol */ + CURLPROTO_FTPS, /* protocol */ PROTOPT_SSL | PROTOPT_DUAL | PROTOPT_CLOSEACTION | PROTOPT_NEEDSPWD | PROTOPT_NOURLQUERY /* flags */ }; #endif diff --git a/lib/http.c b/lib/http.c index 1a2799f..08d9032 100644 --- a/lib/http.c +++ b/lib/http.c @@ -142,11 +142,11 @@ const struct Curl_handler Curl_handler_https = { ZERO_NULL, /* domore_getsock */ ZERO_NULL, /* perform_getsock */ ZERO_NULL, /* disconnect */ ZERO_NULL, /* readwrite */ PORT_HTTPS, /* defport */ - CURLPROTO_HTTP | CURLPROTO_HTTPS, /* protocol */ + CURLPROTO_HTTPS, /* protocol */ PROTOPT_SSL | PROTOPT_CREDSPERREQUEST /* flags */ }; #endif @@ -1769,11 +1769,11 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) if(!data->state.first_host) return CURLE_OUT_OF_MEMORY; } http->writebytecount = http->readbytecount = 0; - if((conn->handler->protocol&(CURLPROTO_HTTP|CURLPROTO_FTP)) && + if((conn->handler->protocol&(PROTO_HTTPFAM|CURLPROTO_FTP)) && data->set.upload) { httpreq = HTTPREQ_PUT; } /* Now set the 'request' pointer to the proper request string */ @@ -1881,11 +1881,11 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) /* Some kind of TE is requested, check if 'chunked' is chosen */ data->req.upload_chunky = Curl_compareheader(ptr, "Transfer-Encoding:", "chunked"); } else { - if((conn->handler->protocol&CURLPROTO_HTTP) && + if((conn->handler->protocol&PROTO_HTTPFAM) && data->set.upload && (data->set.infilesize == -1)) { if(conn->bits.authneg) /* don't enable chunked during auth neg */ ; @@ -3188,11 +3188,11 @@ CURLcode Curl_http_readwrite_headers(struct SessionHandle *data, return res; #else #define HEADER1 k->p /* no conversion needed, just use k->p */ #endif /* CURL_DOES_CONVERSIONS */ - if(conn->handler->protocol & CURLPROTO_HTTP) { + if(conn->handler->protocol & PROTO_HTTPFAM) { nc = sscanf(HEADER1, " HTTP/%d.%d %3d", &httpversion_major, &conn->httpversion, &k->httpcode); diff --git a/lib/http2.c b/lib/http2.c index aa2db43..75956a0 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -134,11 +134,11 @@ const struct Curl_handler Curl_handler_http2_ssl = { ZERO_NULL, /* domore_getsock */ http2_perform_getsock, /* perform_getsock */ http2_disconnect, /* disconnect */ ZERO_NULL, /* readwrite */ PORT_HTTP, /* defport */ - CURLPROTO_HTTP | CURLPROTO_HTTPS, /* protocol */ + CURLPROTO_HTTPS, /* protocol */ PROTOPT_SSL /* flags */ }; /* * Store nghttp2 version info in this buffer, Prefix with a space. Return diff --git a/lib/imap.c b/lib/imap.c index 9808f99..fc162b6 100644 --- a/lib/imap.c +++ b/lib/imap.c @@ -153,11 +153,11 @@ const struct Curl_handler Curl_handler_imaps = { ZERO_NULL, /* domore_getsock */ ZERO_NULL, /* perform_getsock */ imap_disconnect, /* disconnect */ ZERO_NULL, /* readwrite */ PORT_IMAPS, /* defport */ - CURLPROTO_IMAP | CURLPROTO_IMAPS, /* protocol */ + CURLPROTO_IMAPS, /* protocol */ PROTOPT_CLOSEACTION | PROTOPT_SSL | PROTOPT_NEEDSPWD /* flags */ }; #endif diff --git a/lib/ldap.c b/lib/ldap.c index c2fa173..4c98789 100644 --- a/lib/ldap.c +++ b/lib/ldap.c @@ -3,11 +3,11 @@ * Project ___| | | | _ \| | * / __| | | | |_) | | * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2013, Daniel Stenberg, <[email protected]>, et al. + * Copyright (C) 1998 - 2014, Daniel Stenberg, <[email protected]>, et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms * are also available at http://curl.haxx.se/docs/copyright.html. * @@ -157,11 +157,11 @@ const struct Curl_handler Curl_handler_ldaps = { ZERO_NULL, /* domore_getsock */ ZERO_NULL, /* perform_getsock */ ZERO_NULL, /* disconnect */ ZERO_NULL, /* readwrite */ PORT_LDAPS, /* defport */ - CURLPROTO_LDAP | CURLPROTO_LDAPS, /* protocol */ + CURLPROTO_LDAPS, /* protocol */ PROTOPT_SSL /* flags */ }; #endif diff --git a/lib/pop3.c b/lib/pop3.c index d33ca0f..964804b 100644 --- a/lib/pop3.c +++ b/lib/pop3.c @@ -154,11 +154,11 @@ const struct Curl_handler Curl_handler_pop3s = { ZERO_NULL, /* domore_getsock */ ZERO_NULL, /* perform_getsock */ pop3_disconnect, /* disconnect */ ZERO_NULL, /* readwrite */ PORT_POP3S, /* defport */ - CURLPROTO_POP3 | CURLPROTO_POP3S, /* protocol */ + CURLPROTO_POP3S, /* protocol */ PROTOPT_CLOSEACTION | PROTOPT_SSL | PROTOPT_NOURLQUERY /* flags */ }; #endif diff --git a/lib/sendf.c b/lib/sendf.c index 1c30c0b..a6795db 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -3,11 +3,11 @@ * Project ___| | | | _ \| | * / __| | | | |_) | | * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2013, Daniel Stenberg, <[email protected]>, et al. + * Copyright (C) 1998 - 2014, Daniel Stenberg, <[email protected]>, et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms * are also available at http://curl.haxx.se/docs/copyright.html. * @@ -420,11 +420,11 @@ CURLcode Curl_client_write(struct connectdata *conn, return CURLE_OK; } if(type & CLIENTWRITE_BODY) { - if((conn->handler->protocol&CURLPROTO_FTP) && + if((conn->handler->protocol&PROTO_FTPFAM) && conn->proto.ftpc.transfertype == 'A') { /* convert from the network encoding */ CURLcode rc = Curl_convert_from_network(data, ptr, len); /* Curl_convert_from_network calls failf if unsuccessful */ if(rc) diff --git a/lib/smtp.c b/lib/smtp.c index 9512a2a..d367d94 100644 --- a/lib/smtp.c +++ b/lib/smtp.c @@ -153,11 +153,11 @@ const struct Curl_handler Curl_handler_smtps = { ZERO_NULL, /* domore_getsock */ ZERO_NULL, /* perform_getsock */ smtp_disconnect, /* disconnect */ ZERO_NULL, /* readwrite */ PORT_SMTPS, /* defport */ - CURLPROTO_SMTP | CURLPROTO_SMTPS, /* protocol */ + CURLPROTO_SMTPS, /* protocol */ PROTOPT_CLOSEACTION | PROTOPT_SSL | PROTOPT_NOURLQUERY /* flags */ }; #endif diff --git a/lib/transfer.c b/lib/transfer.c index 3fcc600..088f869 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -97,11 +97,11 @@ CURLcode Curl_fillreadbuffer(struct connectdata *conn, int bytes, int *nreadp) size_t buffersize = (size_t)bytes; int nread; #ifdef CURL_DOES_CONVERSIONS bool sending_http_headers = FALSE; - if(conn->handler->protocol&(CURLPROTO_HTTP|CURLPROTO_RTSP)) { + if(conn->handler->protocol&(PROTO_HTTPFAM|CURLPROTO_RTSP)) { const struct HTTP *http = data->req.protop; if(http->sending == HTTPSEND_REQUEST) /* We're sending the HTTP request headers, not the data. Remember that so we don't re-translate them into garbage. */ @@ -317,11 +317,11 @@ static int data_pending(const struct connectdata *conn) content-length is provided, curl waits for the connection close, which we emulate it using conn->proto.httpc.closed = TRUE. The thing is if we read everything, then http2_recv won't be called and we cannot signal the HTTP/2 stream has closed. As a workaround, we return nonzero here to call http2_recv. */ - ((conn->handler->protocol&CURLPROTO_HTTP) && conn->httpversion == 20 && + ((conn->handler->protocol&PROTO_HTTPFAM) && conn->httpversion == 20 && conn->proto.httpc.closed); #else Curl_ssl_data_pending(conn, FIRSTSOCKET); #endif } @@ -525,11 +525,11 @@ static CURLcode readwrite_data(struct SessionHandle *data, #ifndef CURL_DISABLE_HTTP if(0 == k->bodywrites && !is_empty_data) { /* These checks are only made the first time we are about to write a piece of the body */ - if(conn->handler->protocol&(CURLPROTO_HTTP|CURLPROTO_RTSP)) { + if(conn->handler->protocol&(PROTO_HTTPFAM|CURLPROTO_RTSP)) { /* HTTP-only checks */ if(data->req.newurl) { if(conn->bits.close) { /* Abort after the headers if "follow Location" is set @@ -721,11 +721,11 @@ static CURLcode readwrite_data(struct SessionHandle *data, Content-Encoding header. See Curl_readwrite_init; the memset() call initializes k->auto_decoding to zero. */ if(!k->ignorebody) { #ifndef CURL_DISABLE_POP3 - if(conn->handler->protocol&CURLPROTO_POP3) + if(conn->handler->protocol&PROTO_POP3FAM) result = Curl_pop3_write(conn, k->str, nread); else #endif /* CURL_DISABLE_POP3 */ result = Curl_client_write(conn, CLIENTWRITE_BODY, k->str, @@ -852,11 +852,11 @@ static CURLcode readwrite_upload(struct SessionHandle *data, /* set a timeout for the multi interface */ Curl_expire(data, data->set.expect_100_timeout); break; } - if(conn->handler->protocol&(CURLPROTO_HTTP|CURLPROTO_RTSP)) { + if(conn->handler->protocol&(PROTO_HTTPFAM|CURLPROTO_RTSP)) { if(http->sending == HTTPSEND_REQUEST) /* We're sending the HTTP request headers, not the data. Remember that so we don't change the line endings. */ sending_http_headers = TRUE; else @@ -890,11 +890,11 @@ static CURLcode readwrite_upload(struct SessionHandle *data, /* store number of bytes available for upload */ data->req.upload_present = nread; #ifndef CURL_DISABLE_SMTP - if(conn->handler->protocol & CURLPROTO_SMTP) { + if(conn->handler->protocol & PROTO_SMTPFAM) { result = Curl_smtp_escape_eob(conn, nread); if(result) return result; } else @@ -1871,11 +1871,11 @@ CURLcode Curl_retry_request(struct connectdata *conn, *url = NULL; /* if we're talking upload, we can't do the checks below, unless the protocol is HTTP as when uploading over HTTP we will still get a response */ if(data->set.upload && - !(conn->handler->protocol&(CURLPROTO_HTTP|CURLPROTO_RTSP))) + !(conn->handler->protocol&(PROTO_HTTPFAM|CURLPROTO_RTSP))) return CURLE_OK; if(/* workaround for broken TLS servers */ data->state.ssl_connect_retry || ((data->req.bytecount + data->req.headerbytecount == 0) && @@ -1897,11 +1897,11 @@ CURLcode Curl_retry_request(struct connectdata *conn, prevent i.e HTTP transfers to return error just because nothing has been transferred! */ - if(conn->handler->protocol&CURLPROTO_HTTP) { + if(conn->handler->protocol&PROTO_HTTPFAM) { struct HTTP *http = data->req.protop; if(http->writebytecount) return Curl_readrewind(conn); } } @@ -1970,11 +1970,11 @@ Curl_setup_transfer( Thus, we must check if the request has been sent before we set the state info where we wait for the 100-return code */ if((data->state.expect100header) && - (conn->handler->protocol&CURLPROTO_HTTP) && + (conn->handler->protocol&PROTO_HTTPFAM) && (http->sending == HTTPSEND_BODY)) { /* wait with write until we either got 100-continue or a timeout */ k->exp100 = EXP100_AWAITING_CONTINUE; k->start100 = Curl_tvnow(); diff --git a/lib/url.c b/lib/url.c index b871bd6..9a4e5b0 100644 --- a/lib/url.c +++ b/lib/url.c @@ -2701,11 +2701,11 @@ static bool SocketIsDead(curl_socket_t sock) } static bool IsPipeliningPossible(const struct SessionHandle *handle, const struct connectdata *conn) { - if((conn->handler->protocol & CURLPROTO_HTTP) && + if((conn->handler->protocol & PROTO_HTTPFAM) && Curl_multi_pipeline_enabled(handle->multi) && (handle->set.httpreq == HTTPREQ_GET || handle->set.httpreq == HTTPREQ_HEAD) && handle->set.httpversion != CURL_HTTP_VERSION_1_0) return TRUE; @@ -2925,11 +2925,11 @@ ConnectionExists(struct SessionHandle *data, struct connectdata *check; struct connectdata *chosen = 0; bool canPipeline = IsPipeliningPossible(data, needle); bool wantNTLMhttp = ((data->state.authhost.want & CURLAUTH_NTLM) || (data->state.authhost.want & CURLAUTH_NTLM_WB)) && - (needle->handler->protocol & CURLPROTO_HTTP) ? TRUE : FALSE; + (needle->handler->protocol & PROTO_HTTPFAM) ? TRUE : FALSE; struct connectbundle *bundle; *force_reuse = FALSE; /* We can't pipe if the site is blacklisted */ @@ -5328,11 +5328,11 @@ static CURLcode create_conn(struct SessionHandle *data, result = CURLE_UNSUPPORTED_PROTOCOL; goto out; #else /* force this connection's protocol to become HTTP if not already compatible - if it isn't tunneling through */ - if(!(conn->handler->protocol & CURLPROTO_HTTP) && + if(!(conn->handler->protocol & PROTO_HTTPFAM) && !conn->bits.tunnel_proxy) conn->handler = &Curl_handler_http; conn->bits.httpproxy = TRUE; #endif diff --git a/lib/urldata.h b/lib/urldata.h index 19c79cf..1f79384 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -56,10 +56,18 @@ #define DICT_DEFINE3 "/LOOKUP:" #define CURL_DEFAULT_USER "anonymous" #define CURL_DEFAULT_PASSWORD "[email protected]" +/* Convenience defines for checking protocols or their SSL based + version. 'Fam' is short for 'family'. Each protocol handler should only + ever have a single CURLPROTO_ in its protocol field. */ +#define PROTO_HTTPFAM (CURLPROTO_HTTP|CURLPROTO_HTTPS) +#define PROTO_FTPFAM (CURLPROTO_FTP|CURLPROTO_FTPS) +#define PROTO_POP3FAM (CURLPROTO_POP3|CURLPROTO_POP3S) +#define PROTO_SMTPFAM (CURLPROTO_SMTP|CURLPROTO_SMTPS) + #define DEFAULT_CONNCACHE_SIZE 5 /* length of longest IPv6 address string including the trailing null */ #define MAX_IPADR_LEN sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255") @@ -775,11 +783,12 @@ struct Curl_handler { allow the protocol to do extra reads/writes */ CURLcode (*readwrite)(struct SessionHandle *data, struct connectdata *conn, ssize_t *nread, bool *readmore); long defport; /* Default port. */ - unsigned int protocol; /* See CURLPROTO_* */ + unsigned int protocol; /* See CURLPROTO_* - this needs to be the single + specific protocol bit */ unsigned int flags; /* Extra particular characteristics, see PROTOPT_* */ }; #define PROTOPT_NONE 0 /* nothing extra */ #define PROTOPT_SSL (1<<0) /* uses SSL */ -- 1.9.2
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
