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

fix: Embed CSS files into templates #678

Closed
wants to merge 708 commits into from

Conversation

ppodolsky
Copy link
Contributor

There are two changes:

  • Small bugfix for the bug I did in gateway code. Redirecting to index.html does not require trailing slash
  • Accessing files thought subdomain paths makes impossible to use paths like /style.css for loading resources. I mitigated it by embedding CSS into templates. Original files are left here intentionally for easier development in the future.

b5 and others added 30 commits October 14, 2022 18:07
…iroh` commands (n0-computer#348)

* docs: add full help text for `lookup`, `connect`, `get`, `p2p`, and
`iroh` commands

* move long descriptions into constants

Co-authored-by: b5 <[email protected]>
flub and others added 3 commits January 4, 2023 20:07
We used to use the same config for the service and server (aka the
binary).  This is confusing when creating configs to use with
e.g. iroh-one, iroh-embed or iroh-share because some fields are not
used.

This splits off the config structs to avoid this problem, services now
have a Config and binaries a ServerConfig.  This allows creating the
services standalone without all the baggage a server needs.

While this isn't many fields yet, this will get worse as we add more
features (this is split off from another PR where this seemed useful).
This removes the use of mockall which is causing a lot of trouble with
the cargo features intricacies it brings with it.  A lot of the
tooling struggles with these different versions of the structs which
also behave differently in surprising ways.

The tests affected by this will be converted into end-to-end tests as part of
is now unused.  This is possibly only temprorary and that might come
back when the end-to-end tests are added.
@ppodolsky ppodolsky changed the title Embed CSS files into templates fix: Embed CSS files into templates Jan 9, 2023
Copy link
Contributor

@rklaehn rklaehn left a comment

Choose a reason for hiding this comment

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

So this removes the assets from ../assets/style.css and ../assets/icons.css and includes them in the dir_list and 404 page.

Not sure if this is an improvement. It simplifies the routes for sure, but now you have to "recompile" the css and icons to html whenever you want to do a change.

@ppodolsky
Copy link
Contributor Author

ppodolsky commented Jan 9, 2023

So this removes the assets from ../assets/style.css and ../assets/icons.css and includes them in the dir_list and 404 page.

Not sure if this is an improvement. It simplifies the routes for sure, but now you have to "recompile" the css and icons to html whenever you want to do a change.

It is not an improvement but a required fix. There is a subdomain handler that accepts urls such as https://bafy<...>.ipfs.localhost:8080/style.css. This request is routed to bafy<...>/style.css, not to custom axum handler in gateway. So there is no way within subdomain handlers to request custom files because any file can be a child of Unixfs directory.

@dignifiedquire
Copy link
Contributor

How does Kubo do this currently?

@ppodolsky
Copy link
Contributor Author

ppodolsky commented Jan 9, 2023

Kubo embeds all assets.
I thought about putting assets to a separate subdomain but it would lead to all kinds of CORS issues on loading.

@fabricedesre
Copy link
Contributor

Kubo embeds all assets. I thought about putting assets to a separate subdomain but it would lead to all kinds of CORS issues on loading.

In general css & images loads are not subject to SOP restrictions so you should not need CORS. But it should also be possible to serve them from roo_url/static/xxx and set the CORS headers on all of /static/

@ppodolsky
Copy link
Contributor Author

You mean second-level domain by root_url? We need to embed JS snippet for detecting root url then, is it ok?

@fabricedesre
Copy link
Contributor

I think the base url is part of the config, so you could use it to generate the css url without needing a script.

@ppodolsky ppodolsky force-pushed the fix/gateway-path-and-css branch 2 times, most recently from f4d451f to 5c73ff7 Compare January 9, 2023 18:15
@ppodolsky
Copy link
Contributor Author

@fabricedesre Thanks for idea, done. However there is still small regression: by default, public_url_base is empty and users will have to set it manually for development environment

@ppodolsky ppodolsky force-pushed the fix/gateway-path-and-css branch from a1f2f69 to c63d0b6 Compare January 11, 2023 14:00
@ppodolsky
Copy link
Contributor Author

ppodolsky commented Jan 11, 2023

I've also fixed a bug here with rendering index.html if directory has it in the root.
Before, it merely pushed index.html to the end of path, without reloading of path_metadata and other attributes. It led to inconsistency and erratic behaviour. Now path_metadata is reloaded.

ppodolsky and others added 4 commits January 11, 2023 22:18
* feat: Add pub ctor to Directory

* fix: reduce logging level

* feat: Pretty printing for addrs

* fix: clippy

* pr fixes
)

* fix(iroh-share): force close gossip task on finish/done

* fmt

* fix
@ppodolsky
Copy link
Contributor Author

Let's review and merge?

@ramfox
Copy link
Contributor

ramfox commented Feb 20, 2023

Hello! The code that was previously hosted in this repository has been moved to n0-computer/beetle. beetle is in maintenance mode, but will still accept PRs.

If you are still interested in getting your PR merged, please re-open your PR on n0-computer/beetle.

Check out our blog post for more info on our new direction for iroh: https://n0.computer/blog/a-new-direction-for-iroh/

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.