Project

General

Profile

Actions

Bug #3220

closed

Allegedly redundant check not redundant

Added by ef 8 months ago. Updated 8 months ago.

Status:
Invalid
Priority:
Normal
Category:
mod_dirlisting
Target version:
-
ASK QUESTIONS IN Forums:
No

Description

In revision 9a5e1652, in mod_dirlisting (source:src/mod_dirlisting.c#L1462), the check for the physical path to be a directory was #if 0'd out as allegedly redundant:

#if 0 /* redundant check; not necessary */
    /* r->physical.path is a dir since it ends in slash, or else
     * http_response_physical_path_check() would have redirected
     * before calling handle_subrequest_start */
    if (!stat_cache_path_isdir(&r->physical.path)) {
[...]
    }
#endif
It is, however, not redundant as a previous handler (mod_magnet) may have altered the physical path after the check mentioned.

In my case, that led to inadvertent directory listings.
Removing the #if 0/#endif resolved the problem.

Actions #1

Updated by gstrauss 8 months ago

  • Status changed from New to Invalid
  • Target version deleted (1.4.xx)

It is, however, not redundant as a previous handler (mod_magnet) may have altered the physical path after the check mentioned.

If you make changes in your custom lua script run by mod_magnet, then you are responsible for those changes.

Since you did not say why you modified the physical path to include a trailing slash when the target was not a directory, I can only give you the advice "don't do that".

If you expected the modified request to be handled by mod_staticfile instead of mod_dirlisting, then you should read the mod_magnet documentation and use http-response-send-file in your mod_magnet lua script.

Actions #2

Updated by ef 8 months ago

gstrauss wrote in #note-1:

If you make changes in your custom lua script run by mod_magnet, then you are responsible for those changes.

I don't get that. One of the nice features of mod_magnet is you can change the physical path. Lots of examples do that.

Since you did not say why you modified the physical path to include a trailing slash when the target was not a directory, I can only give you the advice "don't do that".

Other way round: The script modifies the physical path (which originally had a trailing slash) to point to a regular file.

If you expected the modified request to be handled by mod_staticfile instead of mod_dirlisting, then you should read the mod_magnet documentation and use http-response-send-file in your mod_magnet lua script.

Except, at that point, I don't know whether I want the request to be handled as a static file, a directory listing, handled by a Perl FastCGI handler, a PHP FastCGI handler or whatever. I'd argue that's lighttpd's job to find out what's appropriate.

Think of my lua script as a generalized mod_indexfile. It actually does much more, but among other things, it will, in the case described, rewrite /foo/bar/ to /foo/bar/index.html, in which case I would expect that to be handled as a static file. It can also rewrite it to /foo/bar/index.htmpl, in which case it's handled by a Perl FastCGI handler using Template::Toolkit. Or to something else entirely.

Oh wow, there's even an example in the mod_magnet documentation doing the same thing. I guess it will also no longer work with that test #if 0'd out.

Actions #3

Updated by gstrauss 8 months ago

Except, at that point, I don't know whether I want the request to be handled as a static file, a directory listing, handled by a Perl FastCGI handler, a PHP FastCGI handler or whatever.

As I said before, if you don't know if the path is a directory, then you should not modify the path to end in '/' in your lua script. Don't do that.

Put another way, you are not a very good programmer. You should always start troubleshooting your own code, looking for your mistaken assumptions.

I'd argue that's lighttpd's job to find out what's appropriate.

lighttpd mod_magnet lets you write lua code. You can write lua code to do many obnoxious things and lighttpd can not and will not second-guess your lua code.

Actions #4

Updated by ef 8 months ago

As I said before, if you don't know if the path is a directory, then you should not modify the path to end in '/' in your lua script. Don't do that.

As I wrote before, I'm not doing that. I'm appending a file name to a (directory) path ending in a slash. Just like the lua mod_indexfile paragraph in ModMagnetExamples does (or so I think).

Actions #5

Updated by gstrauss 8 months ago

Put another way, you are not a very good programmer.

That remains true. You're modifying a physical path and at the same time you claim (above) that you are not sure how you would like that physical path to be handled.

Hint: do not make changes unless they are intentional changes and you understand the effects of making those changes.

If you are making changes and not getting the desired effects, then you are probably making a mistake in how/when you make those changes.

I wrote all the mod_magnet samples to provide examples how arbitrary custom logic could be implemented in lua scripts in mod_magnet. You can do all those things in lua scripts run by mod_magnet. If you are unable to make small modifications in the lua scripts in ways that then allow lighttpd C modules to handle the rest, then you might code more in lua script to do exactly what you want.

Some code in lighttpd operates on the request URL. Some code in lighttpd operates on filesystem paths. Take another look at what is modified by lua mod_indexfile

This is the lighttpd issue tracker and this is not an issue in lighttpd. You should post questions in the lighttpd forums and not in the lighttpd issue tracker. I won't be responding further here.

Actions

Also available in: Atom