Groups keyboard shortcuts have been updated
Dismiss
See shortcuts

[PATCH] delta_handler: fix sensitive case when parsing header

54 views
Skip to first unread message

Adrian Freihofer

unread,
Dec 18, 2024, 9:41:12 AM12/18/24
to swup...@googlegroups.com, Patriarche Hervé
From: Patriarche Hervé <herve.pa...@wattsense.com>

Keep header case to allow correct comparaison when compare to body.

Signed-off-by: Patriarche Hervé <herve.pa...@wattsense.com>
---
handlers/delta_handler.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
index c7a57a3c..2039413e 100644
--- a/handlers/delta_handler.c
+++ b/handlers/delta_handler.c
@@ -642,15 +642,13 @@ static bool parse_headers(struct hnd_priv *priv)

range_answer_t *answer = priv->answer;
answer->data[answer->len] = '\0';
- /* Converto to lower case to make comparison easier */
- string_tolower(answer->data);

/* Check for multipart */
nconv = sscanf(answer->data, "%ms %ms %ms", &header, &value, &boundary_string);

if (nconv == 3) {
- if (!strncmp(header, "content-type", strlen("content-type")) &&
- !strncmp(boundary_string, "boundary", strlen("boundary"))) {
+ if (!strncasecmp(header, "content-type", strlen("content-type")) &&
+ !strncasecmp(boundary_string, "boundary", strlen("boundary"))) {
pair = string_split(boundary_string, '=');
cnt = count_string_array((const char **)pair);
if (cnt == 2) {
@@ -661,8 +659,8 @@ static bool parse_headers(struct hnd_priv *priv)
free_string_array(pair);
}

- if (!strncmp(header, "content-range", strlen("content-range")) &&
- !strncmp(value, "bytes", strlen("bytes"))) {
+ if (!strncasecmp(header, "content-range", strlen("content-range")) &&
+ !strncasecmp(value, "bytes", strlen("bytes"))) {
pair = string_split(boundary_string, '-');
priv->range_type = SINGLE_RANGE;
size_t start = strtoul(pair[0], NULL, 10);
@@ -701,7 +699,7 @@ static bool search_boundary_in_body(struct hnd_priv *priv)
}
s = answer->data;
for (i = 0; i < answer->len; i++, s++) {
- if (!strncmp(s, priv->boundary, strlen(priv->boundary))) {
+ if (!strncasecmp(s, priv->boundary, strlen(priv->boundary))) {
DEBUG("Boundary found in body");
/* Reset buffer to start from here */
if (i != 0)
--
2.47.1

Stefano Babic

unread,
Dec 18, 2024, 9:46:42 AM12/18/24
to Adrian Freihofer, swup...@googlegroups.com, Patriarche Hervé
Hi Patriarche,

is there a better description about the issue you find ? Apart that
comparison is case insensitive and now data is first converted to lower
case, I do not understand if this solves an issue.

Best regards,
Stefano Babic

Hervé Patriarche

unread,
Dec 19, 2024, 8:09:45 AM12/19/24
to swupdate
Hi Stephano,

This is related to the content of the boundary field. When boundary field contains only numbers (which is the case with a standard apache server) there is no issue.
But when boundary field contains string like --CloudFront:85440c5677f30266d3a14df53e855e3e then header record boundary in lower case and when search_boundary_in_body function compare it to the body nothing is found as boundary field in body is with uppercase.
The patch aim to keep boundary field from header without converting it to lowercase.

Hope this clarify.

Best regards,

Hervé Patriarche.

Alexander Broekhuis

unread,
Jan 28, 2025, 2:46:41 AMJan 28
to swupdate
Hi Stefano/Hervé,

We are running into the same issue, to elaborate a bit on it. What happens is, is that the header is lowercased and parsed after that. So if there are alphabetic uppercase characters in the boundary, it turns into the lowercase version. However, where the multipart body is parsed, it does a compare without any conversion, so for the given example, it compares "cloudfront" (lowercase) with "Cloudfront" (uppercase).

Can this patch be applied?

Thanks!

Op donderdag 19 december 2024 om 14:09:45 UTC+1 schreef Hervé Patriarche:

Stefano Babic

unread,
Jan 28, 2025, 3:15:37 AMJan 28
to Alexander Broekhuis, swupdate
Hi,

On 28.01.25 08:46, Alexander Broekhuis wrote:
> Hi Stefano/Hervé,
>
> We are running into the same issue, to elaborate a bit on it. What
> happens is, is that the header is lowercased and parsed after that. So
> if there are alphabetic uppercase characters in the boundary, it turns
> into the lowercase version. However, where the multipart body is parsed,
> it does a compare without any conversion, so for the given example, it
> compares "cloudfront" (lowercase) with "Cloudfront" (uppercase).
>
> Can this patch be applied?
>

Patch is ok, I will apply it soon.

Best regards,
Stefano Babic
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/
> swupdate/b4a44ffa-653d-4cba-ac41-208b79e5d719n%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/b4a44ffa-653d-4cba-
> ac41-208b79e5d719n%40googlegroups.com?utm_medium=email&utm_source=footer>.

Alexander Broekhuis

unread,
Jan 28, 2025, 3:18:59 AMJan 28
to swupdate
Looking at the patch, I do wonder if the last change (in the search_boundary_in_body function) is needed. Since the boundary is kept as is, it can be argued that the webserver should also use the same formatting for the boundary in the body, making a case insensitive compare unnecessary . But I don't see any technical issues the way it is now.

Op dinsdag 28 januari 2025 om 09:15:37 UTC+1 schreef Stefano Babic:

Stefano Babic

unread,
Jan 28, 2025, 3:47:05 AMJan 28
to Alexander Broekhuis, swupdate
On 1/28/25 09:18, Alexander Broekhuis wrote:
> Looking at the patch, I do wonder if the last change (in
> the search_boundary_in_body function) is needed. Since the boundary is
> kept as is, it can be argued that the webserver should also use the same
> formatting for the boundary in the body, making a case insensitive
> compare unnecessary . But I don't see any technical issues the way it is
> now.

Right, I do not see any issue with the change and it is then the same
approach in the whole boundary comparison.

Best regards,
Stefano
> <https://groups.google.com/d/msgid/>
> > swupdate/b4a44ffa-653d-4cba-ac41-208b79e5d719n%40googlegroups.com
> <http://40googlegroups.com>
> > <https://groups.google.com/d/msgid/swupdate/b4a44ffa-653d-4cba-
> <https://groups.google.com/d/msgid/swupdate/b4a44ffa-653d-4cba->
> > ac41-208b79e5d719n%40googlegroups.com?
> utm_medium=email&utm_source=footer <http://40googlegroups.com?
> utm_medium=email&utm_source=footer>>.
>
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/
> swupdate/f5902d32-0724-465a-ac6b-41e25ac1d76bn%40googlegroups.com
> <https://groups.google.com/d/msgid/swupdate/f5902d32-0724-465a-
> ac6b-41e25ac1d76bn%40googlegroups.com?utm_medium=email&utm_source=footer>.

Reply all
Reply to author
Forward
0 new messages