emikulic / darkhttpd

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

Add --single-file flag #63

Closed g-rden closed 6 months ago

g-rden commented 6 months ago

Serves a single file instead of a whole directory.

I also added it the How to run section of the read me. Maybe it should be mentioned in the feature list too. Also adding tests for this flag might be a good idea. I can look into that if it's needed.

closes #23

hhartzer commented 6 months ago

This is nice! I think it looks good, although a basic test would be nice. Shouldn't be too hard to add and verify. Let me know if you want any pointers with that, should you choose to go that route.

g-rden commented 6 months ago

Okay, I have no idea how run-test, test.py and the test_*.py work together.

Most of the tests would be for --single-file --chroot, but there are currently no tests for chroot and you said the tests don't work as root anyway.

We could do something like test_password_equal.c, i.e. check variables and disable chroot() and chdir() erroring with a macro. Currently this would also not work, because the chroot processing is in main(). I think this would be nice, but way overkill.

Now that I think about it, @emikulic can we put the chroot processing in its own function?

hhartzer commented 6 months ago

Are you able to run make test successfully? I would start there and make sure it's running correctly for you.

Privilege dropping functionality in its own function might not be a bad idea, but not sure if there's any global state to consider.

Could always test just --single-file without chroot, too.

emikulic commented 6 months ago

Okay, I have no idea how run-test, test.py and the test_*.py work together.

Yeah, it's a mess. We can merge this first and add tests in a followup.

Most of the tests would be for --single-file --chroot, but there are currently no tests for chroot and you said the tests don't work as root anyway.

I don't think it's worth testing chroot :(

We could do something like test_password_equal.c, i.e. check variables ~and disable chroot() and chdir() erroring with a macro~. Currently this would also not work, because the chroot processing is in main(). I think this would be nice, but way overkill.

Now that I think about it, @emikulic can we put the chroot processing in its own function?

If it makes the code nicer, yeah, extract it to a helper function.

emikulic commented 6 months ago

Do you want to squash these commits or do you prefer I do it at merge time?

emikulic commented 6 months ago

To clarify: don't worry about testing the chroot() syscall. Pull out the string manipulation code into a function and write a unit-test like test_password_equal.c or test_make_safe_uri.c

Maybe that's what you meant in the first place, and it took me until now to interpret it.

g-rden commented 6 months ago

We can merge this first and add tests in a followup.

Yes, I still haven't look much into and don't understand the python files or python in general. I don't think I want to do the tests.

Do you want to squash these commits or do you prefer I do it at merge time?

Please do when merging.

To clarify: don't worry about testing the chroot() syscall. Pull out the string manipulation code into a function and write a unit-test like test_password_equal.c or test_make_safe_uri.c

Okay, if we leave chroot aside, --single-file can be tested with checking 4 server responses. Requests are already being checked in the python files. For consistancy I don't think creating a new c file is the way. But like i said I am not the right person for the job.

it took me until now to interpret it.

Yeah, sorry