On Tue, Mar 30, 2021 at 05:45:39PM +0200, Theo Buehler wrote:
> On Tue, Mar 30, 2021 at 05:30:19PM +0200, Claudio Jeker wrote:
> > Found the hard way. http_new() call http_free() if http_resolv() failes.
> > http_free() call http_fail() in that case since the state is not
> > STATE_DONE. In the main poll loop another http_fail() call is made. This
> > results in bad bad things.
>
> Ugh. This is ok if you want to land it separately.
>
> There is also an issue with close(outfd): http_free() closes
> conn->outfd, so you get an EBADF here on http_resolv() failure.
>
> http_new() should probably take ownership of outfd and close it on
> http_parse_uri() failure.
>
> So I think the entire h == NULL case should go away.
>
Like this?
--
:wq Claudio
Index: http.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
retrieving revision 1.11
diff -u -p -r1.11 http.c
--- http.c 29 Mar 2021 15:37:04 -0000 1.11
+++ http.c 30 Mar 2021 15:50:24 -0000
@@ -402,6 +402,7 @@ http_new(size_t id, char *uri, char *mod
if (http_parse_uri(uri, &host, &port, &path) == -1) {
free(uri);
free(modified_since);
+ close(outfd);
return NULL;
}
@@ -1197,10 +1198,7 @@ proc_http(char *bind_addr, int fd)
io_str_read(fd, &mod);
h = http_new(id, uri, mod, outfd);
- if (h == NULL) {
- close(outfd);
- http_fail(id);
- } else
+ if (h != NULL) {
for (i = 0; i < MAX_CONNECTIONS; i++) {
if (http_conns[i] != NULL)
continue;
@@ -1209,6 +1207,7 @@ proc_http(char *bind_addr, int fd)
http_conns[i] = NULL;
break;
}
+ }
}
if (pfds[MAX_CONNECTIONS].revents & POLLOUT) {
switch (msgbuf_write(&msgq)) {
