Post install script has error in RegEx

[Andy Wagner]

== Begin SRU Template ==
[Impact]

 * On upgrade of tomcat7 package, if a user has updated their JAVA_OPTS variable to include a '%' an upgrade will fail. The sed command in the postinst uses the '%' character to act as a delimiter, previous versions used '/' however it was updated to '%' in hopes it was far less common.
 * This SRU updates it to a character that should not be found in the JAVA_ARGS value, namely '\001'.
 * This is the same solution Debian and tomcat maintainers are now using for Tomcat8.

[Test Case]

An example to test Tomcat7 on Trusty. The same instructions can apply to Tomcat8 on the other releases.

Overview: Install the version from the current release. Modify JAVA_OPTS and then install the version from proposed to validate it upgrades successfully.

 * lxc launch ubuntu-daily:trusty trusty
 * lxc exec trusty bash
 * apt install tomcat7
 * Edit /etc/default/tomcat7, set JAVA_OPTS="-Djava.awt.headless=true -XX:ErrorFile=/var/log/tomcat7/java_error%p.log -XX:+DisableExplicitGC -XX:+UseG1GC"
 * # Enable proposed
 * apt update
 * apt install tomcat7
 * When asked, 'Keep the local version currently installed'
 * With the fix, install will complete
 * Without the fix, the error as described under "Other Info" will appear

[Regression Potential]

 * Users currently experiencing this issue would be expecting a SRU fix to come from us. Working around it would require changing their JAVA_OPTS temporarily, accepting the maintainers version of the defaults script, or modifying the package's postinst script directly.
* In either case the proposed fix will over write any changes an end user may have made to the postinst, and all for correctly working expected behavior.
 * There is the slight, albeit incredibly low chance, that someone actually has the '\001' character in their JAVA_OPTS. In which, case the upgrade would fail as this bug describes.

[Other Info]

 * Using a new delimiter that is far less likely to be in someone's path. This is not the first time the delimiter has changed, as it originally as '/' which is obviously going to show up as soon as someone adds a path.
 * Upstream change to tomcat8: https://anonscm.debian.org/cgit/pkg-java/tomcat8.git/patch/?id=7664221d66701e2c31a31fe3b4f22e8bea4158dc
 * Error message on failure:

Setting up tomcat7 (7.0.52-1ubuntu0.10) ...
sed: -e expression #1, char 97: unknown option to `s'
dpkg: error processing package tomcat7 (--configure):
 subprocess installed post-installation script returned error exit status 1
Errors were encountered while processing:
 tomcat7
E: Sub-process /usr/bin/dpkg returned an error code (1)

== End SRU Template ==

[Joshua Powers (powersj)]

Thank you for taking the time to file a bug report and try to solve it!

Looking at your proposed resolution if we were to escape the $ then it
would replace %JAVA_OPTS with $JAVA_OPTS, however I do not believe that
is what we want. The goal of this line is to set the JAVA_OPTS based on
the following:

db_get tomcat7/javaopts && JAVA_OPTS="$RET" || JAVA_OPTS="-Djava.awt.headless=true -Xmx128m -XX:+UseConcMarkSweepGC"

You will also note how the previous two lines do not escape
$TOMCAT7_USER or $TOMCAT7_GROUP.

My initial guess is that JAVA_OPTS may have the '%' character in it.
That would in turn mess up the sed command since '%' is used as the
delimiter in that statement. In fact there was a similar Debian bug [1]
reported about this issue with the '/' character. There is a comment
as to changing to '%' as it shows up less often :)

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=597814

[Joshua Powers (powersj)]

Can you get the output of the following:

$ cat /etc/default/tomcat7 | grep -i java_opts
$ debconf-show tomcat7

[Andy Wagner (rarian)]

You are indeed correct, the culprit is a `%` in the java options of our affected servers, one example being:
JAVA_OPTS="-Djava.awt.headless=true -XX:ErrorFile=/var/log/tomcat7/java_error%p.log -Xms15025m -Xmx15025m -XX:+DisableExplicitGC -XX:+UseG1GC -XX:+UseStringDeduplication"

And for completeness:
$ debconf-show tomcat7
  tomcat7/groupname: tomcat7
  tomcat7/javaopts: -Djava.awt.headless=true -XX:ErrorFile=/var/log/tomcat7/java_error%p.log -Xms15025m -Xmx15025m -XX:+DisableExplicitGC -XX:+UseG1GC -XX:+UseStringDeduplication
  tomcat7/username: tomcat7

At first blush our solution worked, but this was a side affect of our configuration management system rewriting the file and camouflaging the fact that it isn't actually a solution at all!

