[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH v2] httpd: Pass custom HTTP headers to CGI scripts
From: Alexander Vickberg <wickbergster () gmail ! com>
Date: 2019-03-30 18:40:54
Message-ID: CAAxSGfYeVtLCK6CjM2n-J+VXv10WDMtyj9_YCNCUBssY75tztw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hi Xabier,
Thank you for your comments.
> > diff --git a/networking/httpd.c b/networking/httpd.c
> > index b52526a78..c27cd3001 100644
> > --- a/networking/httpd.c
> > +++ b/networking/httpd.c
> > @@ -2301,32 +2306,8 @@ static void handle_incoming_and_exit(const
len_and_sockaddr *fromAddr)
> > }
> > }
> > #endif
> > -#if ENABLE_FEATURE_HTTPD_CGI
> > - else if (STRNCASECMP(iobuf, "Cookie:") == 0) {
> > - if (!cookie) /* in case they send millions of these, do not OOM */
> > - cookie = xstrdup(skip_whitespace(iobuf + sizeof("Cookie:")-1));
> > - } else if (STRNCASECMP(iobuf, "Content-Type:") == 0) {
> > - if (!content_type)
> > - content_type = xstrdup(skip_whitespace(iobuf +
sizeof("Content-Type:")-1));
> > - } else if (STRNCASECMP(iobuf, "Referer:") == 0) {
> > - if (!G.referer)
> > - G.referer = xstrdup(skip_whitespace(iobuf + sizeof("Referer:")-1));
> > - } else if (STRNCASECMP(iobuf, "User-Agent:") == 0) {
> > - if (!G.user_agent)
> > - G.user_agent = xstrdup(skip_whitespace(iobuf +
sizeof("User-Agent:")-1));
> > - } else if (STRNCASECMP(iobuf, "Host:") == 0) {
> > - if (!G.host)
> > - G.host = xstrdup(skip_whitespace(iobuf + sizeof("Host:")-1));
> > - } else if (STRNCASECMP(iobuf, "Accept:") == 0) {
> > - if (!G.http_accept)
> > - G.http_accept = xstrdup(skip_whitespace(iobuf + sizeof("Accept:")-1));
> > - } else if (STRNCASECMP(iobuf, "Accept-Language:") == 0) {
> > - if (!G.http_accept_language)
> > - G.http_accept_language = xstrdup(skip_whitespace(iobuf +
sizeof("Accept-Language:")-1));
> > - }
> > -#endif
> > #if ENABLE_FEATURE_HTTPD_BASIC_AUTH
> > - if (STRNCASECMP(iobuf, "Authorization:") == 0) {
> > + else if (STRNCASECMP(iobuf, "Authorization:") == 0) {
>
> You cannot convert this to 'else if'.
Doh.. Do you think the approach in my first version would be acceptable?
>
> > /* We only allow Basic credentials.
> > * It shows up as "Authorization: Basic <user>:<passwd>" where
> > * "<user>:<passwd>" is base64 encoded.
> > @@ -2341,7 +2322,7 @@ static void handle_incoming_and_exit(const
len_and_sockaddr *fromAddr)
> > }
> > #endif
> > #if ENABLE_FEATURE_HTTPD_RANGES
> > - if (STRNCASECMP(iobuf, "Range:") == 0) {
> > + else if (STRNCASECMP(iobuf, "Range:") == 0) {
>
> ... Same here.
>
> > /* We know only bytes=NNN-[MMM] */
> > char *s = skip_whitespace(iobuf + sizeof("Range:")-1);
> > if (is_prefixed_with(s, "bytes=")) {
> > @@ -2358,7 +2339,7 @@ static void handle_incoming_and_exit(const
len_and_sockaddr *fromAddr)
> > }
> > #endif
> > #if ENABLE_FEATURE_HTTPD_GZIP
> > - if (STRNCASECMP(iobuf, "Accept-Encoding:") == 0) {
> > + else if (STRNCASECMP(iobuf, "Accept-Encoding:") == 0) {
>
> ... And here.
>
> > /* Note: we do not support "gzip;q=0"
> > * method of _disabling_ gzip
> > * delivery. No one uses that, though */
> > @@ -2373,6 +2354,46 @@ static void handle_incoming_and_exit(const
len_and_sockaddr *fromAddr)
> > //}
> > }
> > }
> > +#endif
> > +#if ENABLE_FEATURE_HTTPD_CGI
> > + else if (STRNCASECMP(iobuf, "Content-Type:") == 0) {
> > + if (!content_type)
> > + content_type = xstrdup(skip_whitespace(iobuf +
sizeof("Content-Type:")-1));
> > + } else {
> > + HTTP_Header *cur;
> > + char *after_colon = strchr(iobuf, ':');
> > + char *ch = iobuf;
> > +
> > + if (!after_colon)
> > + continue;
> > +
> > + cur = xmalloc(sizeof(HTTP_Header));
> > + *after_colon++ = '\0';
> > +
> > + while (*ch) {
> > + if (isalpha(*ch))
> > + *ch &= ~0x20;
> > + else if (!isdigit(*ch))
> > + *ch = '_';
> > + ch++;
> > + }
> > +
> > + cur->name = xmalloc(sizeof("HTTP_")+strlen(iobuf));
> > + memcpy(cur->name, "HTTP_", sizeof("HTTP_")-1);
> > + strcpy(cur->name + sizeof("HTTP_")-1, iobuf);
>
> Why not just:
>
> cur->name = xasprintf("HTTP_%s", iobuf)?
I thought my version would be faster.
> > + cur->value = xstrdup(skip_whitespace(after_colon));
> > +
> > + /* Insert new header into header list */
> > + if (!G.hdr_list) {
> > + G.hdr_list = cur;
> > + cur->next = G.hdr_list;
> > + last = cur;
> > + } else {
> > + cur->next = G.hdr_list;
> > + last->next = cur;
> > + last = cur;
> > + }
> > + }
> > #endif
> > } /* while extra header reading */
> > }
>
> Cheers,
>
> Xabier Oneca_,,_
>
> P.S. Sorry. Gmail ate indentation... :/
/Alexander
[Attachment #5 (text/html)]
<div dir="ltr">Hi Xabier,<div><br></div><div>Thank you for your \
comments.</div><div><br>> > diff --git a/networking/httpd.c \
b/networking/httpd.c<br>> > index b52526a78..c27cd3001 100644<br>> > --- \
a/networking/httpd.c<br>> > +++ b/networking/httpd.c<br>> > @@ -2301,32 \
+2306,8 @@ static void handle_incoming_and_exit(const len_and_sockaddr \
*fromAddr)<br>> > }<br>> > }<br>> > #endif<br>> > \
-#if ENABLE_FEATURE_HTTPD_CGI<br>> > - else if (STRNCASECMP(iobuf, \
"Cookie:") == 0) {<br>> > - if (!cookie) /* in case they send \
millions of these, do not OOM */<br>> > - cookie = \
xstrdup(skip_whitespace(iobuf + sizeof("Cookie:")-1));<br>> > - } \
else if (STRNCASECMP(iobuf, "Content-Type:") == 0) {<br>> > - if \
(!content_type)<br>> > - content_type = xstrdup(skip_whitespace(iobuf + \
sizeof("Content-Type:")-1));<br>> > - } else if (STRNCASECMP(iobuf, \
"Referer:") == 0) {<br>> > - if (!G.referer)<br>> > - G.referer \
= xstrdup(skip_whitespace(iobuf + sizeof("Referer:")-1));<br>> > - } \
else if (STRNCASECMP(iobuf, "User-Agent:") == 0) {<br>> > - if \
(!G.user_agent)<br>> > - G.user_agent = xstrdup(skip_whitespace(iobuf + \
sizeof("User-Agent:")-1));<br>> > - } else if (STRNCASECMP(iobuf, \
"Host:") == 0) {<br>> > - if (!G.host)<br>> > - G.host = \
xstrdup(skip_whitespace(iobuf + sizeof("Host:")-1));<br>> > - } else \
if (STRNCASECMP(iobuf, "Accept:") == 0) {<br>> > - if \
(!G.http_accept)<br>> > - G.http_accept = xstrdup(skip_whitespace(iobuf + \
sizeof("Accept:")-1));<br>> > - } else if (STRNCASECMP(iobuf, \
"Accept-Language:") == 0) {<br>> > - if \
(!G.http_accept_language)<br>> > - G.http_accept_language = \
xstrdup(skip_whitespace(iobuf + sizeof("Accept-Language:")-1));<br>> \
> - }<br>> > -#endif<br>> > #if \
ENABLE_FEATURE_HTTPD_BASIC_AUTH<br>> > - if (STRNCASECMP(iobuf, \
"Authorization:") == 0) {<br>> > + else if (STRNCASECMP(iobuf, \
"Authorization:") == 0) {<br>><br>> You cannot convert this to \
'else if'.</div><div><br></div><div>Doh.. Do you think the approach in my \
first version would be acceptable?</div><div><br>><br>> > /* We only \
allow Basic credentials.<br>> > * It shows up as "Authorization: Basic \
<user>:<passwd>" where<br>> > * \
"<user>:<passwd>" is base64 encoded.<br>> > @@ -2341,7 \
+2322,7 @@ static void handle_incoming_and_exit(const len_and_sockaddr \
*fromAddr)<br>> > }<br>> > #endif<br>> > #if \
ENABLE_FEATURE_HTTPD_RANGES<br>> > - if (STRNCASECMP(iobuf, "Range:") \
== 0) {<br>> > + else if (STRNCASECMP(iobuf, "Range:") == 0) \
{<br>><br>> ... Same here.<br>><br>> > /* We know only \
bytes=NNN-[MMM] */<br>> > char *s = skip_whitespace(iobuf + \
sizeof("Range:")-1);<br>> > if (is_prefixed_with(s, \
"bytes=")) {<br>> > @@ -2358,7 +2339,7 @@ static void \
handle_incoming_and_exit(const len_and_sockaddr *fromAddr)<br>> > }<br>> \
> #endif<br>> > #if ENABLE_FEATURE_HTTPD_GZIP<br>> > - if \
(STRNCASECMP(iobuf, "Accept-Encoding:") == 0) {<br>> > + else if \
(STRNCASECMP(iobuf, "Accept-Encoding:") == 0) {<br>><br>> ... And \
here.<br>><br>> > /* Note: we do not support "gzip;q=0"<br>> \
> * method of _disabling_ gzip<br>> > * delivery. No one uses that, \
though */<br>> > @@ -2373,6 +2354,46 @@ static void \
handle_incoming_and_exit(const len_and_sockaddr *fromAddr)<br>> > \
//}<br>> > }<br>> > }<br>> > +#endif<br>> > +#if \
ENABLE_FEATURE_HTTPD_CGI<br>> > + else if (STRNCASECMP(iobuf, \
"Content-Type:") == 0) {<br>> > + if (!content_type)<br>> > + \
content_type = xstrdup(skip_whitespace(iobuf + \
sizeof("Content-Type:")-1));<br>> > + } else {<br>> > + \
HTTP_Header *cur;<br>> > + char *after_colon = strchr(iobuf, \
':');<br>> > + char *ch = iobuf;<br>> > +<br>> > + if \
(!after_colon)<br>> > + continue;<br>> > +<br>> > + cur = \
xmalloc(sizeof(HTTP_Header));<br>> > + *after_colon++ = '\0';<br>> \
> +<br>> > + while (*ch) {<br>> > + if (isalpha(*ch))<br>> > + \
*ch &= ~0x20;<br>> > + else if (!isdigit(*ch))<br>> > + *ch = \
'_';<br>> > + ch++;<br>> > + }<br>> > +<br>> > + \
cur->name = xmalloc(sizeof("HTTP_")+strlen(iobuf));<br>> > + \
memcpy(cur->name, "HTTP_", sizeof("HTTP_")-1);<br>> > + \
strcpy(cur->name + sizeof("HTTP_")-1, iobuf);<br>><br>> Why not \
just:<br>><br>> cur->name = xasprintf("HTTP_%s", iobuf)?<br><br>I \
thought my version would be faster.<br><br>> > + cur->value = \
xstrdup(skip_whitespace(after_colon));<br>> > +<br>> > + /* Insert new \
header into header list */<br>> > + if (!G.hdr_list) {<br>> > + \
G.hdr_list = cur;<br>> > + cur->next = G.hdr_list;<br>> > + last = \
cur;<br>> > + } else {<br>> > + cur->next = G.hdr_list;<br>> > + \
last->next = cur;<br>> > + last = cur;<br>> > + }<br>> > + \
}<br>> > #endif<br>> > } /* while extra header reading */<br>> \
> }<br>><br>> Cheers,<br>><br>> Xabier Oneca_,,_<br>><br>> \
P.S. Sorry. Gmail ate indentation... \
:/</div><div><br></div><div>/Alexander</div></div>
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic