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

Added basic directory listing fixing #32. Added 301 responses. #35

Closed
wants to merge 67 commits into from

Conversation

triforce
Copy link
Contributor

@triforce triforce commented Nov 3, 2017

  • Extended thread buffer by 2048
  • Added a very basic HTML formatted response with a list of the directory contents
  • When request for a directory is a non forward slash terminated request a 301 is sent to the client, triggering their browser to request a forward slash terminated request.

nemasu and others added 30 commits January 28, 2014 02:18
Added xml, xhtml, gif, png, jpeg, css, and javascript content types.
Changed thread memory size to something reasonable.
Added simple request logging.
Added removal of '../' in URL.
Formating
For ports <1024.
* Fixed SIGPIPE when transfer is cancelled.
* Added a more useful error on bind failure.
* Fixed 206 size calculation.
* Combined seek & get file size system calls.
- Fixed 206 max length bug.
- Commented out simple request logging, uncomment in main.asm to enable.
-Added 400, 413, 416 responses.
-Fixed header processing bug.
@nemasu
Copy link
Owner

nemasu commented Nov 20, 2017

Sorry for the delay, I got around to testing it a bit.

I ran into a problem when I did something like this:
cd web_root; mkdir dir1; touch dir1/{a,b,c}

If I go to localhost/dir1 the listing works, but when I click on one of them,
it tries to go to localhost/dir1/a/ and then 404s instead of downloading a.

@triforce
Copy link
Contributor Author

It think's 'a' is a directory for some reason. I will look into it.

@triforce
Copy link
Contributor Author

triforce commented Dec 5, 2017

Could you retest please @nemasu? It turned out to be quite a simple fix in the end!

@nemasu
Copy link
Owner

nemasu commented Jan 17, 2018

Hello, sorry for the super late reply.
I did some retesting, seems to be working so far!

There's 2 potential things though:
-Putting the previous directory ("...") at the top of the list.
-If 'index.html' exists in a directory, display that as default.

What do you think?

@triforce
Copy link
Contributor Author

Yes sounds fine to me. So presumably when in the web_root there would be no previous directory link shown? Other web servers have directory listing as a config option as well so perhaps we should start planning a way to turn features on and off after this?

@nemasu
Copy link
Owner

nemasu commented Jan 23, 2018

Yeah that makes sense.
Funny you should mention config options, I was just thinking about how to go about disabling features to reduce binary size.

@nemasu
Copy link
Owner

nemasu commented Jan 24, 2018

To make thing easier though, I think we should do optional features as a separate issue after merging the directory code.

@triforce
Copy link
Contributor Author

Yep makes sense.

@triforce
Copy link
Contributor Author

triforce commented Oct 3, 2018

@nemasu feel free to make those changes, I haven't had time recently unfortunately.

@nemasu
Copy link
Owner

nemasu commented Oct 4, 2018

Okay, I'll see if I can.

@triforce
Copy link
Contributor Author

This is teetering on the edge of being finished @nemasu. There is just these two issues which you raised outstanding:

-Putting the previous directory ("...") at the top of the list.
-If 'index.html' exists in a directory, display that as default.

If anyone has some time then feel free to jump in and implement those two issues.

@triforce
Copy link
Contributor Author

triforce commented Feb 13, 2020

@nemasu after studying the code more closely it looks like requests to the root directory (and nothing else) adds the default document (index.html), so as part of the outstanding changes still required here we should remove that logic and integrate it with the directory listing functionality.

@nemasu
Copy link
Owner

nemasu commented Apr 9, 2023

Hello @triforce,
I'm finally looking at getting this committed, there are merge conflicts however since it's been ... a while, to say the least (sorry about that).

I've made your changes into a patch file relative to the current HEAD, if you like, you can close this PR and make a new one with the updated patch, and I'll accept it and work on the additions after. Of course, if you want, you can use this PR too, I just don't really know an easy way to do that.

directory_support.patch

@nemasu
Copy link
Owner

nemasu commented Apr 13, 2023

Merged in PR #58 .

@nemasu nemasu closed this Apr 13, 2023
@nemasu nemasu mentioned this pull request Apr 13, 2023
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 this pull request may close these issues.

4 participants