On 26/01/2026 01:05, Gianluca Cannata wrote:
Hi Amos,
thank you for your feedback.
I have tried to address all of your comments.
Let me know what you think.
Gianluca
Index: httpfs/http.c
===================================================================
--- httpfs.orig/http.c
+++ httpfs/http.c
@@ -34,6 +34,8 @@
#include <hurd/hurd_types.h>
#include <hurd/netfs.h>
+#define MAX_HTTP_STATUS_LINE 256
+
Still need to document that this value is relatively arbitrary. We only
use the first 13 bytes of the first line.
Technically the HTTP "Reason Phrase" string is unlimited length.
/* do a DNS lookup for HOST and store result in *DEST */
error_t lookup_host (const char *host, struct sockaddr_storage *dest, socklen_t *dest_len, int *socktype, int *protocol)
{
@@ -100,34 +102,73 @@ error_t lookup_host (const char *host, s
return 0;
}
-/* read the first line from the socket and return an error_t error */
+/* read the first line from the socket or return an error_t error */
static error_t translate_http_status (int fd, ssize_t *nread)
{
- char buf[32];
+ char buf[MAX_HTTP_STATUS_LINE];
+ size_t i = 0;
+ ssize_t n;
+ char c;
+ int status;
+
+ *nread = 0;
+
+ /* 1. byte-per-byte read until \n */
+ while (i < sizeof (buf) - 1) {
+ n = read (fd, &c, 1);
+ if (n <= 0) return EIO;
+ buf[i++] = c;
+ if (c == '\n') break;
+ }
I suspect this is heading in a bad direction. AFAIK, system calls are
quite expensive.
We know that a minimum of 13 bytes are needed for this function, so you
can start with that many and reduce by the amount actually received
until that much is read in.
After which, you can do whatever to discard the remainder of the line.
No need to even put that part in buf.
AFAICS the header are currently being dropped so a bulk read to drop the
status as well is fine ... for now.
- *nread = read (fd, buf, sizeof (buf) - 1);
- if (*nread < 12) return EIO;
- buf[*nread] = '\0';
+ buf[i] = '\0';
+ *nread = i;
- if (strncmp (buf, "HTTP/", 5) != 0)
- return EPROTO;
+ /* 2. Positional validation: "HTTP/1.N SSS" */
+ int valid = (i >= 12) &&
+ (strncmp (buf, "HTTP/1.", 7) == 0) && /* "HTTP/1." */
We know that buf has more than 7 bytes/octets.
Use memcmp() rather than strncmp().
+ (buf[7] >= '0' && buf[7] <= '9') && /* N (digit) */
+ (buf[8] == ' ') && /* SP */
+ (buf[9] >= '0' && buf[9] <= '9') && /* S (digit 1) */
+ (buf[10] >= '0' && buf[10] <= '9') && /* S (digit 2) */
+ (buf[11] >= '0' && buf[11] <= '9') && /* S (digit 3) */
+ (buf[12] == ' ' || buf[12] == '\r' || buf[12] == '\n');
/* End of line */
Sadly bare-CR is a thing from malicious services. We do have to check
for "\r\n" together instead of just an '\r'.
So IMO the above should be:
(i >= 13) &&
(memcmp (buf, "HTTP/1.", 7) == 0 && isdigit(buf[7])) && // HTTP version
(buf[8] == ' ') && // SP delimiter
(buf[9] >= '1' && buf[9] <= '5' && isdigit(buf[10]) &&
isdigit(buf[10])) && // status code
(buf[12] == ' ' || (buf[12] == '\r' && buf[13] == 'n') || buf[12] ==
'\n'); // SP delimiter or End of Line
Note, with the change to which characters buf[9] is compared against we
are no longer needing the extra (status < 100 || status >= 600) check below.
- int status = atoi (buf + 9);
+ if (!valid) return EPROTO;
+
+ /* 3. HTTP status code conversion */
+ status = atoi (buf + 9);
if (debug_flag)
- fprintf (stderr, "HTTP Status: %d\n", status);
+ fprintf (stderr, "HTTP Protocol Verified. Status: %d\n",
status);
+
+ /* 4. RFC 9110 - POSIX mappings */
+ if (status < 100 || status >= 600) return EPROTO;
+ if (status < 200) return EIO; /* 1xx not supported */
+ if (status < 300) return 0; /* 2xx Success */
+
+ if (status < 400) { /* 3xx Redirection */
+ switch (status) {
+ case 301: case 302: case 303: case 307: case 308:
return EAGAIN;
+ default: return EIO;
+ }
+ }
+
+ if (status < 500) { /* 4xx Client Errors */
+ switch (status) {
+ case 401: case 402: case 403: case 405:
+ case 407: case 421: case 451: return EACCES;
+ case 404: case 410: case 406: case 412: case 415: case
428: return ENOENT;
+ case 426: return EPROTO;
+ default: return EINVAL;
+ }
+ }
- switch (status)
- {
- case 200: return 0;
- case 301:
- case 302: return EAGAIN;
- case 401:
- case 403: return EACCES;
- case 404: return ENOENT;
- case 410: return ENOENT;
- default:
- return (status >= 500) ? EIO : EINVAL;
+ /* 5xx Server Errors */
+ switch (status) {
+ case 501: case 505: case 510: return EINVAL;
+ case 503: return EACCES;
+ default: return EIO;
}
}
