Conversation
Parse extension names and values before deciding whether a parameter is known. This prevents long names and token or quoted-string values from hiding later ma or persist parameters, as required by RFC 7838 section 3. Add unit coverage for long extension names and both value forms, including an escaped quote and semicolon inside a quoted string.
Member
|
Nice find and work! I found some minor tweaks that could be done to the code and I propose this additional fix:
From bf0419cb5fe9c63147db7a3d2e6b7e30a29e00dc Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Sun, 23 Aug 2026 17:29:04 +0200
Subject: [PATCH] tweaks
---
lib/altsvc.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/lib/altsvc.c b/lib/altsvc.c
index c5dfcd192e..c57d3f6b40 100644
--- a/lib/altsvc.c
+++ b/lib/altsvc.c
@@ -469,10 +469,12 @@ static void altsvc_flush(struct altsvcinfo *asi,
altsvc_free(as);
}
}
}
+#define ALTSVC_MA 1
+#define ALTSVC_PERSIST 2
static void altsvc_parse_params(const char **pp,
time_t *pmaxage,
bool *ppersist)
{
curlx_str_passblanks(pp);
@@ -480,46 +482,43 @@ static void altsvc_parse_params(const char **pp,
return;
for(;;) {
struct Curl_str name;
struct Curl_str val;
- const char *vp;
- const char *vend;
curl_off_t num;
- bool known;
+ int keyword = 0;
/* allow some extra whitespaces around name and value */
if(curlx_str_until(pp, &name, MAX_ALTSVC_LINE, '=') ||
curlx_str_single(pp, '='))
break; /* skip further parameter parsing */
curlx_str_trimblanks(&name);
- known = curlx_str_casecompare(&name, "ma") ||
- curlx_str_casecompare(&name, "persist");
-
curlx_str_passblanks(pp);
if(**pp == '\"') {
if(curlx_str_quotedword(pp, &val, MAX_ALTSVC_LINE))
break;
}
else {
if(curlx_str_cspn(pp, &val, ",;\r\n"))
break;
- curlx_str_trimblanks(&val);
}
+ curlx_str_trimblanks(&val);
+
+ if(curlx_str_casecompare(&name, "ma"))
+ keyword = ALTSVC_MA;
+ else if(curlx_str_casecompare(&name, "persist"))
+ keyword = ALTSVC_PERSIST;
- if(known) {
- vp = curlx_str(&val);
- vend = vp + curlx_strlen(&val);
+ if(keyword) {
+ const char *vp = curlx_str(&val);
+ const char *vend = vp + curlx_strlen(&val);
if(curlx_str_number(&vp, &num, TIME_T_MAX))
break; /* not a number, skip further parameter parsing */
- while((vp < vend) && ISBLANK(*vp))
- vp++;
if(vp != vend)
break; /* not entirely a number, skip further parameter parsing */
-
- if(curlx_str_casecompare(&name, "ma"))
+ if(keyword == ALTSVC_MA)
*pmaxage = (time_t)num;
else if(num == 1)
*ppersist = TRUE;
}
--
2.55.0
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RFC 7838 requires unknown Alt-Svc parameters to be ignored. The parser instead
tried to parse every parameter value as a number and stopped at nonnumeric
extension values, which could silently hide a later
maorpersist.Parse extension names up to the existing Alt-Svc line limit, and parse token
and quoted-string values before validating the two known numeric parameters.
This lets parsing continue after long extension names and quoted values
containing escaped quotes or semicolons.
The regression test verifies the cached expiry, persistence flag, and
destination after both extension value forms. I also ran the complete local
test harness: all 1,671 executable tests passed (2,063 considered).
AI assistance was used for initial code exploration and an independent diff
review; the regression was reproduced and the focused and full curl test
suites were run locally.