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(aio): do not fallback to index.html for file requests #15401

Merged
merged 1 commit into from
Mar 22, 2017

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Mar 22, 2017

Previously, all URLs were rewritten to index.html in order to support
deep-linking. This works when navigating to URLs that correspond to existing
resources. E.g. navigating to /tutorial returns index.html and then the
DocViewer takes over and requests tutorial.json.
Navigating to a non-existent URL (e.g. /foo), will return index.html, which
in turn requests (the non-existent) foo.json and throws an error when trying
to parse the returned index.html as JSON.

This commit fixes it by only rewriting URLs that do not request a file (i.e. do
not include a . in the last path segment).

Fixes #15398

Previously, all URLs were rewritten to `index.html` in order to support
deep-linking. This works when navigating to URLs that correspond to existing
resources. E.g. navigating to `/tutorial` returns `index.html` and then the
`DocViewer` takes over and requests `tutorial.json`.
Navigating to a non-existent URL (e.g. `/foo`), will return `index.html`, which
in turn requests (the non-existent) `foo.json` and throws an error when trying
to parse the returned `index.html` as JSON.

This commit fixes it by only rewriting URLs that do not request a file (i.e. do
not include a `.` in the last path segment).

Fixes angular#15398
@gkalpak gkalpak added this to WIP in docs-infra Mar 22, 2017
@gkalpak gkalpak moved this from WIP to REVIEW in docs-infra Mar 22, 2017
@mary-poppins
Copy link

The angular.io preview for c4a4008 is available here.

@IgorMinar IgorMinar merged commit 8b41422 into angular:master Mar 22, 2017
@gkalpak gkalpak deleted the fix-aio-not-rewrite-files branch March 23, 2017 00:19
@petebacondarwin
Copy link
Member

One thing this doesn't fix is if we tried to navigation to a url that maps to a non-valid JSON file.
There shouldn't be any of those in the system but you never know!
I thought that this was handled client app code, since we have a .catch() RxJS handler in the DocumentService. But it appeared that @IgorMinar was able to make it appear in the staging app.

@petebacondarwin petebacondarwin moved this from REVIEW to MERGE in docs-infra Mar 23, 2017
@petebacondarwin petebacondarwin removed this from MERGE in docs-infra Mar 23, 2017
@gkalpak
Copy link
Member Author

gkalpak commented Mar 23, 2017

@petebacondarwin, can you give an example URL?

@petebacondarwin
Copy link
Member

We don't have any bad ones any more because we blocked the private imports and that VERSION const. So no.

@gkalpak
Copy link
Member Author

gkalpak commented Mar 23, 2017

So, the issue you mentioned above affects anchor elements pointing to invalid URLs?

UPDATE: I tried that and everything seems to work as expcted 😕
Can you describe a scenario that would not behave as expected.

@petebacondarwin
Copy link
Member

I think we do have it working but it was breaking when @IgorMinar was clicking on invalid links in the staging website. But we cannot reproduce that now, since this PR stops the staging site from sending invalid JSON to 404 links.

@petebacondarwin
Copy link
Member

I am happy to move on until someone else complains with a repro.

asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
…15401)

Previously, all URLs were rewritten to `index.html` in order to support
deep-linking. This works when navigating to URLs that correspond to existing
resources. E.g. navigating to `/tutorial` returns `index.html` and then the
`DocViewer` takes over and requests `tutorial.json`.
Navigating to a non-existent URL (e.g. `/foo`), will return `index.html`, which
in turn requests (the non-existent) `foo.json` and throws an error when trying
to parse the returned `index.html` as JSON.

This commit fixes it by only rewriting URLs that do not request a file (i.e. do
not include a `.` in the last path segment).

Fixes angular#15398
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
…15401)

Previously, all URLs were rewritten to `index.html` in order to support
deep-linking. This works when navigating to URLs that correspond to existing
resources. E.g. navigating to `/tutorial` returns `index.html` and then the
`DocViewer` takes over and requests `tutorial.json`.
Navigating to a non-existent URL (e.g. `/foo`), will return `index.html`, which
in turn requests (the non-existent) `foo.json` and throws an error when trying
to parse the returned `index.html` as JSON.

This commit fixes it by only rewriting URLs that do not request a file (i.e. do
not include a `.` in the last path segment).

Fixes angular#15398
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aio staging fails with Parse.json error instead of showing 404 page when we navigate to invalid document URL
5 participants