Opened 4 years ago

Closed 4 years ago

#1033 closed defect (invalid)

nginx configtest doesn't fail when missing semicolon on server_name directive

Reported by: geuis@… Owned by:
Priority: minor Milestone:
Component: other Version: 1.10.x
Keywords: Cc:
uname -a: Linux ubuntu 4.5.5-x86_64-linode69 #3 SMP Fri May 20 15:25:13 EDT 2016 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.10.0 (Ubuntu)
built with OpenSSL 1.0.2g-fips 1 Mar 2016
TLS SNI support enabled
configure arguments: --with-cc-opt='-g -O2 -fPIE -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2' --with-ld-opt='-Wl,-Bsymbolic-functions -fPIE -pie -Wl,-z,relro -Wl,-z,now' --prefix=/usr/share/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-debug --with-pcre-jit --with-ipv6 --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_addition_module --with-http_dav_module --with-http_geoip_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module --with-http_v2_module --with-http_sub_module --with-http_xslt_module --with-stream --with-stream_ssl_module --with-mail --with-mail_ssl_module --with-threads

Description

I was tracking down an issue where workers were not being assigned as both ipv4 and ipv6 and finally tracked the issue down to a missing semicolon after the server_name directive that was not being caught by configtest.

Below I will include my nginx.conf and /sites-enabled/blog configurations. Note: These are my final, working config files.

# /etc/nginx/nginx.conf
user www-data;
worker_rlimit_nofile 30000;
worker_processes 8;
pid /run/nginx.pid;

events {
  worker_connections 500000;
}

http {
  sendfile on;
  tcp_nopush on;
  tcp_nodelay on;
  keepalive_timeout 65;
  types_hash_max_size 2048;

  include /etc/nginx/mime.types;
  default_type application/octet-stream;

  # ssl_protocols TLSv1 TLSv1.1 TLSv1.2; # Dropping SSLv3, ref: POODLE
  # ssl_prefer_server_ciphers on;

  access_log /var/log/nginx/access.log;
  error_log /var/log/nginx/error.log;

  gzip on;
  gzip_disable "msie6";
  gzip_vary on;
  gzip_proxied any;
  gzip_comp_level 6;
  gzip_buffers 16 8k;
  gzip_http_version 1.1;
  gzip_types text/plain text/css application/json application/javascript text/xml application/xml application/xml+rss text/javascript;

  include /etc/nginx/conf.d/*.conf;
  include /etc/nginx/sites-enabled/*;
}
# /etc/nginx/sites-enabled/blog
server {
  server_name blog.geuis.com;

  listen 80;
  listen [::]:80;

  root /usr/local/apps/blog;
  index index.php;

  location / {
    try_files $uri $uri/ =404;
  }

  location ~ \.php$ {
    try_files $uri =404;
    fastcgi_split_path_info ^(.+\.php)(/.+)$;
    fastcgi_pass unix:/var/run/php/php7.0-fpm.sock;
    fastcgi_index index.php;
    fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
    include fastcgi_params;
  }
}

If you run the configtest against these files, they will pass:

sudo service nginx configtest 
 * Testing nginx configuration                                                                                                                                                                       [ OK ] 

Now modify /sites-enabled/blog so the first listen directive does not have a semicolon. Then run configtest and it will fail.

server {
  server_name blog.geuis.com;

  listen 80
  listen [::]:80;

  ...
sudo service nginx configtest 
 * Testing nginx configuration                                                                                                                                                                       [fail] 

Now add the semicolon back to the listen directive, then remove the semicolon from server_name and run configtest again. The test will pass.

When the server_name directive doesn't have a trailing semicolon, it was the source of an issue I spent 1.5 days tracking down.

When the semicolon is missing and there are multiple listen directives for both ipv4 and ipv6, then nginx workers are only assigned to one or the other depending on which is encountered first.

There are more details about that behavior described in depth at the following links (my earlier troubleshooting before I understood the source of the issue):

https://forum.nginx.org/read.php?2,268460
http://serverfault.com/questions/791910/nginx-not-spawning-both-ipv4-and-ipv6-workers.

Change History (4)

comment:1 by Maxim Dounin, 4 years ago

Resolution: invalid
Status: newclosed

This is not a bug, it's a result of the server_name directive syntax - "listen" and "80" are perfectly valid server names, and your configuration is interpreted as:

    server_name blog.geuis.com listen 80;
    listen [::]:80;
    ...

Syntax checking can't detect the problem because there are no syntax errors here.

comment:2 by geuis@…, 4 years ago

Resolution: invalid
Status: closedreopened

I'm not aware of the underlying syntax parser, but it seems to me that a new line that is not accompanied by a ';' should at *least* throw a warning.

This may not seem like an issue to you because you are intimately familiar with the nginx project, but since this parsing behavior is not clear or expected it smells like a bug.

More importantly, this is a real-world instance of a user relying on the nginx config test and the test didn't catch a typo. Whether or not the config test is behaving "as expected", the expected behavior from the user is "semicolon ends a line". Its not expected that "no semicolon means to read everything to the next semicolon as domain names".

I would suggest revisiting this and changing the behavior of the parser or to at least throw a warning of some kind when a directive is encountered inline with another directive and no separating semicolon. Again, smells like a bug and its very confusing behavior.

in reply to:  2 comment:3 by Maxim Dounin, 4 years ago

Replying to geuis@…:

I'm not aware of the underlying syntax parser, but it seems to me that a new line that is not accompanied by a ';' should at *least* throw a warning.

Certainly not, it's a normal practice and allowed by nginx configuration syntax, and that's why ';' exists in the first place. You can see multi-line directives even in nginx default/example configuration.

comment:4 by Maxim Dounin, 4 years ago

Resolution: invalid
Status: reopenedclosed
Note: See TracTickets for help on using tickets.