Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
hhartzer opened this issue May 28, 2024 · 5 comments · Fixed by #67
Closed

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

hhartzer opened this issue May 28, 2024 · 5 comments · Fixed by #67

Comments

@hhartzer
Copy link
Contributor

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
Copy link
Contributor

g-rden commented May 29, 2024

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

darkhttpd/darkhttpd.c

Lines 1891 to 1893 in 5031bf1

if (strcmp((*((const struct dlent * const *)a))->name, "..") == 0) {
return -1; /* Special-case ".." to come first. */
}
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
Copy link
Contributor

g-rden commented May 29, 2024

g-rden@914c196
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
Copy link
Contributor Author

Very nice find! I'm good with that.

@emikulic
Copy link
Owner

I don't mind about the date. :)

@g-rden g-rden mentioned this issue May 30, 2024
emikulic pushed a commit that referenced this issue May 30, 2024
No longer includes ".." when sorting the directory entries for the directory listing, but adds it on top of the listing if the requested directory is not wwwroot. Besides fixing the sorting bug, this should make the sorting faster.

This also does not print the mod date for the ".." entry anymore.

Closes #66
@hhartzer
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants