emikulic / darkhttpd

When you need a web server in a hurry.
https://unix4lyfe.org/darkhttpd/
ISC License
1.03k stars 83 forks source link

File starting with ' shows up before ../ in the directory listing (in rare cases) #66

Closed hhartzer closed 4 months ago

hhartzer commented 4 months ago

Still not sure what's going on, but here's a sample directory listing I was able to generate with 1.16 and master.

This doesn't reproduce with just any file with ' for me.

<!DOCTYPE html>
<html>
<head>
<title>/foodir/fooer/</title>
<meta name="viewport" content="width=device-width, initial-scale=1">
</head>
<body>
<h1>/foodir/fooer/</h1>
<pre>
<a href="[%27The%20Trial%20of%20Socrates%27%20%281971%29-x298jsd.mp4](view-source:http://localhost:8080/foodir/fooer/%27The%20Trial%20of%20Socrates%27%20%281971%29-x298jsd.mp4)">&apos;The Trial of Socrates&apos; (1971)-x298jsd.mp4</a>                                              2024-05-28 20:45          0
<a href="[%281080p](view-source:http://localhost:8080/foodir/fooer/%281080p)">(1080p</a>                                                                                  2024-05-28 20:46          0
<a href="[../](view-source:http://localhost:8080/foodir/)">..</a>/                                                                                     2024-05-28 20:46
<a href="[%26](view-source:http://localhost:8080/foodir/fooer/%26)">&amp;</a>                                                                                       2024-05-28 20:46          0
<a href="[%27The](view-source:http://localhost:8080/foodir/fooer/%27The)">&apos;The</a>                                                                                    2024-05-28 20:46          0
<a href="[%28](view-source:http://localhost:8080/foodir/fooer/%28)">(</a>                                                                                       2024-05-28 20:46          0
(snip)

As you can see, ../ is not the first entry like it normally is.

g-rden commented 4 months ago

It took me a while but it's very obvious in hindsight.

https://github.com/emikulic/darkhttpd/blob/5031bf1555649df4194cccb1c99d186f8e3f4566/darkhttpd.c#L1891-L1893 When the entries get sorted we check if the first argument of the compare function is "..", but we never check if the second argument is "..".

So ".." was never sorted correctly, the ascii value of ".." is small so it happens to be the first entry most of the time.

I am not sure how to do this best. Adding the check for the second argument would be an easy fix, but I am sure there is a better way.

    if (strcmp((*((const struct dlent * const *)b))->name, "..") == 0) {
        return 1;  /* Special-case ".." to come first. */
    }
g-rden commented 4 months ago

https://github.com/g-rden/darkhttpd/commit/914c1966f0de3164af77ddaac35cd2ed2919650b This should be better in every way anyway (i hope). Does it matter that the date for ".." is now not listed? It could be added but afaik this is the normal way to display it anyway. For example https://dl-cdn.alpinelinux.org/alpine/

hhartzer commented 4 months ago

Very nice find! I'm good with that.

emikulic commented 4 months ago

I don't mind about the date. :)

hhartzer commented 4 months ago

Awesome, thank you for fixing this @g-rden!