Re: [PATCH] http-backend: give a hint that web browser access is not supported

From: Jan Engelhardt <jengelh@inai.de>
Date: Dec 4, 2021, 11:09 UTC
Message-ID: <7r23s082-o3q0-479o-srqn-r45q778s5nq7@vanv.qr>
In-Reply-To: <xmqqee6spz9s.fsf@gitster.g>

On Saturday 2021-12-04 09:09, Junio C Hamano wrote:

Show 8 quoted lines

>Jan Engelhardt <jengelh@inai.de> writes:
>
>> When using a browser to access a URI that is served by http-backend,
>> nothing but a blank page is shown. This is not helpful.
>>
>> Emit the same "Request not handled" messages, but to the CGI stream
>
>Puzzled.  Same with what?

"Request not handled" is already sent to stderr, which means it (only) shows up in the httpd error log.

So now we send "Request not handled" also to stdout, which is what the browser will see.

>What is in "pathinfo" parameter?

It is getenv("PATH_INFO").

Show 21 quoted lines

>I think the use of internal API in http-backend.c in the new code is
>wrong; I haven't seen how it is used until now, so take this with a
>grain of salt, though.
>
>Did you actually mean something different, that is:
>
>	struct strbuf body = STRBUF_INIT;
>
>	http_status(hdr, 404, "Not Found");
>	hdr_nocache(hdr);
>       
>	/* stuff pathinfo, err, and hint into "body" strbuf ... */
>	if (pathinfo)
>		strbuf_addf(&body, "%s: ", pathinfo);
>	strbuf_addf(&body, "%s.\r\n", err);
>        if (hint)
>		strbuf_addf(&body, "%s\r\n", hint);
>
>	/* ... and send it out */
>	send_strbuf(hdr, "text/plain", &body);
>	strbuf_release(&body);

Yes, that seems more like it. I was not aware of send_strbuf.

>I notice that this is still going to the standard error stream.  Is
>the intention that the remote requestor may get a lightly redacted
>error message while the log will leave detailed record to help
>debugging?

Yes.

>Why do we need the original not_found()?  It seems that there is
>only one remaining caller

I suppose it can be dissolved.

>ANd if we can get rid of the use of the original not_found(), we
>could even take its nice name over.