You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jk...@apache.org on 2014/10/30 09:34:13 UTC
svn commit: r1635428 - in /httpd/httpd/trunk: include/http_core.h
server/core.c server/request.c
Author: jkaluza
Date: Thu Oct 30 08:34:12 2014
New Revision: 1635428
URL: http://svn.apache.org/r1635428
Log:
core: Do not match files when using DirectoryMatch. PR41867.
Modified:
httpd/httpd/trunk/include/http_core.h
httpd/httpd/trunk/server/core.c
httpd/httpd/trunk/server/request.c
Modified: httpd/httpd/trunk/include/http_core.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1635428&r1=1635427&r2=1635428&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_core.h (original)
+++ httpd/httpd/trunk/include/http_core.h Thu Oct 30 08:34:12 2014
@@ -619,6 +619,7 @@ typedef struct {
unsigned int allow_encoded_slashes_set : 1;
unsigned int decode_encoded_slashes_set : 1;
+ unsigned int d_is_directory : 1;
/** Named back references */
apr_array_header_t *refs;
Modified: httpd/httpd/trunk/server/core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1635428&r1=1635427&r2=1635428&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core.c (original)
+++ httpd/httpd/trunk/server/core.c Thu Oct 30 08:34:12 2014
@@ -2224,6 +2224,7 @@ static const char *dirsection(cmd_parms
conf->r = r;
conf->d = cmd->path;
conf->d_is_fnmatch = (apr_fnmatch_test(conf->d) != 0);
+ conf->d_is_directory = 1;
if (r) {
conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
Modified: httpd/httpd/trunk/server/request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1635428&r1=1635427&r2=1635428&view=diff
==============================================================================
--- httpd/httpd/trunk/server/request.c (original)
+++ httpd/httpd/trunk/server/request.c Thu Oct 30 08:34:12 2014
@@ -1216,6 +1216,13 @@ AP_DECLARE(int) ap_directory_walk(reques
pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
}
+ /* core_dir_config is Directory*, but the requested file is
+ * not a directory, so although the regexp could match,
+ * we skip it. */
+ if (entry_core->d_is_directory && r->finfo.filetype != APR_DIR) {
+ continue;
+ }
+
if (ap_regexec(entry_core->r, r->filename, nmatch, pmatch, 0)) {
continue;
}
Re: svn commit: r1635428 - in /httpd/httpd/trunk: include/http_core.h
server/core.c server/request.c
Posted by Jan Kaluža <jk...@redhat.com>.
On 01/22/2015 12:22 AM, William A. Rowe Jr. wrote:
> On Mon, 19 Jan 2015 16:28:46 -0600
> "William A. Rowe Jr." <wr...@rowe-clan.net> wrote:
>
>> On Sun, 18 Jan 2015 23:00:10 -0500
>> Eric Covener <co...@gmail.com> wrote:
>>
>>> On Thu, Oct 30, 2014 at 4:34 AM, <jk...@apache.org> wrote:
>>>>
>>>> + /* core_dir_config is Directory*, but the requested
>>>> file is
>>>> + * not a directory, so although the regexp could
>>>> match,
>>>> + * we skip it. */
>>>> + if (entry_core->d_is_directory &&
>>>> r->finfo.filetype != APR_DIR) {
>>>> + continue;
>>>> + }
>>>> +
>>>> if (ap_regexec(entry_core->r, r->filename, nmatch,
>>>> pmatch, 0)) { continue;
>>>> }
>>>
>>> I think this is broken.
>>
>> You are correct, I don't think it means what the author thought this
>> code means.
>
> Indeed, it was nonsense. The fix begins with some code similar to
> [attached] but there are a number of shortcuts in the code that would
> need to be accounted for by expanding the dir_len at the appropriate
> time. The patch is also insufficient in that the r->filename buffer
> needs to be at least one extra byte to compensate for the 'maybe a
> trailing slash, maybe not' on a final directory component in a file
> name.
Hi,
you are both right, sorry for the patch. I will revert it from trunk (if
not already done by someone else). That part of httpd looks more
complicated that I thought previously.
Jan Kaluza
> That said, I'm putting this out there to get folks thinking, I can't
> spend much more time on it myself, today.
>
> Cheers,
>
> Bill
>
Re: svn commit: r1635428 - in /httpd/httpd/trunk:
include/http_core.h server/core.c server/request.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Mon, 19 Jan 2015 16:28:46 -0600
"William A. Rowe Jr." <wr...@rowe-clan.net> wrote:
> On Sun, 18 Jan 2015 23:00:10 -0500
> Eric Covener <co...@gmail.com> wrote:
>
> > On Thu, Oct 30, 2014 at 4:34 AM, <jk...@apache.org> wrote:
> > >
> > > + /* core_dir_config is Directory*, but the requested
> > > file is
> > > + * not a directory, so although the regexp could
> > > match,
> > > + * we skip it. */
> > > + if (entry_core->d_is_directory &&
> > > r->finfo.filetype != APR_DIR) {
> > > + continue;
> > > + }
> > > +
> > > if (ap_regexec(entry_core->r, r->filename, nmatch,
> > > pmatch, 0)) { continue;
> > > }
> >
> > I think this is broken.
>
> You are correct, I don't think it means what the author thought this
> code means.
Indeed, it was nonsense. The fix begins with some code similar to
[attached] but there are a number of shortcuts in the code that would
need to be accounted for by expanding the dir_len at the appropriate
time. The patch is also insufficient in that the r->filename buffer
needs to be at least one extra byte to compensate for the 'maybe a
trailing slash, maybe not' on a final directory component in a file
name.
That said, I'm putting this out there to get folks thinking, I can't
spend much more time on it myself, today.
Cheers,
Bill
Re: svn commit: r1635428 - in /httpd/httpd/trunk:
include/http_core.h server/core.c server/request.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On Sun, 18 Jan 2015 23:00:10 -0500
Eric Covener <co...@gmail.com> wrote:
> On Thu, Oct 30, 2014 at 4:34 AM, <jk...@apache.org> wrote:
> > Author: jkaluza
> > Date: Thu Oct 30 08:34:12 2014
> > New Revision: 1635428
> >
> > URL: http://svn.apache.org/r1635428
> > Log:
> > core: Do not match files when using DirectoryMatch. PR41867.
> >
> >
> > Modified:
> > httpd/httpd/trunk/include/http_core.h
> > httpd/httpd/trunk/server/core.c
> > httpd/httpd/trunk/server/request.c
> >
> > Modified: httpd/httpd/trunk/include/http_core.h
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1635428&r1=1635427&r2=1635428&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/include/http_core.h (original) +++
> > httpd/httpd/trunk/include/http_core.h Thu Oct 30 08:34:12 2014 @@
> > -619,6 +619,7 @@ typedef struct {
> >
> > unsigned int allow_encoded_slashes_set : 1;
> > unsigned int decode_encoded_slashes_set : 1;
> > + unsigned int d_is_directory : 1;
> >
> > /** Named back references */
> > apr_array_header_t *refs;
> >
> > Modified: httpd/httpd/trunk/server/core.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1635428&r1=1635427&r2=1635428&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/server/core.c (original) +++
> > httpd/httpd/trunk/server/core.c Thu Oct 30 08:34:12 2014 @@ -2224,6
> > +2224,7 @@ static const char *dirsection(cmd_parms conf->r = r;
> > conf->d = cmd->path;
> > conf->d_is_fnmatch = (apr_fnmatch_test(conf->d) != 0);
> > + conf->d_is_directory = 1;
> >
> > if (r) {
> > conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
> >
> > Modified: httpd/httpd/trunk/server/request.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1635428&r1=1635427&r2=1635428&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/server/request.c (original) +++
> > httpd/httpd/trunk/server/request.c Thu Oct 30 08:34:12 2014 @@
> > -1216,6 +1216,13 @@ AP_DECLARE(int) ap_directory_walk(reques pmatch
> > = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t)); }
> >
> > + /* core_dir_config is Directory*, but the requested
> > file is
> > + * not a directory, so although the regexp could match,
> > + * we skip it. */
> > + if (entry_core->d_is_directory && r->finfo.filetype !=
> > APR_DIR) {
> > + continue;
> > + }
> > +
> > if (ap_regexec(entry_core->r, r->filename, nmatch,
> > pmatch, 0)) { continue;
> > }
> >
> >
>
> I think this is broken.
You are correct, I don't think it means what the author thought this
code means.
At this particular step we are testing the resulting filesystem resource
which will almost never be a directory anymore. In the earlier loop,
as we construct r->filename component-by-component, we probably could
have stashed the final parent directory path, updating that path until
r->finfo returned a non-directory result.
But at this point in the code filename consists of the full path and
file entity.
Re: svn commit: r1635428 - in /httpd/httpd/trunk: include/http_core.h
server/core.c server/request.c
Posted by Eric Covener <co...@gmail.com>.
On Sun, Jan 18, 2015 at 11:00 PM, Eric Covener <co...@gmail.com> wrote:
> On Thu, Oct 30, 2014 at 4:34 AM, <jk...@apache.org> wrote:
>> Author: jkaluza
>> Date: Thu Oct 30 08:34:12 2014
>> New Revision: 1635428
>>
>> URL: http://svn.apache.org/r1635428
>> Log:
>> core: Do not match files when using DirectoryMatch. PR41867.
>>
>>
>> Modified:
>> httpd/httpd/trunk/include/http_core.h
>> httpd/httpd/trunk/server/core.c
>> httpd/httpd/trunk/server/request.c
>>
>> Modified: httpd/httpd/trunk/include/http_core.h
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1635428&r1=1635427&r2=1635428&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/include/http_core.h (original)
>> +++ httpd/httpd/trunk/include/http_core.h Thu Oct 30 08:34:12 2014
>> @@ -619,6 +619,7 @@ typedef struct {
>>
>> unsigned int allow_encoded_slashes_set : 1;
>> unsigned int decode_encoded_slashes_set : 1;
>> + unsigned int d_is_directory : 1;
>>
>> /** Named back references */
>> apr_array_header_t *refs;
>>
>> Modified: httpd/httpd/trunk/server/core.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1635428&r1=1635427&r2=1635428&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/server/core.c (original)
>> +++ httpd/httpd/trunk/server/core.c Thu Oct 30 08:34:12 2014
>> @@ -2224,6 +2224,7 @@ static const char *dirsection(cmd_parms
>> conf->r = r;
>> conf->d = cmd->path;
>> conf->d_is_fnmatch = (apr_fnmatch_test(conf->d) != 0);
>> + conf->d_is_directory = 1;
>>
>> if (r) {
>> conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
>>
>> Modified: httpd/httpd/trunk/server/request.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1635428&r1=1635427&r2=1635428&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/server/request.c (original)
>> +++ httpd/httpd/trunk/server/request.c Thu Oct 30 08:34:12 2014
>> @@ -1216,6 +1216,13 @@ AP_DECLARE(int) ap_directory_walk(reques
>> pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
>> }
>>
>> + /* core_dir_config is Directory*, but the requested file is
>> + * not a directory, so although the regexp could match,
>> + * we skip it. */
>> + if (entry_core->d_is_directory && r->finfo.filetype != APR_DIR) {
>> + continue;
>> + }
>> +
>> if (ap_regexec(entry_core->r, r->filename, nmatch, pmatch, 0)) {
>> continue;
>> }
>>
>>
>
Arg. Sorry about previous incomplete email.
The new code blocks stuff like this, where "private" is in some
directory in the mapped path.
<DirectoryMatch private>
require all denied
</DirectoryMatch>
The PR is only referring to (IIUC) the case where we had e.g.
/private.txt mappd to the document root and the DirectoryMatch would
catch it in some cases.
--
Eric Covener
covener@gmail.com
Re: svn commit: r1635428 - in /httpd/httpd/trunk: include/http_core.h
server/core.c server/request.c
Posted by Eric Covener <co...@gmail.com>.
On Thu, Oct 30, 2014 at 4:34 AM, <jk...@apache.org> wrote:
> Author: jkaluza
> Date: Thu Oct 30 08:34:12 2014
> New Revision: 1635428
>
> URL: http://svn.apache.org/r1635428
> Log:
> core: Do not match files when using DirectoryMatch. PR41867.
>
>
> Modified:
> httpd/httpd/trunk/include/http_core.h
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/request.c
>
> Modified: httpd/httpd/trunk/include/http_core.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=1635428&r1=1635427&r2=1635428&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/http_core.h (original)
> +++ httpd/httpd/trunk/include/http_core.h Thu Oct 30 08:34:12 2014
> @@ -619,6 +619,7 @@ typedef struct {
>
> unsigned int allow_encoded_slashes_set : 1;
> unsigned int decode_encoded_slashes_set : 1;
> + unsigned int d_is_directory : 1;
>
> /** Named back references */
> apr_array_header_t *refs;
>
> Modified: httpd/httpd/trunk/server/core.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1635428&r1=1635427&r2=1635428&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/core.c (original)
> +++ httpd/httpd/trunk/server/core.c Thu Oct 30 08:34:12 2014
> @@ -2224,6 +2224,7 @@ static const char *dirsection(cmd_parms
> conf->r = r;
> conf->d = cmd->path;
> conf->d_is_fnmatch = (apr_fnmatch_test(conf->d) != 0);
> + conf->d_is_directory = 1;
>
> if (r) {
> conf->refs = apr_array_make(cmd->pool, 8, sizeof(char *));
>
> Modified: httpd/httpd/trunk/server/request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=1635428&r1=1635427&r2=1635428&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/request.c (original)
> +++ httpd/httpd/trunk/server/request.c Thu Oct 30 08:34:12 2014
> @@ -1216,6 +1216,13 @@ AP_DECLARE(int) ap_directory_walk(reques
> pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
> }
>
> + /* core_dir_config is Directory*, but the requested file is
> + * not a directory, so although the regexp could match,
> + * we skip it. */
> + if (entry_core->d_is_directory && r->finfo.filetype != APR_DIR) {
> + continue;
> + }
> +
> if (ap_regexec(entry_core->r, r->filename, nmatch, pmatch, 0)) {
> continue;
> }
>
>
I think this is broken. Consider this config:
I misin
--
Eric Covener
covener@gmail.com