Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

ErrorController: Bypass for static files #101

Closed
JeremyCaney opened this issue Dec 26, 2021 · 3 comments
Closed

ErrorController: Bypass for static files #101

JeremyCaney opened this issue Dec 26, 2021 · 3 comments
Assignees
@JeremyCaney
Labels
Area: Web Relates to the `AspNetCore` or other web-related functionality. Priority: 1 Severity 1: Minor Status 5: Complete Task is considered complete, and ready for deployment. Type: Improvement Improves the functionality or interface of an existing feature.
Milestone
OnTopic 5.2.0

Comments

@JeremyCaney
Copy link
Member

Typically, static files are referenced as HTML resources, which are only displayed to end-users if they result in an HTTP 200. As a result, it is unnecessary and undesirable for them to return a custom 404 page if they are missing or encounter another error. Instead, it is preferable that they the original status code and a very brief message. This ensures for fast processing of these errors and reduce the amount of bandwidth taken up by them.
@JeremyCaney JeremyCaney self-assigned this Dec 26, 2021
@JeremyCaney
Copy link
Member Author

The initial draft of this always returns a 404. That needs to be resolved to reference the original status code.

@JeremyCaney JeremyCaney added this to the OnTopic 5.2.0 milestone Dec 26, 2021
@JeremyCaney JeremyCaney added Area: Web Relates to the `AspNetCore` or other web-related functionality. Priority: 1 Severity 1: Minor Status 3: In Progress Currently being worked on. Type: Improvement Improves the functionality or interface of an existing feature. labels Dec 26, 2021
@JeremyCaney
Copy link
Member Author

Ideally, this will be configurable so it can be opted into (or, perhaps, out of). This might be limited to e.g., people using a particular route, or even a query string via UseStatusCodePagesWithReExecute().

JeremyCaney added a commit that referenced this issue Dec 26, 2021
@JeremyCaney
Conditionally restrict static files from ErrorController
83f4691 
Custom errors provide human audiences with friendly error messages. Most static files—at least on an OnTopic website—will not be presented to human audiences, however, as they will instead be loaded as resources via e.g. `<script />`, `<style />`, or `<image />` tags (among others). As such, processing the entire `ErrorController` logic and returning a bunch of markup isn't necessary, and is even wasteful in terms of CPU load and bandwidth. That's especially true of sites subjected to a lot of robot activity for identifying potential exploits (which is the majority of public facing sites).

To better control this, the `HttpAsync` action of the `ErrorController` now accepts an optional `includeStaticFiles` parameter, which defaults to true. If this is set in the querystring or a route variable, it will exclude static files from subsequent processing by the `ErrorController`, and instead return a canned string.

This addresses the core requirements of Feature #101.
JeremyCaney added a commit that referenced this issue Dec 26, 2021
@JeremyCaney
Expose includeStaticFiles configuration option to MapTopicErrors()
b591bd7 
The `MapTopicErrors()` extension method is one way to configure routing to the `ErrorController`. It isn't the _only_ approach, but it is the one that provides easiest access to the main variables that implementers are likely to want to change, such as the base URL. This is now extended to optionally accept an `includeStaticFiles` parameter, which maps to the parameter of a corresponding name on the `Http` action itself (83f4691). If set to `false`, static files won't return a custom error page—or, rather, will get one, but it will only contain a very lightweight, canned message. This satisfies the configuration requirements of Feature #101.
JeremyCaney added a commit that referenced this issue Dec 26, 2021
@JeremyCaney
Updated Host sites to disable includeStaticFiles
74b7cc8 
By passing the `includeStaticFiles` argument in `MapTopicErrors()` (b591bd7), we are effectively disabling `includeStaticFiles` in the corresponding `ErrorController.Http()` action (83f4691). While `includeStaticFiles` defaults to true, since it's the intuitive baseline, we expect most implementers will want to disable it, and thus have made it the standard in the boilerplate `Host` file. Adding it to the integration tests' `Host` will allow us to follow-up with an integration test of the Feature #101 functionality; see next commit.
JeremyCaney added a commit that referenced this issue Dec 26, 2021
@JeremyCaney
Introduced integration test for new includeStaticFiles parameter
6fbe387 
Confirm that by setting the `includeStaticFiles` argument in `MapTopicErrors()` (b591bd7) for the integration tests host (74b7cc8), we are effectively disabling `includeStaticFiles` in the corresponding `ErrorController.Http()` action (83f4691).

As part of this, I needed to remove the (otherwise extraneous) check for the correct content type, since the custom error for static files will be of type `text/plain` instead of `text/html`.

This validates the functionality called for in Feature #101.
JeremyCaney added a commit that referenced this issue Dec 26, 2021
@JeremyCaney
Merge branch 'improvement/ErrorController-static-files' into develop
65f6414 
Customer error pages for HTTP status codes are intended to provide a friendly experience for human audiences. This requires additional processing and bandwidth, however, as the `ErrorController` must find the appropriate `Topic`, generate the corresponding view model, execute the view, and return the markup. That's a lot of overhead for content that _doesn't_ have a human audience. Most notably, web resources—such as scripts, style sheets, images, fonts, &c.—that return a 404 error won't typically be seen by human audiences, outside of developers, since the errors are swallowed by the browser, or simply result in loss of functionality (such as a broken image). In these cases, it doesn't make sense to process and deliver a custom HTTP error.

To facilitate this, this update introduces a new `includeStaticFiles` parameter to both the `UseTopicErrors()` extension method as well as the `ErrorController`'s `HttpAsync()` action. This isn't the only way to configure a route to the error controller, but it's the one that exposes custom control over how it behaves, and will be the preferred option for customers requiring that level of flexibility. This is an optional parameter, and defaults to `true`—meaning static files will return the normal error page experience. While that's an intuitive default, however, we expect most implementers will want to disable this default, and thus the `Host` boilerplate code has been updated to include this configuration option.

This satisfies Feature #101.
@JeremyCaney
Copy link
Member Author

Implemented this by adding a new includeStaticFiles parameter onto the ErrorController's HttpAsync() action (83f4691) and, for convenience, the MapTopicErrors() extension method (b591bd7). This defaults to true, which is the intuitive default, suggesting that all static files will be included in the custom error pages. This can easily be disabled by disabling the includeStaticFiles, primarily through the MapTopicErrors() extension (though it can also be done via the query string or a custom route parameter, if needed). As we expect most implementers to want to take advantage of this functionality, the parameter is explicitly disabled in the Hosts boilerplate code (74b7cc8) as well as the documentation (240d47f). Finally, an integration test was extended to account for this behavior (6fbe387).

@JeremyCaney JeremyCaney added Status 5: Complete Task is considered complete, and ready for deployment. and removed Status 3: In Progress Currently being worked on. labels Dec 27, 2021
@JeremyCaney JeremyCaney mentioned this issue Jun 26, 2022
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@JeremyCaney JeremyCaney

Labels
Area: Web Relates to the `AspNetCore` or other web-related functionality. Priority: 1 Severity 1: Minor Status 5: Complete Task is considered complete, and ready for deployment. Type: Improvement Improves the functionality or interface of an existing feature.
Projects
None yet
Milestone
OnTopic 5.2.0
Development

No branches or pull requests
1 participant
@JeremyCaney