Anyway, your explanation of what the regular expression is actually trying to accomplish leads to what I believe is a relatively trivial use of parameter expansion to escape the '%' in the JAVA_OPT environment variable:
| sed "s%^JAVA_OPTS=.*$%JAVA_OPTS=\"${JAVA_OPTS/\%/\\\%}\"%"

I was rather suspicious that this wasn't a known bug already, so I guess I should have dug deeper originally.

[Andy Wagner (rarian)]

Let me know if there is anything further I can do to move this forward.

[Joshua Powers (powersj)]

Thanks for the ping.

I've filed the bug with Debian. I'd like to first see what they think of my solution and then can get this SRU'ed back to Trusty.

[Joshua Powers (powersj)]

This is how upstream has fixed this:

https://anonscm.debian.org/cgit/pkg-java/tomcat8.git/commit/?id=7664221

Let me see about getting an SRU to trusty now.

[Andy Wagner (rarian)]

That is an elegant solution, thanks for staying on top of this!

[Joshua Powers (powersj)]

Created PPA with fix for Trusty here:
https://launchpad.net/~powersj/+archive/ubuntu/tomcat7-1666570

Attaching debdiff for trusty.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "debdiff for trusty" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Joshua Powers (powersj)]

Only source package, binary is not present in zesty

[Joshua Powers (powersj)]

@Andy I have committed fixes for all changes to our importer. They will be reviewed and accepted, unless I need to make any changes :) Once accepted, the new packages will enter the proposed pocket for final testing and acceptance and after a minimum of 7 days released. I believe I have that right, but open to correction from anyone ;)

[Brian Murray (brian-murray)]

Hello Andy, or anyone else affected,

Accepted tomcat7 into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/tomcat7/7.0.72-1ubuntu0.1 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 on 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!

[Brian Murray (brian-murray)]

Hello Andy, or anyone else affected,

Accepted tomcat7 into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/tomcat7/7.0.68-1ubuntu0.2 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 on 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!

[Brian Murray (brian-murray)]

Hello Andy, or anyone else affected,

Accepted tomcat7 into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/tomcat7/7.0.52-1ubuntu0.11 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 on 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!

[Robie Basak (racb)]

I've reviewed and uploaded Josh's MPs for tomcat8. Since there are three and some review comments are shared, I'm writing them up here to avoid confusion between the multiple MPs.

For all three, I had to s/tomcat7/tomcat8/ in the conffile pathname, as this has switched in tomcat8.

For Zesty, I changed the version from 8.0.38-2ubuntu1.1 to 8.0.38-2ubuntu2. 1.1 would work, but convention for the development release is that we would use ubuntu2. Exceptionally we are in final freeze so it may become an SRU anyway.

I missed that tomcat8 Zesty wasn't fixed before. I would have required this before uploading tomcat7 too had I noticed, because really the SRU rule of "fix development first" should apply in spirit between tomcat7 and tomcat8 because it is only a technicality that they have different source package names in this case. But as we're including Zesty in this upload, we're fixing that up anyway, and the tomcat7 fixes haven't hit the release pocket yet so users of not -proposed won't see a regression.

There was a bug in the "usd import" tool that caused Josh's Yakkety fixes to be based on 8.0.37-1 instead of 8.0.37-1ubuntu0.1 that is in yakkety-security currently. So the upload got rejected, I noticed, and rebased Josh's changes on top of the yakkety-security tree for now. The yakkety-devel branch pointer in the git imported tomcat8 repository is still wrong, but I think it may fix itself when it imports this upload. The commit hash will still be "wrong", but will be fixed when we re-import.

Release and SRU teams: as the SRU for tomcat7 is already in progress (due to my previous review omission), I figured that having this in the Zesty unapproved queue would be the best thing to do for now. It's a minor fix so we can skip it for Zesty release if you like, but we should process it as post-release SRU in Zesty as Josh is SRUing all the other releases anyway and Zesty would be left out otherwise.

[Launchpad Janitor (janitor)]

This bug was fixed in the package tomcat8 - 8.0.38-2ubuntu2

---------------
tomcat8 (8.0.38-2ubuntu2) zesty; urgency=medium

  * Fix an upgrade error when JAVA_OPTS in /etc/default/tomcat8
    contains the '%' character (LP: #1666570).

 -- Joshua Powers <email address hidden> Tue, 28 Mar 2017 16:47:32 -0700

[Robie Basak (racb)]

I think everything is in -proposed or in the unapproved queues pending SRU review now, so unsubscribing ~ubuntu-sponsors (I forgot to do this when I uploaded).

[Joshua Powers (powersj)]

@sru-team

Verified tomcat7 trusty (7.0.52-1ubuntu0.11) based on the test case described in the SRU template. The update to the proposed version completed without error. Marking as verification-done-trusty.

I will do Xenial and Yakkety once I have binaries.

[Joshua Powers (powersj)]

@racb,

It looks like the uploads to Xenial and Yakkety did not produce binaries. Looking at the logs the builds failed on tests. I uploaded the current released version in xenial to a ppa and it also failed. What are my next steps?

[Launchpad Janitor (janitor)]

This bug was fixed in the package tomcat7 - 7.0.52-1ubuntu0.11

---------------
tomcat7 (7.0.52-1ubuntu0.11) trusty; urgency=medium

  * Fix an upgrade error when JAVA_OPTS in /etc/default/tomcat7 contains
    the '%' character (LP: #1666570).
  * Fix javax.servlet.jsp POM to use servlet-api version 3.0 instead of
    2.2 (LP: #1664179).

 -- Joshua Powers <email address hidden> Wed, 22 Mar 2017 13:42:56 -0600

[Brian Murray (brian-murray)]

Hello Andy, or anyone else affected,

Accepted tomcat8 into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/tomcat8/8.0.37-1ubuntu0.2 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 on 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!

[Brian Murray (brian-murray)]

I've clicked the rebuild button on Launchpad for tomcat7 for yakkety and xenial to see if it was a temporary failure.

[Brian Murray (brian-murray)]

Hello Andy, or anyone else affected,

Accepted tomcat8 into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/tomcat8/8.0.32-1ubuntu1.4 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 on 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!

[Brian Murray (brian-murray)]

Well, clicking rebuild didn't fix anything - sorry!

[Andy Wagner (rarian)]

Tested and verified tomcat7 7.0.52-1ubuntu0.11 and tomcat8 8.0.32-1ubuntu1.4 on Trusty and Xenial.

[Launchpad Janitor (janitor)]

This bug was fixed in the package tomcat8 - 8.0.32-1ubuntu1.4

---------------
tomcat8 (8.0.32-1ubuntu1.4) xenial; urgency=medium

  * Fix an upgrade error when JAVA_OPTS in /etc/default/tomcat8
    contains the '%' character (LP: #1666570).

 -- Joshua Powers <email address hidden> Thu, 09 Mar 2017 14:38:04 -0700

[Marc Deslauriers (mdeslaur)]

Could someone please test the tomcat8 yakkety package so I can follow it with a security update? Thanks!

[Joshua Powers (powersj)]

Tomcat8 in proposed (8.0.37-1ubuntu0.2) verified per instructions under test case.

[Joshua Powers (powersj)]

Sorry, hit post too soon... Tomcat8 in yakkety proposed (8.0.37-1ubuntu0.2) verified per instructions under test case.

[Launchpad Janitor (janitor)]

This bug was fixed in the package tomcat8 - 8.0.37-1ubuntu0.2

---------------
tomcat8 (8.0.37-1ubuntu0.2) yakkety; urgency=medium

  * Fix an upgrade error when JAVA_OPTS in /etc/default/tomcat8
    contains the '%' character (LP: #1666570).

 -- Joshua Powers <email address hidden> Tue, 28 Mar 2017 16:46:16 -0700

[Mathew Hodson (mhodson)]

tomcat7 on Xenial and Yakkety failed to build.

[Joshua Powers (powersj)]

Marking yakkety as won't fix due to EOL status.

[Joshua Powers (powersj)]

Removing verification-needed, as all verification is complete on this bug. Working on LP: #1664179 next.

[Per Lundberg (perlun)]

Note: as implied by Mathew Hodson, the tomcat7 package for Xenial has _not_ yet been updated (because the package failed to build.)

(We started seeing this bug appearing on some servers after upgrading to tomcat7, so I can personally verify that it's still present with 7.0.68-1ubuntu0.4.)

It would be great if the tomcat7 package could be fixed, because right now it's uninstallable under certain scenarios.

[Sebastian (slovdahl)]

This bug causes tomcat7 postinst to fail if GC logging with rotation is enabled in /etc/default/tomcat7, e.g. like this:

JAVA_OPTS="${JAVA_OPTS} -Xloggc:/var/log/tomcat7/gc-%t.log"

[Per Lundberg (perlun)]

One way to work around this until the package gets fixed in xenial is to escape the % sign in the JAVA_OPTS string:

JAVA_OPTS="${JAVA_OPTS} -Xloggc:/var/log/tomcat7/gc-\%t.log"

This has been tested and seems to make both the postinst script work, as well as make the JVM include the timestamp in the GC log file at runtime.