[PATCH v2] httpd: Pass custom HTTP headers to CGI scripts

[Xabier Oneca -- xOneca]

Hi Alex,

Nice work! Some comments follow:

> 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'.

>   /* 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)?

> + 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... :/