search

dingyuliang / prerender-dotnet

prerender.io Middlewares for ASP.NET MVC, ASP.NET Core, IIS
MIT License
13 stars 9 forks source link

Fix for multiple bugs #1

Closed marc1404 closed 6 years ago

marc1404 commented 7 years ago

Hi @dingyuliang

First of all, thanks for porting the prerender middleware to .NET Core! This is really useful. While using your package I've encountered some bugs which I try to fix in this pull request. I'm actively testing the code changes in a live product of the company I work for.

This pull request addresses the following bugs:

The check for the X-Forwarded-Proto-Header uses the constant HttpsProtocol which is "https://". The comparison cannot be true because the header only contains "https", therefore I've added a new constant value to check against.

The default ignored extensions regex is not surrounded by parentheses, this leads to false positives e.g. if the URL contains js anywhere the prerender service will not be used. A safer approach would be to only check if the string ends with one of the extensions.

I've added Ignore Case options to the crawler user agent pattern check for more robustness (Facebook was not recognized before). Furthermore, the short comparison Configuration.CrawlerUserAgentPattern ?? Constants.CrawlerUserAgentPattern actually results in an empty string because it is a truthy value in C#. Added string.IsNullOrEmpty with a ternary expression to fix that.

Please let me know if I've missed anything and feel free to edit :)

rosrosros commented 6 years ago

Great job @marc1404 - hope this will get pulled in soon