don't call client_close() from fcgi/proxy bev handlers

We might end up calling client_close() from start_reply(), but that
will free the fcgi/proxy bufferevent while they're still used on the
stack.

Instead, start_reply() only sets REQUEST_DONE and exits, returning the
error eventually, so callers know when to stop.
This commit is contained in:
Omar Polo 2023-08-09 19:13:13 +00:00
parent 01481c255a
commit 390d312b22
4 changed files with 17 additions and 16 deletions

17
fcgi.c
View File

@ -258,8 +258,8 @@ fcgi_handle_stdout(struct client *c, struct evbuffer *src, size_t len)
return; return;
} }
start_reply(c, code, c->sbuf + 3); if (start_reply(c, code, c->sbuf + 3) == -1 ||
if (c->code < 20 || c->code > 29) { c->code < 20 || c->code > 29) {
fcgi_error(bev, EVBUFFER_EOF, c); fcgi_error(bev, EVBUFFER_EOF, c);
return; return;
} }
@ -280,7 +280,7 @@ fcgi_read(struct bufferevent *bev, void *d)
struct fcgi_end_req_body end; struct fcgi_end_req_body end;
size_t len; size_t len;
for (;;) { while (c->type != REQUEST_DONE) {
if (EVBUFFER_LENGTH(src) < sizeof(hdr)) if (EVBUFFER_LENGTH(src) < sizeof(hdr))
return; return;
@ -310,8 +310,7 @@ fcgi_read(struct bufferevent *bev, void *d)
/* TODO: do something with the status? */ /* TODO: do something with the status? */
c->type = REQUEST_DONE; c->type = REQUEST_DONE;
client_write(c->bev, c); break;
return;
case FCGI_STDERR: case FCGI_STDERR:
/* discard stderr (for now) */ /* discard stderr (for now) */
@ -333,6 +332,7 @@ fcgi_read(struct bufferevent *bev, void *d)
err: err:
fcgi_error(bev, EVBUFFER_ERROR, c); fcgi_error(bev, EVBUFFER_ERROR, c);
client_write(c->bev, c);
} }
void void
@ -352,10 +352,12 @@ fcgi_error(struct bufferevent *bev, short err, void *d)
/* /*
* If we're here it means that some kind of non-recoverable * If we're here it means that some kind of non-recoverable
* error happened. * error happened.
*
* Don't free bev as we might be called by a function that
* still uses it.
*/ */
bufferevent_free(bev); bufferevent_disable(bev, EVBUFFER_READ);
c->cgibev = NULL;
close(c->pfd); close(c->pfd);
c->pfd = -1; c->pfd = -1;
@ -367,7 +369,6 @@ fcgi_error(struct bufferevent *bev, short err, void *d)
} }
c->type = REQUEST_DONE; c->type = REQUEST_DONE;
client_write(c->bev, c);
} }
void void

2
gmid.h
View File

@ -409,7 +409,7 @@ int vhost_disable_log(struct vhost*, const char*);
void mark_nonblock(int); void mark_nonblock(int);
void client_write(struct bufferevent *, void *); void client_write(struct bufferevent *, void *);
void start_reply(struct client*, int, const char*); int start_reply(struct client*, int, const char*);
void client_close(struct client *); void client_close(struct client *);
struct client *client_by_id(int); struct client *client_by_id(int);
void server_accept(int, short, void *); void server_accept(int, short, void *);

View File

@ -182,9 +182,8 @@ proxy_read(struct bufferevent *bev, void *d)
return; return;
} }
start_reply(c, code, hdr + 3); if (start_reply(c, code, hdr + 3) == -1 ||
c->code < 20 || c->code > 29) {
if (c->code < 20 || c->code > 29) {
proxy_error(bev, EVBUFFER_EOF, c); proxy_error(bev, EVBUFFER_EOF, c);
return; return;
} }

View File

@ -1122,7 +1122,7 @@ client_error(struct bufferevent *bev, short error, void *d)
client_close(c); client_close(c);
} }
void int
start_reply(struct client *c, int code, const char *meta) start_reply(struct client *c, int code, const char *meta)
{ {
struct evbuffer *evb = EVBUFFER_OUTPUT(c->bev); struct evbuffer *evb = EVBUFFER_OUTPUT(c->bev);
@ -1161,18 +1161,19 @@ start_reply(struct client *c, int code, const char *meta)
if (code != 20) if (code != 20)
c->type = REQUEST_DONE; c->type = REQUEST_DONE;
return; return 0;
err: err:
log_warnx("evbuffer_add_printf error: no memory"); log_warnx("evbuffer_add_printf error: no memory");
evbuffer_drain(evb, EVBUFFER_LENGTH(evb)); evbuffer_drain(evb, EVBUFFER_LENGTH(evb));
client_close(c); c->type = REQUEST_DONE;
return; return -1;
overflow: overflow:
log_warnx("reply header overflow"); log_warnx("reply header overflow");
evbuffer_drain(evb, EVBUFFER_LENGTH(evb)); evbuffer_drain(evb, EVBUFFER_LENGTH(evb));
start_reply(c, TEMP_FAILURE, "internal error"); start_reply(c, TEMP_FAILURE, "internal error");
return -1;
} }
static void static void