Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Commit

Permalink
content-handling-errors: fall back to nginx's default handling
Browse files Browse the repository at this point in the history
This change passes on non succesful status codes for pagespeed
resources and other places where we act as a content handler to
nginx. This has two benefits:
- Instead of a blank page, the user agent receives a formatted
  and (hopefully customized and informative) response.
- Header modules are able to operate on that response, which was
  requested in #612 (comment)
  • Loading branch information
oschaaf authored and crowell committed Feb 23, 2015
1 parent 2cbeaef commit f4f5ac5
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/ngx_base_fetch.h
Expand Up @@ -116,6 +116,7 @@ class NgxBaseFetch : public AsyncFetch {
bool detached() { return detached_; }

ngx_http_request_t* request() { return request_; }
NgxBaseFetchType base_fetch_type() { return base_fetch_type_; }

private:
virtual bool HandleWrite(const StringPiece& sp, MessageHandler* handler);
Expand Down
13 changes: 13 additions & 0 deletions src/ngx_pagespeed.cc
Expand Up @@ -266,6 +266,19 @@ ngx_int_t ps_base_fetch_handler(ngx_http_request_t* r) {
"ps fetch handler: %V", &r->uri);

if (!r->header_sent) {
int status_code = ctx->base_fetch->response_headers()->status_code();
bool status_ok = (status_code != 0) && (status_code < 400);

// Pass on error handling for non-success status codes to nginx for
// responses where PSOL acts as a content generator (pagespeed resource,
// ipro hit, admin pages).
// This generates nginx's default error responses, but also allows header
// modules running after us to manipulate those responses.
if (!status_ok && (ctx->base_fetch->base_fetch_type() != kHtmlTransform
&& ctx->base_fetch->base_fetch_type() != kIproLookup)) {
return status_code;
}

if (ctx->preserve_caching_headers != kDontPreserveHeaders) {
ngx_table_elt_t* header;
NgxListIterator it(&(r->headers_out.headers.part));
Expand Down
10 changes: 10 additions & 0 deletions test/nginx_system_test.sh
Expand Up @@ -1143,6 +1143,16 @@ OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1)
MATCHES=$(echo "$OUT" | grep -c "Server: override") || true
check [ $MATCHES -eq 1 ]

start_test Conditional cache-control header override in resource flow.
URL=http://headers.example.com/mod_pagespeed_test/
URL+=A.doesnotexist.css.pagespeed.cf.0.css
# The 404 response makes wget exit with an error code, which we ignore.
OUT=$(http_proxy=$SECONDARY_HOSTNAME $WGET_DUMP -O /dev/null -S $URL 2>&1) || true
# We ignored the exit code, check if we got a 404 response.
check_from "$OUT" fgrep -qi '404'
MATCHES=$(echo "$OUT" | grep -c "Cache-Control: override") || true
check [ $MATCHES -eq 1 ]

if $USE_VALGRIND; then
# It is possible that there are still ProxyFetches outstanding
# at this point in time. Give them a few extra seconds to allow
Expand Down
3 changes: 2 additions & 1 deletion test/pagespeed_test.conf.template
Expand Up @@ -1192,7 +1192,8 @@ http {
pagespeed LoadFromFile "http://headers.example.com/"
"@@SERVER_ROOT@@/";
location /mod_pagespeed_test/ {
more_set_headers "Server: overriden";
more_set_headers "Server: override";
more_set_headers -s '404' 'Cache-Control: override';
}
}

Expand Down

0 comments on commit f4f5ac5

Please sign in to comment.