From: Stefan Sperling Subject: Re: gotwebd: guard against missing qs->file To: Omar Polo Cc: gameoftrees@openbsd.org Date: Mon, 29 Jan 2024 14:59:45 +0100 On Mon, Jan 29, 2024 at 02:52:04PM +0100, Omar Polo wrote: > On 2024/01/29 14:20:43 +0100, Stefan Sperling wrote: > > On Mon, Jan 29, 2024 at 01:52:30PM +0100, Omar Polo wrote: > > > @@ -218,6 +223,10 @@ gotweb_process_request(struct request *c) > > > > > > switch (qs->action) { > > > case BLAME: > > > + if (qs->folder == NULL || qs->file == NULL) { > > > > This change introduces a small inconsistency: > > qs->folder is allowed to be NULL in got_output_file_blame() which now > > becomes impossible to reach. > > I realized it, but now we seem to always emit a folder parameter[1], maybe > empty, so I was considering being more strict here. Good! > I think got_oput_file_blame() did it either to be pedantic against bad > input or because before folder was not always set. I am not sure. The most likely case is just careful coding. I suspect this code was added before templating was introduced. I might even have been the first line of input validation defence back then. > I can remove the qs->folder check if you prefer. Please remove it from the other place instead. Overall, I would prefer to keep input validation checks in one place. That makes it easier to spot problems where such checks are missing. So when adding the qs->folder check at this higher level please remove it from the lower level so that we converge towards eventual consistency.