[SRU] init script pid parsing has failure cases

Bug #1314740 reported by Scott Hollenbeck on 2014-04-30
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
nginx (Debian)
Fix Released
Unknown
nginx (Ubuntu)
Medium
Unassigned
Trusty
Medium
Thomas Ward

Bug Description

[Impact]

* The init script fails to start correctly due to the parsing used by the init file. The init file here now will scan all of nginx.conf and find references to 'pid, rather than the actual pidfile line.

* This means that other directives can cause this to fail. Removing /dev/null redirections from the init script, the failure error is wildly apparent. Using the below test case, the 'failure' shows as this when /dev/null redirections in the init script are removed"

teward@trusty:/etc/nginx$ sudo service nginx start
nginx: invalid option: "~*(crawl|Google|Slurp|bingbot|tracker|click|parser|spider|msnbot|Gigabot)"

[Test Case]

There are multiple ways to trigger this, however we'll use the provided test case here.

(TEST CASE 1)

Add this section of code to anywhere in the http { } block of /etc/nginx/nginx.conf:

map $http_user_agent $is_bot {
    default 0;
    ~*(crawl|Google|Slurp|bingbot|tracker|click|parser|spider|msnbot|Gigabot) 1;
}

With the currently existing package, run `sudo service nginx start` - it should return a Failure state. Or it will silently fail. `pidof nginx` will show no output if it fails.

With the modified code/package, do the same, it will not error out, and will correctly start nginx. `pidof nginx` will show multiple PIDs.

[Regression Potential]

The changes applied originate from Debian, and are in all subsequent package releases currently in supported Ubuntu releases.

Regression potential is likely near-zero.

[Original Description]

After upgrading a server from 12.04 to 14.04 I found that the nginx web server wasn't starting automatically. I could start the server without error from the command line. Running "sudo /etc/init.d/nginx start" would produce no visible error, but the service would not be started. Running "sudo /etc/init.d/nginx restart" produces a visible "fail" error.

The nginx startup script (/etc/init.d/nginx) includes this line:

PID=$(awk -F'[ \t;]+' '/[^#]pid/ {print $2}' /etc/nginx/nginx.conf)

This is intended to find this line (or similar) in the nginx.conf file:

pid /var/run/nginx.pid;

My nginx.conf file includes this map directive:

map $http_user_agent $is_bot {
                default 0;
                ~*(crawl|Google|Slurp|bingbot|tracker|click|parser|spider|msnbot|Gigabot) 1;
 }

The issue is that awk returns the line that includes "spider", which creates an incorrect value for the location of the pid file, and the startup script fails. The version of /etc/init.d/nginx provided with 12.04 did not inlcude this awk command and the startup script worked without error.

$ lsb_release -rd
Description: Ubuntu 14.04 LTS
Release: 14.04

$ sudo nginx -v
nginx version: nginx/1.4.6 (Ubuntu)
$

ProblemType: Bug
DistroRelease: Ubuntu 14.04
Package: nginx 1.4.6-1ubuntu3
ProcVersionSignature: Ubuntu 3.13.0-24.46-generic 3.13.9
Uname: Linux 3.13.0-24-generic x86_64
ApportVersion: 2.14.1-0ubuntu3
Architecture: amd64
Date: Wed Apr 30 13:30:35 2014
Dependencies:

InstallationDate: Installed on 2014-01-30 (90 days ago)
InstallationMedia: Ubuntu-Server 12.04.3 LTS "Precise Pangolin" - Release amd64 (20130820.2)
PackageArchitecture: all
ProcEnviron:
 SHELL=/bin/bash
 TERM=xterm
 PATH=(custom, no user)
 LANG=en_US.UTF-8
 XDG_RUNTIME_DIR=<set>
SourcePackage: nginx
UpgradeStatus: Upgraded to trusty on 2014-04-25 (4 days ago)

Robie Basak (racb) wrote :

Thank you for taking the time to report this bug and helping to make Ubuntu better.

Confirmed in Trusty. This needs checking on Debian, and perhaps submission to the Debian BTS.

Marking Importance: Medium since there's an easy workaround available (modify /etc/init.d/nginx).

Changed in nginx (Ubuntu):
status: New → Triaged
importance: Undecided → Medium
Robie Basak (racb) on 2014-06-03
summary: - awk in startup script finds wrong pid
+ init script pid parsing has failure cases
tags: added: needs-upstream-report

PID=$(sed -n 's/^\s*pid\s\s*\([^;]*\).*/\1/p' /etc/nginx/nginx.conf)

works for the three testcases shown in this and linked bugs.

not sure why I cant use \s+ after pid in the expression above, but it works this way.

tags: removed: needs-upstream-report
Changed in nginx (Debian):
status: Unknown → New
Changed in nginx (Debian):
status: New → Fix Committed
Changed in nginx (Debian):
status: Fix Committed → Fix Released
Thomas Ward (teward) wrote :

This was hit by someone today in the nginx support chat room on Freenode. I found that diff in Debian to fix this, and will likely poke at this today.

Thomas Ward (teward) wrote :

This was fixed in later releases after Trusty.

Changed in nginx (Ubuntu):
status: Triaged → Fix Released
Thomas Ward (teward) wrote :

Additional changes were made in my testing to verbosify the init scripts. I can CONFIRM that this does happen, and will get this in the SRU format.

Thomas Ward (teward) on 2015-07-25
description: updated
Changed in nginx (Ubuntu Trusty):
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Thomas Ward (teward)
Thomas Ward (teward) on 2015-07-25
summary: - init script pid parsing has failure cases
+ [SRU] init script pid parsing has failure cases
Thomas Ward (teward) wrote :

Updated debdiff after SRU team feedback.

Hello Scott, or anyone else affected,

Accepted nginx into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/nginx/1.4.6-1ubuntu3.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in nginx (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Thomas Ward (teward) wrote :

Verified that the bug has been fixed with the upload on Trusty.

tags: added: verification-done
removed: verification-needed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nginx - 1.4.6-1ubuntu3.3

---------------
nginx (1.4.6-1ubuntu3.3) trusty-proposed; urgency=medium

  * debian/nginx-common.nginx.init: Fix pidfile extraction, due to multiple
    failure cases, using Debian's solution. (LP: #1314740)

 -- Thomas Ward <email address hidden> Wed, 29 Jul 2015 19:43:04 -0400

Changed in nginx (Ubuntu Trusty):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for nginx has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.