From ed1f181013894bc32aeaa8b88cb971aee2254208 Mon Sep 17 00:00:00 2001 From: User Date: Thu, 1 May 2025 09:33:25 +0300 Subject: [PATCH] moved retry logic into multi instead of the connection itself Update tests/data/Makefile.am Co-authored-by: Viktor Szakats Apply suggestions from code review whitespace stuff Co-authored-by: Viktor Szakats more whitspace fixes more docs changed pull request fixes (note that CI requires that symbols-in-versions be something that it should not be) moving to new curl version better documentation try and see maybe async is destroyed properly now? removed whitespace Co-authored-by: Viktor Szakats Update docs/libcurl/opts/CURLOPT_ALTSVC_CTRL.md "-" uses insteadvof space Co-authored-by: Viktor Szakats better handeling of the recovering and checking whether or not to recover --- docs/libcurl/opts/CURLOPT_ALTSVC_CTRL.md | 7 +- docs/libcurl/symbols-in-versions | 2 +- lib/altsvc.c | 2 - lib/altsvc.h | 1 + lib/multi.c | 61 +++++++ lib/url.c | 47 ++--- tests/data/Makefile.am | 3 +- tests/{breaking_test => data/test3301} | 6 + tests/libtest/Makefile.inc | 2 +- tests/libtest/lib3303 | 210 ----------------------- tests/libtest/lib3303.c | 5 +- 11 files changed, 94 insertions(+), 252 deletions(-) rename tests/{breaking_test => data/test3301} (88%) delete mode 100755 tests/libtest/lib3303 diff --git a/docs/libcurl/opts/CURLOPT_ALTSVC_CTRL.md b/docs/libcurl/opts/CURLOPT_ALTSVC_CTRL.md index 9024bd84fd..02f0269902 100644 --- a/docs/libcurl/opts/CURLOPT_ALTSVC_CTRL.md +++ b/docs/libcurl/opts/CURLOPT_ALTSVC_CTRL.md @@ -55,7 +55,12 @@ Setting any bit enables the alt-svc engine. If the connection fails when trying to connect to an alternative host, the hostname from the URL is attempted instead. -Failed connection problems that trigger this fallback include: ... +Failed connection problems that trigger this fallback: +1. bad addresses such as .onion +2. non-responsive servers +3. bad SSL +4. unsupported protocols +5. response appears to be the wrong protocol ## CURLALTSVC_READONLYFILE diff --git a/docs/libcurl/symbols-in-versions b/docs/libcurl/symbols-in-versions index e91cb6da73..5eed93efef 100644 --- a/docs/libcurl/symbols-in-versions +++ b/docs/libcurl/symbols-in-versions @@ -199,7 +199,7 @@ CURLALTSVC_H1 7.64.1 CURLALTSVC_H2 7.64.1 CURLALTSVC_H3 7.64.1 CURLALTSVC_READONLYFILE 7.64.1 -CURLALTSVC_NO_RETRY 8.14.0 +CURLALTSVC_NO_RETRY 8.15.0 CURLAUTH_ANY 7.10.6 CURLAUTH_ANYSAFE 7.10.6 CURLAUTH_AWS_SIGV4 7.75.0 diff --git a/lib/altsvc.c b/lib/altsvc.c index 1c9128ff79..a06f40bc2a 100644 --- a/lib/altsvc.c +++ b/lib/altsvc.c @@ -303,8 +303,6 @@ struct altsvcinfo *Curl_altsvc_init(void) return NULL; Curl_llist_init(&asi->list, NULL); - asi->used = FALSE; - /* set default behavior */ asi->flags = CURLALTSVC_H1 #ifdef USE_HTTP2 diff --git a/lib/altsvc.h b/lib/altsvc.h index 6225a1c297..597b954630 100644 --- a/lib/altsvc.h +++ b/lib/altsvc.h @@ -49,6 +49,7 @@ struct altsvcinfo { struct Curl_llist list; /* list of entries */ long flags; /* the publicly set bitmask */ + BIT(errored); BIT(used); }; diff --git a/lib/multi.c b/lib/multi.c index 1c09b14b6b..b325a95b39 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -58,6 +58,7 @@ #include "curl_printf.h" #include "curl_memory.h" #include "memdebug.h" +#include "altsvc.h" /* initial multi->xfers table size for a full multi */ #define CURL_XFER_TABLE_SIZE 512 @@ -2327,6 +2328,33 @@ static CURLMcode state_connect(struct Curl_multi *multi, return rc; } +#ifndef CURL_DISABLE_ALTSVC +static bool is_altsvc_error(CURLcode rc) +{ + switch(rc) { + case CURLE_URL_MALFORMAT: + case CURLE_COULDNT_RESOLVE_PROXY: + case CURLE_COULDNT_RESOLVE_HOST: + case CURLE_COULDNT_CONNECT: + case CURLE_HTTP2: + case CURLE_HTTP2_STREAM: + case CURLE_HTTP3: + case CURLE_QUIC_CONNECT_ERROR: + case CURLE_SSL_CONNECT_ERROR: + case CURLE_GOT_NOTHING: + case CURLE_SEND_ERROR: + case CURLE_RECV_ERROR: + case CURLE_PROXY: + case CURLE_ECH_REQUIRED: + case CURLE_BAD_CONTENT_ENCODING: + return true; + + default: + return false; + } +} +#endif + static CURLMcode multi_runsingle(struct Curl_multi *multi, struct curltime *nowp, struct Curl_easy *data) @@ -2412,6 +2440,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, FALLTHROUGH(); case MSTATE_CONNECT: + +#ifndef CURL_DISABLE_ALTSVC +do_connect: +#endif rc = state_connect(multi, data, nowp, &result); break; @@ -2642,6 +2674,35 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, } statemachine_end: + /* maybe retry if altsvc is breaking */ +#ifndef CURL_DISABLE_ALTSVC + if( + result && + data->asi && data->asi->used && !data->asi->errored + ) { + data->asi->errored = is_altsvc_error(result); + + if(data->asi->errored && + !(data->asi->flags & CURLALTSVC_NO_RETRY) && + data->mstate <= MSTATE_PROTOCONNECTING && + data->mstate >= MSTATE_CONNECT) { + infof(data, "Alt-Svc connection failed(%d). " + "Retrying with original target", result); + + if(data->conn && stream_error) { + bool dead_connection = result == CURLE_OPERATION_TIMEDOUT; + struct connectdata *conn = data->conn; + + Curl_detach_connection(data); + Curl_conn_terminate(data, conn, dead_connection); + } + + stream_error = FALSE; + multistate(data, MSTATE_CONNECT); + goto do_connect; + } + } +#endif if(data->mstate < MSTATE_COMPLETED) { if(result) { diff --git a/lib/url.c b/lib/url.c index f8d153864b..3dc7dfc2db 100644 --- a/lib/url.c +++ b/lib/url.c @@ -3113,7 +3113,8 @@ static CURLcode parse_connect_to_slist(struct Curl_easy *data, } #ifndef CURL_DISABLE_ALTSVC - if(data->asi && !host && (port == -1) && + /* only use altsvc if its the first time we tried it */ + if(data->asi && !host && (port == -1) && !data->asi->errored && ((conn->handler->protocol == CURLPROTO_HTTPS) || #ifdef DEBUGBUILD /* allow debug builds to circumvent the HTTPS restriction */ @@ -3177,6 +3178,9 @@ static CURLcode parse_connect_to_slist(struct Curl_easy *data, char *hostd = strdup((char *)as->dst.host); if(!hostd) return CURLE_OUT_OF_MEMORY; + + data->asi->used = TRUE; + conn->conn_to_host.rawalloc = hostd; conn->conn_to_host.name = hostd; conn->bits.conn_to_host = TRUE; @@ -3426,8 +3430,7 @@ static void conn_meta_freeentry(void *p) static CURLcode create_conn(struct Curl_easy *data, struct connectdata **in_connect, - bool *reusedp, - bool *use_slist) + bool *reusedp) { CURLcode result = CURLE_OK; struct connectdata *conn; @@ -3538,13 +3541,9 @@ static CURLcode create_conn(struct Curl_easy *data, * Process the "connect to" linked list of hostname/port mappings. * Do this after the remote port number has been fixed in the URL. *************************************************************/ - if(!data->asi || !data->asi->used) { - result = parse_connect_to_slist(data, conn, data->set.connect_to); - if(data->asi) - data->asi->used = TRUE; - if(result) - goto out; - } + result = parse_connect_to_slist(data, conn, data->set.connect_to); + if(result) + goto out; /************************************************************* * IDN-convert the proxy hostnames @@ -3839,6 +3838,12 @@ CURLcode Curl_setup_conn(struct Curl_easy *data, DEBUGASSERT(dns); Curl_pgrsTime(data, TIMER_NAMELOOKUP); + if(conn->handler->flags & PROTOPT_NONETWORK) { + /* nothing to setup when not using a network */ + *protocol_done = TRUE; + return result; + } + if(!conn->bits.reuse) result = Curl_conn_setup(data, conn, FIRSTSOCKET, dns, CURL_CF_SSL_DEFAULT); @@ -3857,7 +3862,6 @@ CURLcode Curl_connect(struct Curl_easy *data, CURLcode result; struct connectdata *conn; bool reused = FALSE; - bool use_slist = TRUE; /* start by attempting to use the slist */ #ifndef CURL_DISABLE_ALTSVC if(data->asi) @@ -3877,27 +3881,6 @@ CURLcode Curl_connect(struct Curl_easy *data, DEBUGASSERT(!conn); return result; } - result = create_conn(data, &conn, asyncp, &use_slist); - -#ifndef CURL_DISABLE_ALTSVC - /* if we failed because of the avc cache retry */ - if(result && data-> asi - && data->asi->used - && !(data-> asi-> flags & CURLALTSVC_NO_RETRY) - ) { - - infof(data, "Alt-Svc connection failed, retrying with original target"); - - - if(conn && result != CURLE_NO_CONNECTION_AVAILABLE) { - Curl_detach_connection(data); - Curl_conn_terminate(data, conn, TRUE); - } - - Curl_req_hard_reset(&data->req, data); - result = create_conn(data, &conn, asyncp); - } -#endif if(!result) { DEBUGASSERT(conn); diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index d8237773b7..1db144f3b1 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -281,8 +281,7 @@ test3200 test3201 test3202 test3203 test3204 test3205 test3207 test3208 \ test3209 test3210 test3211 test3212 test3213 test3214 test3215 \ test3209 test3210 test3211 test3212 test3213 test3214 \ \ -test3300 test3301 test3302 test3303\ -\ +test3300 test3301 test3302 test3303 \ test4000 test4001 EXTRA_DIST = $(TESTCASES) DISABLED diff --git a/tests/breaking_test b/tests/data/test3301 similarity index 88% rename from tests/breaking_test rename to tests/data/test3301 index a2b51b6106..36e2f35b3b 100644 --- a/tests/breaking_test +++ b/tests/data/test3301 @@ -68,6 +68,12 @@ Content-Length: 6 -foo- +CONNECT %HOSTIP:%NOLISTENPORT HTTP/1.1 +Host: %HOSTIP:%NOLISTENPORT +User-Agent: curl/%VERSION +Proxy-Connection: Keep-Alive + +[DISCONNECT] GET http://test.remote.haxx.se.%TESTNUMBER:8990/ HTTP/1.1 Host: test.remote.haxx.se.%TESTNUMBER:8990 User-Agent: curl/%VERSION diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc index 84a8a00777..9f92478639 100644 --- a/tests/libtest/Makefile.inc +++ b/tests/libtest/Makefile.inc @@ -107,4 +107,4 @@ TESTS_C = \ lib3010.c lib3025.c lib3026.c lib3027.c lib3033.c lib3034.c \ lib3100.c lib3101.c lib3102.c lib3103.c lib3104.c lib3105.c \ lib3207.c lib3208.c \ - lib3303.c \ No newline at end of file + lib3303.c diff --git a/tests/libtest/lib3303 b/tests/libtest/lib3303 deleted file mode 100755 index 77cbbac37f..0000000000 --- a/tests/libtest/lib3303 +++ /dev/null @@ -1,210 +0,0 @@ -#! /bin/bash - -# lib3303 - temporary wrapper script for .libs/lib3303 -# Generated by libtool (GNU libtool) 2.4.7 Debian-2.4.7-7build1 -# -# The lib3303 program cannot be directly executed until all the libtool -# libraries that it depends on are installed. -# -# This wrapper script should never be moved out of the build directory. -# If it is, it will not operate correctly. - -# Sed substitution that helps us do robust quoting. It backslashifies -# metacharacters that are still active within double-quoted strings. -sed_quote_subst='s|\([`"$\\]\)|\\\1|g' - -# Be Bourne compatible -if test -n "${ZSH_VERSION+set}" && (emulate sh) >/dev/null 2>&1; then - emulate sh - NULLCMD=: - # Zsh 3.x and 4.x performs word splitting on ${1+"$@"}, which - # is contrary to our usage. Disable this feature. - alias -g '${1+"$@"}'='"$@"' - setopt NO_GLOB_SUBST -else - case `(set -o) 2>/dev/null` in *posix*) set -o posix;; esac -fi -BIN_SH=xpg4; export BIN_SH # for Tru64 -DUALCASE=1; export DUALCASE # for MKS sh - -# The HP-UX ksh and POSIX shell print the target directory to stdout -# if CDPATH is set. -(unset CDPATH) >/dev/null 2>&1 && unset CDPATH - -relink_command="" - -# This environment variable determines our operation mode. -if test "$libtool_install_magic" = "%%%MAGIC variable%%%"; then - # install mode needs the following variables: - generated_by_libtool_version='2.4.7' - notinst_deplibs=' ../../lib/libcurl.la' -else - # When we are sourced in execute mode, $file and $ECHO are already set. - if test "$libtool_execute_magic" != "%%%MAGIC variable%%%"; then - file="$0" - -# A function that is used when there is no print builtin or printf. -func_fallback_echo () -{ - eval 'cat <<_LTECHO_EOF -$1 -_LTECHO_EOF' -} - ECHO="printf %s\\n" - fi - -# Very basic option parsing. These options are (a) specific to -# the libtool wrapper, (b) are identical between the wrapper -# /script/ and the wrapper /executable/ that is used only on -# windows platforms, and (c) all begin with the string --lt- -# (application programs are unlikely to have options that match -# this pattern). -# -# There are only two supported options: --lt-debug and -# --lt-dump-script. There is, deliberately, no --lt-help. -# -# The first argument to this parsing function should be the -# script's ../../libtool value, followed by no. -lt_option_debug= -func_parse_lt_options () -{ - lt_script_arg0=$0 - shift - for lt_opt - do - case "$lt_opt" in - --lt-debug) lt_option_debug=1 ;; - --lt-dump-script) - lt_dump_D=`$ECHO "X$lt_script_arg0" | /usr/bin/sed -e 's/^X//' -e 's%/[^/]*$%%'` - test "X$lt_dump_D" = "X$lt_script_arg0" && lt_dump_D=. - lt_dump_F=`$ECHO "X$lt_script_arg0" | /usr/bin/sed -e 's/^X//' -e 's%^.*/%%'` - cat "$lt_dump_D/$lt_dump_F" - exit 0 - ;; - --lt-*) - $ECHO "Unrecognized --lt- option: '$lt_opt'" 1>&2 - exit 1 - ;; - esac - done - - # Print the debug banner immediately: - if test -n "$lt_option_debug"; then - echo "lib3303:lib3303:$LINENO: libtool wrapper (GNU libtool) 2.4.7 Debian-2.4.7-7build1" 1>&2 - fi -} - -# Used when --lt-debug. Prints its arguments to stdout -# (redirection is the responsibility of the caller) -func_lt_dump_args () -{ - lt_dump_args_N=1; - for lt_arg - do - $ECHO "lib3303:lib3303:$LINENO: newargv[$lt_dump_args_N]: $lt_arg" - lt_dump_args_N=`expr $lt_dump_args_N + 1` - done -} - -# Core function for launching the target application -func_exec_program_core () -{ - - if test -n "$lt_option_debug"; then - $ECHO "lib3303:lib3303:$LINENO: newargv[0]: $progdir/$program" 1>&2 - func_lt_dump_args ${1+"$@"} 1>&2 - fi - exec "$progdir/$program" ${1+"$@"} - - $ECHO "$0: cannot exec $program $*" 1>&2 - exit 1 -} - -# A function to encapsulate launching the target application -# Strips options in the --lt-* namespace from $@ and -# launches target application with the remaining arguments. -func_exec_program () -{ - case " $* " in - *\ --lt-*) - for lt_wr_arg - do - case $lt_wr_arg in - --lt-*) ;; - *) set x "$@" "$lt_wr_arg"; shift;; - esac - shift - done ;; - esac - func_exec_program_core ${1+"$@"} -} - - # Parse options - func_parse_lt_options "$0" ${1+"$@"} - - # Find the directory that this script lives in. - thisdir=`$ECHO "$file" | /usr/bin/sed 's%/[^/]*$%%'` - test "x$thisdir" = "x$file" && thisdir=. - - # Follow symbolic links until we get to the real thisdir. - file=`ls -ld "$file" | /usr/bin/sed -n 's/.*-> //p'` - while test -n "$file"; do - destdir=`$ECHO "$file" | /usr/bin/sed 's%/[^/]*$%%'` - - # If there was a directory component, then change thisdir. - if test "x$destdir" != "x$file"; then - case "$destdir" in - [\\/]* | [A-Za-z]:[\\/]*) thisdir="$destdir" ;; - *) thisdir="$thisdir/$destdir" ;; - esac - fi - - file=`$ECHO "$file" | /usr/bin/sed 's%^.*/%%'` - file=`ls -ld "$thisdir/$file" | /usr/bin/sed -n 's/.*-> //p'` - done - - # Usually 'no', except on cygwin/mingw when embedded into - # the cwrapper. - WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no - if test "$WRAPPER_SCRIPT_BELONGS_IN_OBJDIR" = "yes"; then - # special case for '.' - if test "$thisdir" = "."; then - thisdir=`pwd` - fi - # remove .libs from thisdir - case "$thisdir" in - *[\\/].libs ) thisdir=`$ECHO "$thisdir" | /usr/bin/sed 's%[\\/][^\\/]*$%%'` ;; - .libs ) thisdir=. ;; - esac - fi - - # Try to get the absolute directory name. - absdir=`cd "$thisdir" && pwd` - test -n "$absdir" && thisdir="$absdir" - - program='lib3303' - progdir="$thisdir/.libs" - - - if test -f "$progdir/$program"; then - # Add our own library path to LD_LIBRARY_PATH - LD_LIBRARY_PATH="/home/user/Desktop/open_source_work/curl/lib/.libs:$LD_LIBRARY_PATH" - - # Some systems cannot cope with colon-terminated LD_LIBRARY_PATH - # The second colon is a workaround for a bug in BeOS R4 sed - LD_LIBRARY_PATH=`$ECHO "$LD_LIBRARY_PATH" | /usr/bin/sed 's/::*$//'` - - export LD_LIBRARY_PATH - - if test "$libtool_execute_magic" != "%%%MAGIC variable%%%"; then - # Run the actual program with our arguments. - func_exec_program ${1+"$@"} - fi - else - # The program doesn't exist. - $ECHO "$0: error: '$progdir/$program' does not exist" 1>&2 - $ECHO "This script is just a wrapper for $program." 1>&2 - $ECHO "See the libtool documentation for more information." 1>&2 - exit 1 - fi -fi diff --git a/tests/libtest/lib3303.c b/tests/libtest/lib3303.c index 98058d0c4e..ac2d4319fb 100644 --- a/tests/libtest/lib3303.c +++ b/tests/libtest/lib3303.c @@ -21,13 +21,12 @@ * SPDX-License-Identifier: curl * ***************************************************************************/ - -/* testing the CURLALTSVC_NO_RETRY flag */ - #include "first.h" #include "memdebug.h" +/* testing the CURLALTSVC_NO_RETRY flag */ + static CURLcode test_lib3303(char *URL) { CURL *curl;