SSI include fails with pagespeed in more complex scenarios #1560

jvnn · 2018-05-15T09:03:30Z

I ran into some issues with SSI support when doing testing with pagespeed. Using Nginx 1.13.9 and ngx_pagespeed 1.13.35.2-0, built together using the provided automated script. First I'll describe some scenarios where I managed to trigger problems with pagespeed and at the end you'll find a couple crash stack traces from another installation.

Two scenarios:

trying to include a page that does not exists and the error page is redirected to another server (another server config on the same Nginx instance)
including a file with "virtual" directive, so that the included file triggers another location configuration block than the original request (see example below for better explanation)

Here is my nginx config (relevant parts):

http {
  pagespeed on;
  pagespeed FileCachePath "/some/path/nginx_pagespeed/ps_cache";
  pagespeed EnableFilters combine_css,combine_javascript;

  server {
    listen       8080;
    server_name  localhost;
    root   /path/to/webserver/root;

    location / {
      index  index.html;
      ssi on;
    }

    location /ssi {
      root /path/to/webserver/root;
    }

    error_page 404 = @notfound;
    location @notfound {
      proxy_pass http://localhost:6666;
    }
  }

  server {
    listen 6666;
    server_name failfailfail;
    root   /path/to/webserver/root;

    error_page 404 /404.html;
    location /404.html {
        internal;
    }
  }
}

For the first scenario, I have the following HTML file in the webserver root directory:

<html>
<head>
  <title>SSI!</title>
</head>
<body>
  From SSI:<br>
  <!--# include file="doesnotexist.html" -->
</body>
</html>

The request with pagespeed enabled looks like this:

From SSI:
�‹��%ŽA�Ã �ï¼bû��%GËOÈ�@¸�)…Š:móû¢ø8«ÕìRÖçÎŽ²„Ä� -º�¯MñhGMä-päB±¥ójæ‰—û2òÉpæu+õ�íE�>ÒOäÐ�â¡Ð,x…Mð oÔ!×†(6q�Šy\ðf�t]ú�“�ÁTš

When I disable the error page redirection or pagespeed, the response correctly displays a 404 error under the "From SSI:" line. This problem is somewhat scary, because the output looks like random memory contents.

The second scenario uses the following HTML file:

<html>
<head>
  <title>SSI!</title>
</head>
<body>
  From SSI:<br>
  <!--# include virtual="/ssi/foo.html" -->
</body>
</html>

In this case the response only contains the line "From SSI:", nothing gets included. No errors in the Nginx error.log either. If I disable pagespeed it works, and interestingly, it also works if I add "ssi on;" directive to the "location /ssi" block. This should not be necessary and as mentioned, everything works correctly without pagespeed.

Then the background: I started looking at this issue after receiving some core dumps where a module that I'm developing was suspected of crashing Nginx. What I found out was that it was pagespeed that was crashing and its context structure stored in the request object seemed totally corrupted. Everything else in the dump seemed correct, so it is very likely a pagespeed bug. Unfortunately I'm not allowed to include the full dumps or configuration here because they come from a customer, but here are the two stack traces that I have:

First a segfault:

nginx!net_instaweb::(anonymous namespace)::in_place::ps_in_place_body_filter(ngx_http_request_s*, ngx_chain_s*)+f8 /usr/lib/nginx/ngx_pagespeed/src/ngx_pagespeed.cc:2547
nginx!net_instaweb::ps_base_fetch::ps_base_fetch_filter(ngx_http_request_s*, ngx_chain_s*)+48 /usr/lib/nginx/ngx_pagespeed/src/ngx_pagespeed.cc:259
nginx!net_instaweb::(anonymous namespace)::html_rewrite::ps_html_rewrite_body_filter(ngx_http_request_s*, ngx_chain_s*)+71 /usr/lib/nginx/ngx_pagespeed/src/ngx_pagespeed.cc:2381
nginx!ngx_http_postpone_filter+5e /home/downloads/nginx-setup/nginx/src/http/ngx_http_postpone_filter_module.c:82
nginx!ngx_http_ssi_output+7c /home/downloads/nginx-setup/nginx/src/http/modules/ngx_http_ssi_filter_module.c:936
nginx!ngx_http_ssi_body_filter+142c /home/downloads/nginx-setup/nginx/src/http/modules/ngx_http_ssi_filter_module.c:805
nginx!ngx_http_charset_body_filter+42 /home/downloads/nginx-setup/nginx/src/http/modules/ngx_http_charset_filter_module.c:557
nginx!ngx_http_sub_body_filter+30 /home/downloads/nginx-setup/nginx/src/http/modules/ngx_http_sub_filter_module.c:299
nginx!ngx_output_chain+745 /home/downloads/nginx-setup/nginx/src/core/ngx_output_chain.c:214
nginx!ngx_http_copy_filter+130 /home/downloads/nginx-setup/nginx/src/http/ngx_http_copy_filter_module.c:154
nginx!ngx_http_range_body_filter+3b /home/downloads/nginx-setup/nginx/src/http/modules/ngx_http_range_filter_module.c:627

