On Wed, Dec 04, 2024 at 03:40:51PM +0100, Omar Polo wrote:
> Hello,
> 
> 
> sorry for the delay!
> 
> 
> On 02/12/24 18:01, Stefan Sperling wrote:
> > dump_fcgi_record() gets called for a length that is at least the size
> > of the header, but the function also processes parts of the body. Add
> > a length check to skip the body if the buffer is too short.
> >
> > ok?
> 
> Looks fine to me, but what about deferring the call to dump_fcgi_record() in fcgi_parse_record() by a couple of lines? Likewise, the other call can be just changed to dump_fcgi_record_header() since it's called on a just constructed header.
> 
> I think it's cleaner if dump_fcgi_record() is defined to work only on a fully-read fcgi record.

Oh, yes, that approach seems better.

Is this ok?


M  gotwebd/fcgi.c     |  6+  5-
M  gotwebd/gotwebd.h  |  0+  1-

2 files changed, 6 insertions(+), 6 deletions(-)

commit - 8c5906074ad5756611aa1977542e314d8e3662bb
commit + 388441cdf262f31be12cb25a5dee475902c75f3c
blob - ecad0c59902279155f09c3e1229c8ba25759a7d5
blob + 4c8f861d6c42639779642701c2392fb2bd30606d
--- gotwebd/fcgi.c
+++ gotwebd/fcgi.c
@@ -45,6 +45,7 @@ void	 fcgi_parse_begin_request(uint8_t *, uint16_t, st
 void	 fcgi_parse_params(uint8_t *, uint16_t, struct request *, uint16_t);
 int	 fcgi_send_response(struct request *, int, const void *, size_t);
 
+void	 dump_fcgi_request_body(const char *, struct fcgi_record_header *);
 void	 dump_fcgi_record_header(const char *, struct fcgi_record_header *);
 void	 dump_fcgi_begin_request_body(const char *,
 	    struct fcgi_begin_request_body *);
@@ -119,12 +120,14 @@ fcgi_parse_record(uint8_t *buf, size_t n, struct reque
 
 	h = (struct fcgi_record_header*) buf;
 
-	dump_fcgi_record("", h);
+	dump_fcgi_record_header("", h);
 
 	if (n < sizeof(struct fcgi_record_header) + ntohs(h->content_len)
 	    + h->padding_len)
 		return 0;
 
+	dump_fcgi_request_body("", h);
+
 	if (h->version != 1)
 		log_warn("wrong version");
 
@@ -306,7 +309,7 @@ send_response(struct request *c, int type, const uint8
 	iov[2].iov_base = (void *)padding;
 	iov[2].iov_len = header.padding_len;
 
-	dump_fcgi_record("resp ", &header);
+	dump_fcgi_record_header("resp ", &header);
 
 	/*
 	 * XXX: add some simple write heuristics here
@@ -411,10 +414,8 @@ fcgi_cleanup_request(struct request *c)
 }
 
 void
-dump_fcgi_record(const char *p, struct fcgi_record_header *h)
+dump_fcgi_request_body(const char *p, struct fcgi_record_header *h)
 {
-	dump_fcgi_record_header(p, h);
-
 	if (h->type == FCGI_BEGIN_REQUEST)
 		dump_fcgi_begin_request_body(p,
 		    (struct fcgi_begin_request_body *)(h + 1));
blob - 59fef09083a448dfb1173969b2c5b5426af439a8
blob + 07263753b899195d6adff82948378fd8a64c4c26
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -483,7 +483,6 @@ void fcgi_request(int, short, void *);
 void fcgi_timeout(int, short, void *);
 void fcgi_cleanup_request(struct request *);
 void fcgi_create_end_record(struct request *);
-void dump_fcgi_record(const char *, struct fcgi_record_header *);
 int fcgi_write(void *, const void *, size_t);
 
 /* got_operations.c */