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

Optional file browser #2661

Closed
1 task done
ahopkins opened this issue Jan 24, 2023 · 6 comments · Fixed by #2662
Closed
1 task done

Optional file browser #2661

ahopkins opened this issue Jan 24, 2023 · 6 comments · Fixed by #2662

Comments

@ahopkins
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe.

No response

Describe the solution you'd like

An auto-generated file browser and index.html fallback.

  1. When enabled, if serving a static dir check for existence of index.html and serve that
  2. If it does not exist, and autoindex is also enabled, show a basic file browser

Additional context

See #893

@Tronic
Copy link
Member

Tronic commented Jan 27, 2023

With the current draft both features are separately opt-in, and sanic somedir --simple enables only the file browser. I see two main use cases:

  1. Webapps that serve their front as index.html i.e. app.static('/', './static/', index_name='index.html')
  2. Simple web server or otherwise browsing files, where file browser is enabled.

In the first case the file browser should still stay disabled by default (you don't want people on the Internet to browse the files of your webapp). In the latter case, I find myself entering folders such as coverage reports, where there is index.html, and would find it helpful to have it opened when possible rather than getting a directory listing first, so in this case both features should be enabled.

@ahopkins
Copy link
Member Author

  1. I believe that already is the logic. I agree.
  2. That also should be the case. I'm pretty sure I updated the simple server to behave that way.

@ahopkins
Copy link
Member Author

Or, are you saying that adding autoindex should itself also enable index.html? This was the orig pattern.

@Tronic
Copy link
Member

Tronic commented Jan 27, 2023

We'll have to consider how those arguments on static() are implemented (and their names) but sanic --simple should enable both.

@ahopkins
Copy link
Member Author

I updated this to work OOTB to enable index.html if autoindex=True. We will document to say that you can explicitly disable this by passing index_name=None.

@Tronic
Copy link
Member

Tronic commented Feb 5, 2023

Note: The implementation changed substantially this week, prior to merging. Most of the discussion in this issue and PR pre-date those changes and are no longer correct nor very relevant.

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

Successfully merging a pull request may close this issue.

2 participants