And a SIGABRT after an internal pagespeed sanity check fails:

#0  0x00007f31c23e11f7 in raise ()
#1  0x00007f31c23e28e8 in abort ()
#2  0x00000000004e4239 in base::debug::BreakDebugger() ()
#3  0x00000000004b5184 in (anonymous namespace)::LogMessageHandler (severity=4, file=<optimized out>, line=<optimized out>, message_start=<optimized out>, str=...) at /usr/lib/nginx/ngx_pagespeed/src/log_message_handler.cc:87
#4  0x00000000004e6d30 in logging::LogMessage::~LogMessage() ()
#5  0x00000000004bb051 in net_instaweb::(anonymous namespace)::ps_send_to_pagespeed (r=r@entry=0x50bed70, ctx=ctx@entry=0x402e510, cfg_s=cfg_s@entry=0x32dcee8, in=in@entry=0x3ccdc40) at /usr/lib/nginx/ngx_pagespeed/src/ngx_pagespeed.cc:2107
#6  0x00000000004bb78c in net_instaweb::(anonymous namespace)::html_rewrite::ps_html_rewrite_body_filter (r=<optimized out>, in=0x3ccdc40) at /usr/lib/nginx/ngx_pagespeed/src/ngx_pagespeed.cc:2394
#7  0x000000000048637c in ngx_http_postpone_filter (r=<optimized out>, in=<optimized out>) at src/http/ngx_http_postpone_filter_module.c:82
#8  0x0000000000486844 in ngx_http_ssi_output (r=r@entry=0x4d566e0, ctx=ctx@entry=0x3ccd508) at src/http/modules/ngx_http_ssi_filter_module.c:934
#9  0x00000000004899bc in ngx_http_ssi_body_filter (r=<optimized out>, in=<optimized out>) at src/http/modules/ngx_http_ssi_filter_module.c:805
#10 0x000000000048abbb in ngx_http_charset_body_filter (r=0x4d566e0, in=0x3ccd900) at src/http/modules/ngx_http_charset_filter_module.c:557
#11 0x000000000048c419 in ngx_http_sub_body_filter (r=0x4d566e0, in=0x3ccd900) at src/http/modules/ngx_http_sub_filter_module.c:299
#12 0x0000000000433965 in ngx_output_chain (ctx=ctx@entry=0x3ccd898, in=in@entry=0x3ccd7d8) at src/core/ngx_output_chain.c:214
#13 0x000000000048ead8 in ngx_http_copy_filter (r=0x4d566e0, in=0x3ccd7d8) at src/http/ngx_http_copy_filter_module.c:152
#14 0x0000000000484d61 in ngx_http_range_body_filter (r=0x4d566e0, in=<optimized out>) at src/http/modules/ngx_http_range_filter_module.c:627

Note that these are NOT dumps from the above scenarios, the test cases were simply some problems I managed to find when performing different kinds of tests. The customer is using Nginx 1.12.1 and pagespeed version 1.12.34.2-0.

The text was updated successfully, but these errors were encountered:

oschaaf · 2018-05-15T09:15:52Z

The segfault probably happens when dereferencing the recorder here:

incubator-pagespeed-ngx/src/ngx_pagespeed.cc

Line 2547 in f3c64ca

recorder->Write(contents, recorder->handler());

In the second trace, this CHECK fires:

incubator-pagespeed-ngx/src/ngx_pagespeed.cc

Line 2107 in f3c64ca

CHECK(ctx->proxy_fetch != NULL);

jvnn · 2018-05-15T09:21:21Z

One more detail, the content of the request line might be of interest:

request_line = {len = 34, data = 0x3c6f820 "GET /<removed>/ HTTP/2.0"}

And although I cannot provide the dump itself, if you need some specific data from the dumps (request struct members etc.), I can do some digging.

oschaaf · 2018-05-15T09:27:09Z

Detailed error logs at the debug level for these scenarios would be helpful in analysing this more -- if you are able to provide them (anonimized).

I do have a suspicion. ngx_pagespeed may need to ensure it maintains a single request context when nginx restarts the requests in this scenario to function properly. That may be quite tricky, but I did do a PoC for that at one point which was quite hacky and passed the context around in the request headers internally. It seems doable, but I really wouldn't be happy to merge it that way. But perhaps we can create an internal nginx parameter or something like that to persist our context across restarted requests.

One other upside, outside of this scenario, is that doing so may save us per-request cache-lookups in common configuration scenarios.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SSI include fails with pagespeed in more complex scenarios #1560

SSI include fails with pagespeed in more complex scenarios #1560

jvnn commented May 15, 2018

oschaaf commented May 15, 2018

jvnn commented May 15, 2018

oschaaf commented May 15, 2018

SSI include fails with pagespeed in more complex scenarios #1560

SSI include fails with pagespeed in more complex scenarios #1560

Comments

jvnn commented May 15, 2018

oschaaf commented May 15, 2018

jvnn commented May 15, 2018

oschaaf commented May 15, 2018