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

static file server #2218

Merged
merged 1 commit into from
Mar 14, 2024
Merged

static file server #2218

merged 1 commit into from
Mar 14, 2024

Conversation

laduke
Copy link
Contributor

@laduke laduke commented Feb 5, 2024

this lets you host web apps out of

:9993/app/{app_name}
:9993/app/{other_app}
etc

from $ZT_HOME/app/{app_name}

@laduke
Copy link
Contributor Author

laduke commented Feb 9, 2024

waiting on #2220

@laduke laduke force-pushed the controller-ui-server branch from faffaf2 to 5d33988 Compare February 9, 2024 17:26
@laduke laduke force-pushed the controller-ui-server branch from 5d33988 to 886c075 Compare February 22, 2024 19:48
@laduke laduke added this to the 1.14.0 milestone Feb 29, 2024
@laduke laduke force-pushed the controller-ui-server branch from 886c075 to cb0c3f3 Compare February 29, 2024 18:42
@laduke laduke changed the base branch from main to dev February 29, 2024 18:43
@dch
Copy link
Contributor

dch commented Mar 3, 2024

hmm liking the look of this

@laduke laduke force-pushed the controller-ui-server branch from cb0c3f3 to c852a40 Compare March 4, 2024 21:55
@joseph-henry
Copy link
Contributor

Tested and this seems to work. Ready to merge?

@laduke
Copy link
Contributor Author

laduke commented Mar 4, 2024

thanks for checking. let me remove all these per-request printfs actually.

@laduke laduke force-pushed the controller-ui-server branch from cbb8c46 to 74d24ff Compare March 4, 2024 23:35
@laduke laduke marked this pull request as ready for review March 4, 2024 23:35
@laduke laduke changed the title wip - static file server static file server Mar 4, 2024
@laduke laduke force-pushed the controller-ui-server branch from 74d24ff to b295100 Compare March 4, 2024 23:39
@laduke
Copy link
Contributor Author

laduke commented Mar 4, 2024

If you're interested in checking on a specific platform:

(start zerotier-one)

cd $ZT_HOME
sudo mkdir  app/hello
echo "hello" | sudo tee app/hello/index.html
curl -L localhost:9993/app/hello

joseph-henry
joseph-henry previously approved these changes Mar 5, 2024
Copy link
Contributor

@joseph-henry joseph-henry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and still seems to work

@laduke laduke force-pushed the controller-ui-server branch 7 times, most recently from 7a58da3 to fe39db9 Compare March 11, 2024 16:14
@dch
Copy link
Contributor

dch commented Mar 13, 2024

LGTM too here. I'm interested to know what the original & intended use case for this.

Some comments, for docs:

  • the dir only needs to be readable by root, so you can chown dch $ZT_HOME/app and it still works just fine

  • when no index.html is found, please return 404 instead of 500
    e.g.GET /app/missing/ should return 404

  • please consider documenting/allowing the the directory location configurable.
    I tested it with a softlink and it seeems to work, but a nice config option
    would be great, you know we will be asking for it anyway ;-)

  • I did basic path traversal checks, what (if any) security guarantees are on offer here?

  • It seems urls with .. or %2e%2e are resolved "safely", and trying to escape with
    http://localhost:9994/app/../../usr/share/doc/ntp/index.html or
    http://localhost:9994/app/%2e%2e/%2e%2e/usr/share/doc/ntp/index.html all fail
    which is great

  • you have some support for MIME types, e.g. index.json is returned as `application/json, nice

@laduke
Copy link
Contributor Author

laduke commented Mar 13, 2024

Thanks for testing it out!
We're relying on https://github.com/yhirose/cpp-httplib for the heavy lifting, including not allow traversal. It was already included in this codebase.

More news and info Coming Soon

this lets you host web apps out of
:9993/app/{app_name}
:9993/app/{other_app}

from $ZT_HOME/app/{app_name}
@laduke laduke force-pushed the controller-ui-server branch from 63afe7b to b4eb39f Compare March 13, 2024 19:39
Copy link
Contributor

@glimberg glimberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@laduke laduke merged commit b4eb39f into dev Mar 14, 2024
5 checks passed
@laduke laduke deleted the controller-ui-server branch March 14, 2024 17:08
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