uhaciogullari / SimpleMvcSitemap

A minimalist library for creating sitemap files inside ASP.NET MVC and ASP.NET Core MVC applications
MIT License
128 stars 39 forks source link

No option to gzip the result #18

Closed nicholastic closed 7 years ago

nicholastic commented 7 years ago

I tried slapping a Compress attribute on the ActionResult as described here:

https://stackoverflow.com/questions/3802107/how-to-gzip-content-in-asp-net-mvc

But it's not working. I've tried using OnActionExecuting, OnActionExecuted, OnResultExecuting, and OnResultExecuted. OnResultExecuted doesn't work because the headers have already been sent. The other three, chrome gives an ERR_CONTENT_DECODING_FAILED error.

I see that on this page https://github.com/uhaciogullari/SimpleMvcSitemap/blob/40aed73035fde5aad59b3104f4349b62741de8d9/src/SimpleMvcSitemap/XmlResult.cs

you've got an ExecuteResult method which generates the XML. https://stackoverflow.com/questions/37356389/asp-net-mvc-executeresult-vs-actionresult

Anyway, it would be very awesome if you could put in an option to gzip this result.

Thanks Ufuk, your library is really elegant and awesome!

uhaciogullari commented 7 years ago

I guess it's only needed for .NET Core. If you are using the full framework, you are probably using IIS; which does the gzip compression for you.

nicholastic commented 7 years ago

Yes you're right, I'm using Azure for deployment and you can change compression settings in applicationhost.config. I was also a bit confused because Chrome was downloading .gz files instead of displaying them in the browser when I was testing, which turns out to be a known bug. Thanks Ufuk, you're right, it's probably only needed for .NET Core.

uhaciogullari commented 7 years ago

I think we can always leave the compression to the web server. I am sure nginx, haproxy etc. that will be typically used in .NET Core deployments can handle this as well.

If the issue is resolved for you, I'm going to close this one.

nicholastic commented 7 years ago

Probably. Maybe an edge case will pop up, but for now it's not a problem. Thanks again!