2018-03-24 17:29 GMT+01:00 Luca Toscano <toscano.l...@gmail.com>: > > > 2018-03-23 19:01 GMT+01:00 Joe Orton <jor...@redhat.com>: > >> On Fri, Mar 16, 2018 at 11:50:17AM +0100, Luca Toscano wrote: >> > From my point of view, adding a comment nearby a directive (except in >> some >> > cases like you explained above) should be totally safe and transparent >> to >> > the user. I haven't ever thought about the possibility that having a >> inline >> > comment could be dangerous, and in my opinion we should enforce this >> vision >> > and explicitly document when it is not possible it and why. >> > >> > The above is my naive view though (after working on this project for a >> very >> > short time) so I'd really like to know what's your angle about not >> > encouraging inline comments (pretty sure that there are use cases that I >> > didn't think of, and that might be good to be documented). >> >> I'd be fine with making in-line comments 100% safe (stripped) >> everywhere. I'd think I'd also be fine with making inline comments a >> config error in all cases, or increasing the X% of cases where that's an >> error already. >> >> I'm not happy about increasing (but to still below 100%) the places >> where comments are silently stripped, leaving the remaining places where >> comments might be a security issue (as in Require host foo#bar). I'm >> worried this will *increase* the risk of security issues as users become >> accustomed to using in-line comments. >> > > Thanks a lot for the explanation, now I completely got what you mean. I > was convinced that inline comments were safe for all directives, and I > think that a lot of our users already think this too (I was pinged by a > colleague that was puzzled about the ServerAlias behavior). I came up with > the following code patch that introduces a new function in utils.c, heavily > inspired by your patches (I felt bad to say copy/pasted :) that could help: > > http://people.apache.org/~elukey/httpd-trunk-core_ > server_alias_comment.patch > > Still not 100% tested, but it seems to work for ServerAlias (might need > more development time, comments welcome!). Reviewing all the directives to > apply this though might become a big burden, and potentially introduce new > bugs in 2.4 that we don't want. On the other side, the simplest solution of > raising an error when inline comments are used might be better, but we'd > risk to break existing working configurations when users upgrade to the new > release.. > > I don't have a strong position on this, I am dumping thoughts more than > giving solutions, really interested in what others think. >
If anybody still has patience and wants to review some code, I tried to update my patch to swap Joe's changes for forward_dns_check_authorization with a call to ap_getword_conf_nocomment: http://people.apache.org/~elukey/httpd-trunk-core_server_alias_comment.patch Tried to test it with 'Require forward-dns' and it seems working fine.. Luca
