incubator-pagespeed-ngx
incubator-pagespeed-ngx copied to clipboard
Pagespeed works in server block but not http block.
Hi, I'm having problems with getting pagespeed to work on Ubuntu Precise. I built it on Trusty and it worked fine, then I did the same process on Precise and I'm running into problems.
nginx -V:
configure arguments: --prefix=/etc/nginx --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-log-path=/var/log/nginx/access.log --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --lock-path=/var/lock/nginx.lock --pid-path=/var/run/nginx.pid --with-debug --with-http_addition_module --with-http_dav_module --with-http_geoip_module --with-http_gzip_static_module --with-http_image_filter_module --with-http_realip_module --with-http_stub_status_module --with-http_ssl_module --with-http_sub_module --with-http_xslt_module --with-ipv6 --with-sha1=/usr/include/openssl --with-md5=/usr/include/openssl --with-mail --with-mail_ssl_module --add-module=/build/buildd/nginx-1.1.19/debian/modules/nginx-auth-pam --add-module=/build/buildd/nginx-1.1.19/debian/modules/nginx-echo --add-module=/build/buildd/nginx-1.1.19/debian/modules/nginx-upstream-fair --add-module=/build/buildd/nginx-1.1.19/debian/modules/nginx-dav-ext-module --add-module=/build/buildd/nginx-1.1.19/debian/modules/ngx_pagespeed
Now when I turn pagespeed on in /etc/nginx/nginx.conf, nginx -t
does not complain
http {
..
pagespeed on;
pagespeed RewriteLevel PassThrough;
pagespeed FileCachePath "/var/cache/ngx_pagespeed/";
..
nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
nginx: configuration file /etc/nginx/nginx.conf test is successful
I paste the same code in a server block and it fails
server {
listen 80 default_server;
listen [::]:80 default_server ipv6only=on;
pagespeed on;
...
nginx -t:
nginx: [emerg] "pagespeed" directive is not allowed here in /etc/nginx/sites-enabled/test:24
This is similar to https://github.com/pagespeed/ngx_pagespeed/issues/485, https://github.com/pagespeed/ngx_pagespeed/issues/576 and https://github.com/pagespeed/ngx_pagespeed/issues/576
@pdufour The issues that you are referencing are about having two nginx installs on a single machine, one with and one without ngx_pagespeed. Are you running two different nginx binaries as well?
@pdufour You mention both /etc/nginx/nginx.conf
and /etc/nginx/sites-enabled/test
. Does nginx -t
also fail if you add pagespeed on;
in a server{}
block in /etc/nginx/nginx.conf
?
Does this run ok when you have pagespeed on
at the http{}
level in /etc/nginx/nginx.conf
?:
nginx -t -c /etc/nginx/nginx.conf
"Does nginx -t also fail if you add pagespeed on; in a server{} block in /etc/nginx/nginx.conf?"
http {
...
server {
listen 80;
keepalive_timeout 70;
pagespeed on;
}
...
}
nginx: [emerg] "pagespeed" directive is not allowed here in /etc/nginx/nginx.conf:82
"Does this run ok when you have pagespeed on at the http{} level in /etc/nginx/nginx.conf?:"
First it says:
nginx: [emerg] "gzip_types" directive FileCachePath must be set in /etc/nginx/nginx.conf:80
Line 80 is the ending }
right after pagespeed on. Then I add pagespeed FileCachePath "/var/cache/ngx_pagespeed/";
nginx: the configuration file /etc/nginx/nginx.conf syntax is ok
nginx: configuration file /etc/nginx/nginx.conf test is successful
And I don't believe I have two nginx versions running. I tried ps aux | grep nginx
to see where the binary was, and then ran nginx -t
with the full path.
Here is the package I'm using if it helps: https://launchpad.net/~britco/+archive/ubuntu/main (1.1.19-1ubuntu0.6+britco0.5+precise)
Basically all I did was take the source here https://launchpad.net/ubuntu/+source/nginx and add the pagespeed module.
@pdufour thanks, I'll have a go at reproducing and figuring out what happens
For me, I got same result @pdufour reported. After testing about varius version, 1.8.x results same error, and 1.7.x does not. Is anyone tell me, config check module diffierance between 1.8.x and 1.7.x in ngx_pagespeed? Thank you.
-editted to completion, I wasn't ready yet-
@pdufour I could reproduce this when building with nginx 1.1.19
Configuration at the server{}
level is marked as invalid by that nginx version at this line:
https://github.com/nginx/nginx/blob/ed2df87c83d3a68ab2a395d4c2549dbe26e86ab0/src/core/ngx_conf_file.c#L322
It seems that NGX_CONF_MULTI was deprecated at some point.
This patch seems to make configuration work for nginx 1.1.19 at the server{}
level, but not yet at the location level.
diff --git a/src/ngx_pagespeed.cc b/src/ngx_pagespeed.cc
index 66f5c8c..9a011c1 100644
--- a/src/ngx_pagespeed.cc
+++ b/src/ngx_pagespeed.cc
@@ -23,6 +23,8 @@
* }
*/
+
+
#include "ngx_pagespeed.h"
#include <vector>
@@ -506,14 +508,14 @@ char* ps_loc_configure(ngx_conf_t* cf, ngx_command_t* cmd, void* conf);
// not NGX_HTTP_LOC_CONF_OFFSET or NGX_HTTP_MAIN_CONF_OFFSET.
ngx_command_t ps_commands[] = {
{ ngx_string("pagespeed"),
- NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1|
+ NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1|NGX_CONF_MULTI|
NGX_CONF_TAKE2|NGX_CONF_TAKE3|NGX_CONF_TAKE4|NGX_CONF_TAKE5,
ps_main_configure,
NGX_HTTP_SRV_CONF_OFFSET,
0,
NULL },
{ ngx_string("pagespeed"),
- NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1|
+ NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1|NGX_CONF_MULTI|
NGX_CONF_TAKE2|NGX_CONF_TAKE3|NGX_CONF_TAKE4|NGX_CONF_TAKE5,
ps_srv_configure,
NGX_HTTP_SRV_CONF_OFFSET,
@@ -521,7 +523,7 @@ ngx_command_t ps_commands[] = {
NULL },
{ ngx_string("pagespeed"),
- NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF|NGX_CONF_TAKE1|
+ NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF|NGX_CONF_TAKE1||NGX_CONF_MULTI|
NGX_CONF_TAKE2|NGX_CONF_TAKE3|NGX_CONF_TAKE4|NGX_CONF_TAKE5,
ps_loc_configure,
NGX_HTTP_SRV_CONF_OFFSET,
@@ -686,7 +688,6 @@ char* ps_configure(ngx_conf_t* cf,
const char* status = (*options)->ParseAndSetOptions(
args, n_args, cf->pool, handler, cfg_m->driver_factory, option_scope, cf,
process_script_variables);
-
// nginx expects us to return a string literal but doesn't mark it const.
return const_cast<char*>(status);
}
@@ -1218,6 +1219,7 @@ ngx_int_t ps_base_fetch_handler(ngx_http_request_t* r) {
ngx_log_error(NGX_LOG_DEBUG, ctx->r->connection->log, 0,
"CollectAccumulatedWrites, %d", rc);
+
if (rc == NGX_ERROR) {
ps_set_buffered(r, false);
return NGX_HTTP_INTERNAL_SERVER_ERROR;
@sangyun which nginx version are you using?
@oschaaf I use nginx v1.2.0 (w/ my own tweak and extension module), and with your patch it's running correctly. Do you know what happened between these, and what does mean 'NGX_CONF_MULTI'? I had searched that it's used in 'server' directive, and i'm curious about this config's usage. Thank You.
@sangyun without NGX_CONF_MULTI
, the pagespeed
configuration directive isn't allowed outside of the http{} scope on lower nginx versions.
For those interested, https://github.com/pagespeed/ngx_pagespeed/pull/798 improves configuration behaviour on older nginx versions.
still won't work in the location block though, right? anything i can do to help?
@pdufour correct, getting location blocks to work needs more looking into how these older nginx versions work.
@oschaaf this is also same on latest versions of linux it seems.