incubator-pagespeed-ngx icon indicating copy to clipboard operation
incubator-pagespeed-ngx copied to clipboard

Pagespeed works in server block but not http block.

Open pdufour opened this issue 10 years ago • 13 comments

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 avatar Aug 31 '14 01:08 pdufour

@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?

oschaaf avatar Aug 31 '14 08:08 oschaaf

@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

oschaaf avatar Sep 02 '14 09:09 oschaaf

"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 avatar Sep 03 '14 00:09 pdufour

@pdufour thanks, I'll have a go at reproducing and figuring out what happens

oschaaf avatar Sep 03 '14 07:09 oschaaf

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.

sangyun avatar Sep 03 '14 11:09 sangyun

-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;

oschaaf avatar Sep 03 '14 11:09 oschaaf

@sangyun which nginx version are you using?

oschaaf avatar Sep 03 '14 11:09 oschaaf

@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 avatar Sep 04 '14 07:09 sangyun

@sangyun without NGX_CONF_MULTI, the pagespeed configuration directive isn't allowed outside of the http{} scope on lower nginx versions.

oschaaf avatar Sep 04 '14 11:09 oschaaf

For those interested, https://github.com/pagespeed/ngx_pagespeed/pull/798 improves configuration behaviour on older nginx versions.

oschaaf avatar Sep 22 '14 21:09 oschaaf

still won't work in the location block though, right? anything i can do to help?

pdufour avatar Sep 26 '14 04:09 pdufour

@pdufour correct, getting location blocks to work needs more looking into how these older nginx versions work.

oschaaf avatar Sep 26 '14 10:09 oschaaf

@oschaaf this is also same on latest versions of linux it seems.

uhlhosting avatar Feb 23 '22 06:02 uhlhosting