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

HttpFileService could provide more options related to trailing slash #1655

Open
anuraaga opened this issue Mar 12, 2019 · 6 comments
Open

HttpFileService could provide more options related to trailing slash #1655

anuraaga opened this issue Mar 12, 2019 · 6 comments

Comments

@anuraaga
Copy link
Collaborator

I was trying to clean up code using the new file service capabilities (removing hacky path mapping to redirect everything to index.html for example) but found I can't remove all my hacks since we use "clean URLs" without a trailing slash to read index.html for that page. The current HttpFileService will read index.html for requests that end with / but not otherwise. Adding an option to HttpFileServiceBuilder that allows URLs without an extension to have slash added before resolving the file would make this simpler.

For reference, firebase's related setting
https://firebase.google.com/docs/hosting/full-config#control_trailing_slashes

@trustin
Copy link
Member

trustin commented Mar 14, 2019

Sounds like a good idea. Interested in sending a pull request?

@anuraaga
Copy link
Collaborator Author

@trustin I'm thinking of working on this now. I realized, the current HttpFileService already has a branch for where the path doesn't end with slash but the index exists, and it uses an HTTP redirect. Just wondering, is this a convention from other servers? It's a bit surprising behavior to me and I'm wondering if I can't just make it not use an HTTP redirect without adding a new option.

@trustin
Copy link
Member

trustin commented May 26, 2019

@anuraaga I didn't think much about redirection, but HttpServerHandler also makes use of redirection when received a path that does not end with slash:

https://github.com/line/armeria/blob/master/core/src/main/java/com/linecorp/armeria/server/HttpServerHandler.java#L461-L483

I guess I just wanted to make them consistent, but I think we can change that if other servers behave differently.

@anuraaga
Copy link
Collaborator Author

From what I've seen, I think most servers will give 404 and both of us may be being too nice to try to infer a response :)

I guess it's common for HttpFileService to be serving a static SPA, and redirecting the URL will often break those since they have their own routing config - I myself have been confused when this happened to me in the past. But consistency with HttpServerHandler seems more important. In that case, for the behavior in this issue it seems good to add a flag so users opt-in to behavior that does not follow HttpServerHandler's.

I'm also wondering though if we need to have a server-level flag that controls whether empty is redirected to slash, or slash is redirected to empty? I guess it's similar to the trailingSlash config parameter in the firebase config I linked, though that config option I've found to be a bit silly since it's happy to redirect index.js to index.js/ which I don't think anyone would ever want...

@trustin
Copy link
Member

trustin commented May 27, 2019

nginx and Apache HTTPd at least seem to send a redirect response when a client sent a request to a directory without a trailing slash (empty-to-slash), which matches the current HttpServerHandler behavior. I guess this kind of redirection is common for directories.

However, they indeed don't send a redirect when showing an index file. That being said, I guess we can:

  • keep the current HttpServerHandler behavior as-is
  • make HttpFileService redirect for empty-to-slash cases while serving an index file without redirection

Additionally, we might want to add some configuration property to Server, like TrailingSlashPolicy if we need more flexibility?

@anuraaga
Copy link
Collaborator Author

Thanks for checking the behavior! It sounds like they follow armeria's current behavior then. I guess I'll add a flag, and it's basically similar to doing this sort of config in nginx when needing slash-less URLs - https://stackoverflow.com/a/31189870

ikhoon added a commit to ikhoon/armeria that referenced this issue Jul 9, 2024
Motivation:

I personally prefer clean URL patterns without a trailing slash.
`/app/projects` is prefered over `/app/projects/`.  If I built
`/app/projects` with a Javascript framework, `/app/projects/index.html`
or `/app/projects.html` may be exported by the framework.

In `FileService`, `/app/projects/index.html` can be served by
`/app/projects/` path, but cannot be found by `/app/projects`. Handling
trailing slash is tricky and difficult to modify.

Alternatively, I propose an option that appends an extension if there is
no file for the request path. For example, a request sent to
`/app/projects` also finds `/app/projects.[ext]` as a fallback.

Relate links:

- line#4542
- line#1655
- https://ktor.io/docs/server-static-content.html#extensions

Modifications:

- Allow configuring `fileExtensions()` via `FileServiceBuilder`
- Find a file with fallback extensions if missing.

Result:

You can now set fallback file extensions to look up files in
`FileService`.

```java
FileService
  .builder(rootDir)
  .fileExtensions("html", "txt")
  ...
```
ikhoon added a commit to ikhoon/armeria that referenced this issue Jul 9, 2024
Motivation:

I personally prefer clean URL patterns without a trailing slash.
`/app/projects` is prefered over `/app/projects/`.  If I built
`/app/projects` with a Javascript framework, `/app/projects/index.html`
or `/app/projects.html` may be exported by the framework.

In `FileService`, `/app/projects/index.html` can be served by
`/app/projects/` path, but cannot be found by `/app/projects`. Handling
trailing slash is tricky and difficult to modify.

Alternatively, I propose an option that appends an extension if there is
no file for the request path. For example, a request sent to
`/app/projects` also finds `/app/projects.[ext]` as a fallback.

Relate links:

- line#4542
- line#1655
- https://ktor.io/docs/server-static-content.html#extensions

Modifications:

- Allow configuring `fileExtensions()` via `FileServiceBuilder`
- Find a file with fallback extensions if missing.

Result:

You can now set fallback file extensions to look up files in
`FileService`.

```java
FileService
  .builder(rootDir)
  .fileExtensions("html", "txt")
  ...
```
ikhoon added a commit that referenced this issue Jul 16, 2024
Motivation:

I prefer clean URL patterns without a trailing slash so `/app/projects`
is preferred over `/app/projects/`. If I built `/app/projects` with a
Javascript framework, `/app/projects/index.html` or `/app/projects.html`
may be exported by the framework which is a common feature.

In `FileService`, `/app/projects/index.html` can be served by
`/app/projects/` path, but cannot be found by `/app/projects`. A
trailing slash `/` can be converted into `/index.html` or an auto index
page. As some fallback logics are already implemented, I didn't want to
add a new fallback option for a trailing slash.

Alternatively, I propose an option that appends an extension if there is
no file for the request path. For example, a request sent to
`/app/projects` also finds `/app/projects.[ext]` as a fallback.

Related links:

- #4542
- #1655
- https://ktor.io/docs/server-static-content.html#extensions

Modifications:

- Allow configuring `fallbackFileExtensions()` via `FileServiceBuilder`
- Find a file with fallback extensions if missing.

Result:

- You can now set fallback file extensions to look up files in
`FileService`.

```java
FileService
  .builder(rootDir)
  .fallbackFileExtensions("html", "txt")
  ...
```
- Closes #4542
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants