On Mon, Oct 18, 2021 at 08:27:36PM +0200, Laszlo Ersek wrote:
On 10/15/21 16:17, Richard W.M. Jones wrote:
+#ifndef HAVE_CURLOPT_UNIX_SOCKET_PATH + fprintf (stderr, "%s: curl does not support CURLOPT_UNIX_SOCKET_PATH\n", + argv[0]); + exit (77); +#endif + + sockpath = web_server ("disk", NULL); + if (sockpath == NULL) { + fprintf (stderr, "%s: could not start web server thread\n", argv[0]); + exit (EXIT_FAILURE); + } + + /* Start nbdkit. */ + if (asprintf (&usp_param, "unix-socket-path=%s", sockpath) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE);
Is exit() safe here (with the web server running in another thread)?
I didn't know, but I temporarily modified the test changing #ifndef -> #ifdef above and it did exit (skip) fine.
Furthermore, should we do anything for (e.g.) deleting the pathname of the unix domain socket? (I don't think it matters much; I'm asking this is mostly for my own education.) And then that would apply more or less to every error exit after this point.
I had to go and check this, and it turns out we do unlink the socket in an at-exit handler (cleanup) in tests/web-server.c, so I think we're good even on error paths.
+ } + if (test_start_nbdkit ("--filter=retry-request", + "curl", usp_param, "http://localhost/mirror", + "retry-request-delay=1", + NULL) == -1) + exit (EXIT_FAILURE); + + nbd = nbd_create (); + if (nbd == NULL) { + nbd_error: + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); + exit (EXIT_FAILURE); + }
Can we put this block of code at the very end of the function, and here, just say "goto nbd_error"?
Because right now we have gotos jumping backward, which I find awkward even for (otherwise justified) "algorithmic reasons", and very awkward for a usual error path.
OK, adjusted it as you say. Thanks, Rich.
... Either way:
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks! Laszlo
+ + if (nbd_connect_unix (nbd, sock /* NBD socket */) == -1) + goto nbd_error; + + /* The way the test works is we fetch the magic "/mirror" path (see + * web-server.c). This redirects to /mirror1, /mirror2, /mirror3 + * round robin on each request. /mirror1 returns all 1's, /mirror2 + * returns all 2's, and /mirror3 returns a 404 error. The 404 error + * should be transparently skipped by the filter, so we should see + * alternating 1's and 2's buffers. + */ + if (nbd_pread (nbd, buf, sizeof buf, 0, 0) == -1) + goto nbd_error; + + if (buf[0] == 1 && buf[1] == 1 && buf[511] == 1) + state = 2; + else if (buf[0] == 2 && buf[1] == 2 && buf[511] == 2) + state = 1; + else { + fprintf (stderr, "%s: unexpected start state\n", argv[0]); + exit (EXIT_FAILURE); + } + + for (i = 0; i < 10; ++i) { + if (nbd_pread (nbd, buf, sizeof buf, 0, 0) == -1) + goto nbd_error; + if (buf[0] != state || buf[1] != state || buf[511] != state) { + fprintf (stderr, "%s: unexpected state\n", argv[0]); + exit (EXIT_FAILURE); + } + state++; + if (state == 3) + state = 1; + } + + nbd_close (nbd); + exit (EXIT_SUCCESS); +} diff --git a/tests/web-server.c b/tests/web-server.c index ac44d16b2..07a2ec398 100644 --- a/tests/web-server.c +++ b/tests/web-server.c @@ -57,6 +57,8 @@ #define SOCK_CLOEXEC 0 #endif
+enum method { HEAD, GET }; + static char tmpdir[] = "/tmp/wsXXXXXX"; static char sockpath[] = "............./sock"; static int listen_sock = -1; @@ -67,7 +69,12 @@ static check_request_t check_request;
static void *start_web_server (void *arg); static void handle_requests (int s); -static void handle_request (int s, bool headers_only); +static void handle_file_request (int s, enum method method); +static void handle_mirror_redirect_request (int s); +static void handle_mirror_data_request (int s, enum method method, char byte); +static void send_404_not_found (int s); +static void send_405_method_not_allowed (int s); +static void send_500_internal_server_error (int s); static void xwrite (int s, const char *buf, size_t len); static void xpread (char *buf, size_t count, off_t offset);
@@ -177,12 +184,15 @@ start_web_server (void *arg) static void handle_requests (int s) { - size_t r, n, sz; bool eof = false;
fprintf (stderr, "web server: accepted connection\n");
while (!eof) { + size_t r, n, sz; + enum method method; + char path[128]; + /* Read request until we see "\r\n\r\n" (end of headers) or EOF. */ n = 0; for (;;) { @@ -214,30 +224,73 @@ handle_requests (int s) /* Call the optional user function to check the request. */ if (check_request) check_request (request);
- /* HEAD or GET request? */ - if (strncmp (request, "HEAD ", 5) == 0) - handle_request (s, true); - else if (strncmp (request, "GET ", 4) == 0) - handle_request (s, false); + /* Get the method and path fields from the first line. */ + if (strncmp (request, "HEAD ", 5) == 0) { + method = HEAD; + n = strcspn (&request[5], " \n\t"); + if (n >= sizeof path) { + send_500_internal_server_error (s); + eof = true; + break; + } + memcpy (path, &request[5], n); + path[n] = '\0'; + } + else if (strncmp (request, "GET ", 4) == 0) { + method = GET; + n = strcspn (&request[4], " \n\t"); + if (n >= sizeof path) { + send_500_internal_server_error (s); + eof = true; + break; + } + memcpy (path, &request[4], n); + path[n] = '\0'; + } else { - /* Return 405 Method Not Allowed. */ - const char response[] = - "HTTP/1.1 405 Method Not Allowed\r\n" - "Content-Length: 0\r\n" - "Connection: close\r\n" - "\r\n"; - xwrite (s, response, strlen (response)); + send_405_method_not_allowed (s); eof = true; break; } + + fprintf (stderr, "web server: requested path: %s\n", path); + + /* For testing retry-request + curl: + * /mirror redirects round-robin to /mirror1, /mirror2, /mirror3 + * /mirror1 returns a file of \x01 bytes + * /mirror2 returns a file of \x02 bytes + * /mirror3 returns 404 errors + * Anything else returns a 500 error + */ + if (strcmp (path, "/mirror") == 0) + handle_mirror_redirect_request (s); + else if (strcmp (path, "/mirror1") == 0) + handle_mirror_data_request (s, method, 1); + else if (strcmp (path, "/mirror2") == 0) + handle_mirror_data_request (s, method, 2); + else if (strcmp (path, "/mirror3") == 0) { + send_404_not_found (s); + eof = true; + } + else if (strncmp (path, "/mirror", 7) == 0) { + send_500_internal_server_error (s); + eof = true; + } + + /* Otherwise it's a regular file request. 'path' is ignored, we + * only serve a single file passed to web_server(). + */ + else + handle_file_request (s, method); }
close (s); }
static void -handle_request (int s, bool headers_only) +handle_file_request (int s, enum method method) { + const bool headers_only = method == HEAD; uint64_t offset, length, end; const char *p; const char response1_ok[] = "HTTP/1.1 200 OK\r\n"; @@ -295,6 +348,120 @@ handle_request (int s, bool headers_only) free (data); }
+/* Request for /mirror */ +static void +handle_mirror_redirect_request (int s) +{ + static char rr = '1'; /* round robin '1', '2', '3' */ + /* Note we send 302 (temporary redirect), same as Fedora's mirrorservice. */ + const char found[] = "HTTP/1.1 302 Found\r\nContent-Length: 0\r\n"; + char location[] = "Location: /mirrorX\r\n"; + const char eol[] = "\r\n"; + + location[17] = rr; + rr++; + if (rr == '4') + rr = '1'; + + xwrite (s, found, strlen (found)); + xwrite (s, location, strlen (location)); + xwrite (s, eol, strlen (eol)); +} + +static void +handle_mirror_data_request (int s, enum method method, char byte) +{ + const bool headers_only = method == HEAD; + uint64_t offset, length, end; + const char *p; + const char response1_ok[] = "HTTP/1.1 200 OK\r\n"; + const char response1_partial[] = "HTTP/1.1 206 Partial Content\r\n"; + const char response2[] = + "Accept-rANGES: bytes\r\n" /* See RHBZ#1837337 */ + "Connection: keep-alive\r\n" + "Content-Type: application/octet-stream\r\n"; + char response3[64]; + const char response4[] = "\r\n"; + char *data; + + /* If there's no Range request header then send the full size as the + * content-length. + */ + p = strcasestr (request, "\r\nRange: bytes="); + if (p == NULL) { + offset = 0; + length = statbuf.st_size; + xwrite (s, response1_ok, strlen (response1_ok)); + } + else { + p += 15; + if (sscanf (p, "%" SCNu64 "-%" SCNu64, &offset, &end) != 2) { + fprintf (stderr, "web server: could not parse " + "range request from curl client\n"); + exit (EXIT_FAILURE); + } + /* Unclear but "Range: bytes=0-4" means bytes 0-3. '4' is the + * byte beyond the end of the range. + */ + length = end - offset; + xwrite (s, response1_partial, strlen (response1_partial)); + } + + xwrite (s, response2, strlen (response2)); + snprintf (response3, sizeof response3, + "Content-Length: %" PRIu64 "\r\n", length); + xwrite (s, response3, strlen (response3)); + xwrite (s, response4, strlen (response4)); + + if (headers_only) + return; + + /* Send the file content. */ + data = malloc (length); + if (data == NULL) { + perror ("malloc"); + exit (EXIT_FAILURE); + } + + memset (data, byte, length); + xwrite (s, data, length); + + free (data); +} + +static void +send_404_not_found (int s) +{ + const char response[] = + "HTTP/1.1 404 Not Found\r\n" + "Content-Length: 0\r\n" + "Connection: close\r\n" + "\r\n"; + xwrite (s, response, strlen (response)); +} + +static void +send_405_method_not_allowed (int s) +{ + const char response[] = + "HTTP/1.1 405 Method Not Allowed\r\n" + "Content-Length: 0\r\n" + "Connection: close\r\n" + "\r\n"; + xwrite (s, response, strlen (response)); +} + +static void +send_500_internal_server_error (int s) +{ + const char response[] = + "HTTP/1.1 500 Internal Server Error\r\n" + "Content-Length: 0\r\n" + "Connection: close\r\n" + "\r\n"; + xwrite (s, response, strlen (response)); +} + static void xwrite (int s, const char *buf, size_t len) { diff --git a/.gitignore b/.gitignore index 847b72dd6..e7b07d510 100644 --- a/.gitignore +++ b/.gitignore @@ -155,6 +155,7 @@ plugins/*/*.3 /tests/test-python /tests/test-random /tests/test-readahead +/tests/test-retry-request-mirror /tests/test-ruby /tests/test-S3/boto3/__pycache__/ /tests/test-shell
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v