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

Bypass or deactivate i18n response filter once installed #333

Open
macote opened this issue Feb 2, 2017 · 9 comments
Open

Bypass or deactivate i18n response filter once installed #333

macote opened this issue Feb 2, 2017 · 9 comments

Comments

@macote
Copy link
Contributor

macote commented Feb 2, 2017

I'm using i18n with OWIN middleware and it is configured with i18n.UrlLocalizationScheme set to Void.

I'm having an issue in some cases where specific controller actions are used to download binaries. For those cases, I'm faced with two possibilities:

  • add the controller action path component URL to the UrlsToExcludeFromProcessing regex
  • remove the response filter while handling the request in the controller action

The problem with the UrlsToExcludeFromProcessing solution is that there are multiple controller actions and it is not simple to manage. Also, the need to exclude a specific action path URL could be conditionaly based on some logic determined inside the controller action.

It is quite easy to remove the response filter by setting the Response.Filter to null inside a controller action and this is what I'm doing currently:

    if (this.HttpContext.Response.Filter is i18n.ResponseFilter)
    {
        // Remove i18n filter since a file doesn't need to be translated.
        this.HttpContext.Response.Filter = null;
    }

Unfortunately, the i18n response filter might be wrapped by another filter at that point. In that case, it is not possible to determine if a i18n filter is installed.

Is there something I missed that I could do besides those two possibilities? If not, I propose to add some code to i18n to allow bypassing or deactivating the i18n filter. One way to accomplish this would be to add a 'Bypass' property to the ResponseFilter class and store a reference of the filter somewhere like for example the OWIN context. If 'Bypass' is true, use a logic similar to the one used when a stream is compressed.

@turquoiseowl
Copy link
Owner

Have you looked at i18n.LocalizedApplication.Current.ContentTypesToLocalize setting?

The default setting for this property should effect a bypass of the ResponseFilter if your binary download responses' Content-Type header is set to something like application/octet-stream.

@macote
Copy link
Contributor Author

macote commented Feb 2, 2017

The ContentTypesToLocalize check performed by the OWIN middleware is done before the MVC handler gets executed. At that time, the Response.ContentType is text/html, which I think is the default value.

@turquoiseowl
Copy link
Owner

I see. Are you able to investigate possibility of shunting i18n's hook into the OWIN pipeline until AFTER the MVC handler? That would then accord more with what we're doing with normal ASP.NET.

@macote
Copy link
Contributor Author

macote commented Feb 3, 2017

I did a bit more researching and testing and I concluded that i18n's OWIN implementation cannot replace at 100 % the behavior of the normal ASP.NET implementation since there's no way using Katana's OWIN to execute something after the request handler gets executed.

This means that the EntityLocalizationMiddleware can only do its job partially (URL exclusion), and the only way that I can think of to make it work 100 % is to drop that middleware and call the response filter installer logic like the way the normal ASP.NET implementation does it, but using Global.asax.

Until we have a new ASP.NET Core version of i18n that works 100 % using middleware, I propose some changes to the current OWIN implementation:

  • deprecate EntityLocalizationMiddleware
  • extract the filter response logic code located in LocalizingModule to an HttpContextBase extension method (InstallResponseFilter?)
  • update the OWIN documentation to mention that the InstallResponseFilter method must be called inside the Application_ReleaseRequestState event handler using Global.asax.

I tested these changes and it seems to be working just fine. I can try to prepare a pull request, but I don't have VS2012/2013 and I might encounter some issues because of that during the process.

@macote
Copy link
Contributor Author

macote commented Feb 3, 2017

I created macote/i18n@9d07290aa4.

@turquoiseowl
Copy link
Owner

Looks good. I'm happy to merge that. Can you issue a PR please?

@turquoiseowl
Copy link
Owner

Perhaps the InstallResponseFilter would be better as a static method on LocalizedApplication?

Does it make sense for it to be an extension method off of the context?

@macote
Copy link
Contributor Author

macote commented Feb 3, 2017

I agree, this can be improved. Let me revise the code and I'll issue a PR.

@macote
Copy link
Contributor Author

macote commented Feb 6, 2017

I created #334 . Let me know if you have any other questions or concerns.

Thanks.

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

No branches or pull requests

2 participants