Skip to content

feat: SPA routing fallback support#491

Open
lifeart wants to merge 2 commits intobroccolijs:mainfrom
lifeart:improve-routing
Open

feat: SPA routing fallback support#491
lifeart wants to merge 2 commits intobroccolijs:mainfrom
lifeart:improve-routing

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
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