On Thu, Nov 3, 2016 at 12:51 PM, Stefan Eissing < stefan.eiss...@greenbytes.de> wrote:
> Like to get your opinion: > > ap_get_status_line(int) converts any status code it does not know into a > 500. This is fine for the use cases we had so far, although it can be > discussed if it is a good idea, too strict or whatnot. But I do not want to > go down that rathole... > > My question is how httpd should handle unknown codes coming from an > upstream server. Currently: > - mod_proxy_http sends it on (tested for intermediate responses) > - mod_proxy_http2 sends a 500 server error > > The reason for this is that mod_proxy_http2 generates a status line using > ap_get_status_line() while mod_proxy_http can parse that line from the > upstream response. > > So, we have different handling of status code acceptance in proxies > responses for HTTP/1.x and HTTP/2. This should not be. Question is, do we > need another function which returns NULL for such cases, so the caller can > decide to give up or work around it? > We really shouldn't be losing the metadata. A 500 error means something very specific, and although it can represent unrecognized 5xx codes, it isn't applicable to unrecognized, newer 1xx-4xx codes. What would a non-three-digit, >599 response look like? Perhaps we have a good case for a NULL response or a generic 500. The actually callers to this function are fairly limited; ./server/core.c: r->status_line = ap_get_status_line(r->status); ./modules/http2/h2_proxy_session.c: r->status_line = ap_get_status_line(r->status); ./modules/dav/main/mod_dav.c: r->status_line = ap_get_status_line(status); ./modules/dav/main/mod_dav.c: ap_get_status_line(response->status), ./modules/arch/win32/mod_isapi.c: cid->r->status_line = ap_get_status_line(cid->r->status); ./modules/arch/win32/mod_isapi.c: cid->r->status_line = ap_get_status_line(cid->r->status); ./modules/arch/win32/mod_isapi.c: cid->r->status_line = ap_get_status_line(cid->r->status); ./modules/arch/win32/mod_isapi.c: cid->r->status_line = ap_get_status_line(cid->r->status); ./modules/arch/win32/mod_isapi.c: cid->r->status_line = ap_get_status_line(cid->r->status); ./modules/http/http_filters.c: ap_get_status_line(HTTP_CONTINUE), CRLF CRLF, NULL); ./modules/http/http_filters.c: r->status_line = ap_get_status_line(r->status); ./modules/http/http_filters.c: const char *tmp = ap_get_status_line(r->status); Replacing these with a more robust function that doesn't throw away newer response codes would be a good thing, but note that they expect to have some valid result, a NULL result will probably be surprising (and this goes for 3rd party modules as well.)
