Claudio Jeker([email protected]) on 2021.11.04 18:43:10 +0100:
> On Thu, Nov 04, 2021 at 11:27:46AM -0600, Theo de Raadt wrote:
> > Claudio Jeker <[email protected]> wrote:
> >
> > > This diff replaces the errx() call in the poll fd check with warnings plus
> > > an exit of the main event loop. It also prints an error in case not all
> > > files have been processed.
> > >
> > > An example after kill -9 of the rsync process is:
> > > rpki-client: https://rrdp.lacnic.net/rrdp/notification.xml: loaded from
> > > network
> > > rpki-client: poll[1]: hangup
> > > rpki-client: rsync terminated signal 9
> >
> > I am not thrilled with giving people error messages about a system call
> > (poll).
> >
> > In this specific case, the actual issue is on the next like (it is a rsync
> > failure, which you caused I suspect)
> >
> > Is that 2nd message not enough?
> >
> > Could you set hangup = 1, but skip the warnx, so that the code just reacts
> > correctly?
> >
> > Are there other circumstances (different types of fd), which do not make
> > it to a wait (rrdp?). Can those report a nice termination message?
> >
>
> I agree that the hangup message is not very helpful. It is actually
> the one message I did not alter. I'm happy to remove that message.
> For the errors from msgbuf_write() I think those are valid error messages
> and I feel the POLLERR|POLLNVAL fall into the same boat.
>
> Updated diff below
i'm fine with that
ok benno@
> --
> :wq Claudio
>
> Index: http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 http.c
> --- http.c 4 Nov 2021 14:24:41 -0000 1.48
> +++ http.c 4 Nov 2021 14:25:03 -0000
> @@ -159,7 +159,7 @@ static uint8_t *tls_ca_mem;
> static size_t tls_ca_size;
>
> /* HTTP request API */
> -static void http_req_new(size_t, char *, char *, int);
> +static void http_req_new(size_t, char *, char *, int, int);
> static void http_req_free(struct http_request *);
> static void http_req_done(size_t, enum http_result, const char *);
> static void http_req_fail(size_t);
> @@ -507,7 +507,7 @@ http_resolv(struct addrinfo **res, const
> * Create and queue a new request.
> */
> static void
> -http_req_new(size_t id, char *uri, char *modified_since, int outfd)
> +http_req_new(size_t id, char *uri, char *modified_since, int count, int
> outfd)
> {
> struct http_request *req;
> char *host, *port, *path;
> @@ -530,6 +530,7 @@ http_req_new(size_t id, char *uri, char
> req->path = path;
> req->uri = uri;
> req->modified_since = modified_since;
> + req->redirect_loop = count;
>
> TAILQ_INSERT_TAIL(&queue, req, entry);
> }
> @@ -1135,7 +1136,8 @@ http_redirect(struct http_connection *co
> err(1, NULL);
>
> logx("redirect to %s", http_info(uri));
> - http_req_new(conn->req->id, uri, mod_since, outfd);
> + http_req_new(conn->req->id, uri, mod_since, conn->req->redirect_loop,
> + outfd);
>
> /* clear request before moving connection to idle */
> http_req_free(conn->req);
> @@ -1867,7 +1869,7 @@ proc_http(char *bind_addr, int fd)
> io_read_str(b, &mod);
>
> /* queue up new requests */
> - http_req_new(id, uri, mod, b->fd);
> + http_req_new(id, uri, mod, 0, b->fd);
> ibuf_free(b);
> }
> }
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.162
> diff -u -p -r1.162 main.c
> --- main.c 4 Nov 2021 14:24:41 -0000 1.162
> +++ main.c 4 Nov 2021 17:42:42 -0000
> @@ -1020,19 +1020,23 @@ main(int argc, char *argv[])
> }
>
> for (i = 0; i < NPFD; i++) {
> - if (pfd[i].revents & (POLLERR|POLLNVAL))
> - errx(1, "poll[%zu]: bad fd", i);
> - if (pfd[i].revents & POLLHUP) {
> - warnx("poll[%zu]: hangup", i);
> + if (pfd[i].revents & (POLLERR|POLLNVAL)) {
> + warnx("poll[%zu]: bad fd", i);
> hangup = 1;
> }
> + if (pfd[i].revents & POLLHUP)
> + hangup = 1;
> if (pfd[i].revents & POLLOUT) {
> switch (msgbuf_write(queues[i])) {
> case 0:
> - errx(1, "write[%zu]: "
> + warnx("write[%zu]: "
> "connection closed", i);
> + hangup = 1;
> + break;
> case -1:
> - err(1, "write[%zu]", i);
> + warn("write[%zu]", i);
> + hangup = 1;
> + break;
> }
> }
> }
> @@ -1147,7 +1151,7 @@ main(int argc, char *argv[])
>
> /* processing did not finish because of error */
> if (entity_queue != 0)
> - return 1;
> + errx(1, "not all files processed, giving up");
>
> logx("all files parsed: generating output");
>
> Index: rrdp_delta.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rrdp_delta.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 rrdp_delta.c
> --- rrdp_delta.c 3 Nov 2021 13:30:56 -0000 1.5
> +++ rrdp_delta.c 4 Nov 2021 09:20:13 -0000
> @@ -225,6 +225,16 @@ delta_content_handler(void *data, const
> PARSE_FAIL(p, "parse failed - content too big");
> }
>
> +static void
> +delta_doctype_handler(void *data, const char *doctypeName,
> + const char *sysid, const char *pubid, int subset)
> +{
> + struct delta_xml *dxml = data;
> + XML_Parser p = dxml->parser;
> +
> + PARSE_FAIL(p, "parse failed - DOCTYPE not allowed");
> +}
> +
> struct delta_xml *
> new_delta_xml(XML_Parser p, struct rrdp_session *rs, struct rrdp *r)
> {
> @@ -243,6 +253,7 @@ new_delta_xml(XML_Parser p, struct rrdp_
> delta_xml_elem_end);
> XML_SetCharacterDataHandler(dxml->parser, delta_content_handler);
> XML_SetUserData(dxml->parser, dxml);
> + XML_SetDoctypeDeclHandler(dxml->parser, delta_doctype_handler, NULL);
>
> return dxml;
> }
> Index: rrdp_notification.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rrdp_notification.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 rrdp_notification.c
> --- rrdp_notification.c 29 Oct 2021 09:27:36 -0000 1.9
> +++ rrdp_notification.c 4 Nov 2021 09:20:13 -0000
> @@ -308,6 +308,16 @@ notification_xml_elem_end(void *data, co
> PARSE_FAIL(p, "parse failed - unexpected elem exit found");
> }
>
> +static void
> +notification_doctype_handler(void *data, const char *doctypeName,
> + const char *sysid, const char *pubid, int subset)
> +{
> + struct notification_xml *nxml = data;
> + XML_Parser p = nxml->parser;
> +
> + PARSE_FAIL(p, "parse failed - DOCTYPE not allowed");
> +}
> +
> struct notification_xml *
> new_notification_xml(XML_Parser p, struct rrdp_session *repository,
> struct rrdp_session *current, const char *notifyuri)
> @@ -325,6 +335,8 @@ new_notification_xml(XML_Parser p, struc
> XML_SetElementHandler(nxml->parser, notification_xml_elem_start,
> notification_xml_elem_end);
> XML_SetUserData(nxml->parser, nxml);
> + XML_SetDoctypeDeclHandler(nxml->parser, notification_doctype_handler,
> + NULL);
>
> return nxml;
> }
> Index: rrdp_snapshot.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/rrdp_snapshot.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 rrdp_snapshot.c
> --- rrdp_snapshot.c 3 Nov 2021 13:30:56 -0000 1.4
> +++ rrdp_snapshot.c 4 Nov 2021 09:20:13 -0000
> @@ -201,6 +201,16 @@ snapshot_content_handler(void *data, con
> PARSE_FAIL(p, "parse failed - content too big");
> }
>
> +static void
> +snapshot_doctype_handler(void *data, const char *doctypeName,
> + const char *sysid, const char *pubid, int subset)
> +{
> + struct snapshot_xml *sxml = data;
> + XML_Parser p = sxml->parser;
> +
> + PARSE_FAIL(p, "parse failed - DOCTYPE not allowed");
> +}
> +
> struct snapshot_xml *
> new_snapshot_xml(XML_Parser p, struct rrdp_session *rs, struct rrdp *r)
> {
> @@ -219,6 +229,8 @@ new_snapshot_xml(XML_Parser p, struct rr
> snapshot_xml_elem_end);
> XML_SetCharacterDataHandler(sxml->parser, snapshot_content_handler);
> XML_SetUserData(sxml->parser, sxml);
> + XML_SetDoctypeDeclHandler(sxml->parser, snapshot_doctype_handler,
> + NULL);
>
> return sxml;
> }
>
