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

feat: SPA routing fallback support #491

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lifeart
Copy link
Contributor

@lifeart lifeart commented Sep 25, 2021

resolves: #425

broccoli server able to detect index.html files on path requests like

/foo -> /foo/index.html and return it.

But, If we use livereload and js-based app navigation, broccoli don't drill down to root index.html for requests like:

/foo/bar

-> current lookup:
foo/bar/index.html, 404

-> expected lookup:
/foo/bar/index.html, /foo/index.html, index.html, 404

@lifeart lifeart changed the title SPA routing support feat: SPA routing fallback support Sep 26, 2021
lib/middleware.ts Outdated Show resolved Hide resolved
lib/middleware.ts Show resolved Hide resolved
@@ -27,6 +27,16 @@ interface MiddlewareOptions {
liveReloadPath?: string;
}

function findClosestIndexFileForPath(outputPath: string, prefix: string): string | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

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

Path manipulation from inbound requests always gives me the heebeegeebeez do to the potential of path traversal security issues. I would prefer to avoid as much liability here as possible. Do we think the fallback index.html lookup is something sufficiently commonly used to justify?

Or would a fixed index.html such as https://github.com/ember-cli/ember-cli/blob/2d77f099c19f2b54328e7e961e0b23a31a638661/lib/tasks/server/middleware/history-support/index.js#L63 be sufficient.

I can be convinced by either approach, the former will just require substantially more testing and care.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stefanpenner we could skip index search if path contain . to prevent traversal security issues.

I seen cases where multiple static apps composed into one using nesting (and have to deal with it):

root_app
   /child-app
   /some-side-app
   /help-app

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya it’s not a bad feature at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Broccoli server [file serving] web navigation API support
2 participants