Thanks Wesley, this looks good.

Some minor changes please:

1) Please could you add an Origin: header to the debdiff? Something like
"Origin: upstream,
https://github.com/apache/httpd/commit/f0529e54b8d889322b5113eb623e263556bfa28e";
or "Origin: backport,
https://github.com/apache/httpd/commit/f0529e54b8d889322b5113eb623e263556bfa28e";
depending on whether you had to tweak the diff or not. Sorry I didn't
mention this on IRC before - I saw the Closes/LP thing on a quick glance
but I didn't review properly.

2) Please could you note what's going on with the new
DirectoryCheckHandler directive in the debian/changelog entry? In
particular, I think it should answer: should users expect any behaviour
changes when updating and not changing anything; and what users need to
do to fix the bug after the update is applied (set DirectoryCheckHandler
to something?)

3) Given that the directive is being added, it should probably be noted
in the Regression Potential section, along with stating whether default
behaviour is being changed or not, and we should probably add a test
case to the Test Case section to make sure that the path that shouldn't
change is also exercised to ensure that it indeed hasn't changed.

I think I'm asking for 2 and 3 really because I don't feel that I yet
have clarity on exactly which way round the new directive works, and
what expectations are with regard to behaviour changes on the SRU update
(rather than against 2.2). I understand that parity against 2.2 is
needed to fix the bug, but from an SRU and regression perspective it's
behaviour changes on the SRU update itself that I'm bothered about.
Essentially, it comes down to: how are we ensuring that no user will
scream at us for breaking behaviour in pushing this SRU?

I'm sorry that this is a bit more complicated that I originally expected
when I asked you to look at this because of the behaviour issue.

With these changes, assuming it builds and you've tested it then I'm
happy to sponsor an upload. Thanks!

Log from IRC:

11:30 <rbasak> mdeslaur: around? I'm looking at sponsoring bug 1394403 -
as you're looked at it before I'd like your opinion please.

11:30 <ubottu> bug 1394403 in apache2 (Ubuntu Trusty) "RewriteRule of
"^$" is broken" [Medium,Confirmed] https://launchpad.net/bugs/1394403

11:31 <rbasak> when I asked magicalChicken to look at it I didn't
realise the upstream fix would add a configuration directive. But it
looks like it's safe as it defaults to the same behaviour. Had you
considered this already? Does it also look reasonable to you?

11:31 <rbasak> I also think we should include the documentation update
in our backport - better than not having it in the SRU IMHO.

12:10 <mdeslaur> rbasak: I'm ok with the new config option...I've added
options before to packages as security updates, so it's not like we
haven't done it before. The option will change the behaviour though, but
cases where it will break something are unlikely

12:10 <mdeslaur> rbasak: for the documentation, meh...if it were man
pages, I'd push for it...but the static web documentation, meh

12:10 <mdeslaur> rbasak: especially since there are localized versions
of the documentation and we'd only be updating the english version

12:11 <mdeslaur> rbasak: the only thing is perhaps add what the option
is and how the default has changed to the changelog

12:16 <rbasak> mdeslaur: OK. Thanks!

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1394403

Title:
  RewriteRule of "^$" is broken

To manage notifications about this bug go to:
https://bugs.launchpad.net/apache2/+bug/1394403/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to