Please remove content scripts

[evil-shrike]

Hi. (It's slightly related to issue #35 but more general)

Currently the lib has a couple of content script which are processed on install:

• App_Start/EmbeddedResourceVirtualPathProviderStart.cs.pp - creates a AppStart script which register EmbeddedResourceVirtualPathProvider with all assemblies
• web.config.transform - adds StaticFileHandler into web.config

I understand it could be handy for some people to have auto created such files. But in general it a bad idea. It blindly adds what I didn't ask: scan all (!)non system assemblies for embedded resources, add a http handlers. The former is usually unneeded as you'd better specify assemblies to scan explicitly The latter is even more bad thing. I can't understand why a lib should add such settings at all. But particularly this handler overwrites headers for static content specified in web.config. For example I have staticContent/clientCache in my web.cofnig with httpExpires="Fri, 31 Dec 1999 00:00:00 GMT" - it tells browsers do not cache anything. It's very important during development. StaticFileHandler overrides this settings and set current timestamp in Expire header.

Sure I can remove all this stuff manully. But please take into account that the lib can be a transitive dependency. So people even can be not installing particularly this lib but some other depending on it. And they will get these arguable settings/defaults.

[mcintyre321]

The VPP registration is necessary, as otherwise people can't the VPP working and raise github issues, or give up on the project silently.

The staticfilehandler is necessary, as otherwise js/css etc. don't get served and people raise github issues.

Maybe these issues could be solved with improved documentation, but I find that the people who find this a major problem tend to know enough about what they are doing to revert the file changes that the nuget install makes.

[evil-shrike]

The staticfilehandler is necessary, as otherwise js/css etc. don't get served and people raise github issues.

I think it's not necessary. This should be enough:

    <modules runAllManagedModulesForAllRequests="true" />

the people who find this a major problem tend to know enough about what they are doing

In my cases users of my lib started complaining that the browser begun to cache scripts while before it didn't. web.config had and still has httpExpires but the webserver started returning Expire with current timestamp and that allows caching. So they just saw that something was broken with caching. And had no idea that the reason is in a transitive dependency's content scripts...

[mcintyre321]

runAllManagedModulesForAllRequests is a problem for some people, so I wouldn't want to enable it.

As for the cache headers, it's a tricky one - the default StaticFileHandler doesn't set cache headers (which is another issue with RAMMFAR as I recall), which is bad if you've been used to the js/css/etc. being cached automatically by IIS.

The best thing I guess would be for an etag based on the assembly to be returned as the cache control values for embedded assets.

This is the PR which added cache control options - are you sure it's the content scripts that added caching, and not this?

[evil-shrike]

runAllManagedModulesForAllRequests is a problem for some people, so I wouldn't want to enable it.

Maybe I was wrong and enabling runAllManagedModulesForAllRequests isn't enough for serving static files via EmbeddedResourceVirtualPathProvider, I'm not sure. That's because I use EmbeddedResourceVirtualPathProvider only for loading server view templates not serving static file to client. So in such a case not runAllManagedModulesForAllRequests nor StaticFileHandler are needed at all.

As for mentioned blogpost on runAllManagedModulesForAllRequests. it's pretty old stuff. currently we don't have to enable runAllManagedModulesForAllRequests for MVC to work. it was long before. now we have ExtensionlessUrlHandler for this. but it's true that runAllManagedModulesForAllRequests should be disabled if an app don't need to process unmanaged requests. but sometimes it needs. in this case it have to be enabled.

I'd suggest adding handlers in comments.

are you sure it's the content scripts that added caching, and not this?

Yes, I'm sure. I've just removed EmbeddedResourceVirtualPathProvider from my app and added StaticFileHandler handlers:

      <add verb="GET" path="*.js" name="Static for js" type="System.Web.StaticFileHandler" />
      <add verb="GET" path="*.css" name="Static for css" type="System.Web.StaticFileHandler" />
      <add verb="GET" path="*.png" name="Static for png" type="System.Web.StaticFileHandler" />
      <add verb="GET" path="*.jpg" name="Static for jpg" type="System.Web.StaticFileHandler" />

and now for all static js files (not loaded via EmbeddedResourceVirtualPathProvider, just all normal *.js files) the server returns (at 21 Jul 2017 20:48:13 GMT) header Expires with value Sat, 22 Jul 2017 20:48:13 GMT. So it tells browser to cache the file for 1 day. And it's unavoidable! :( Here's the code - http://referencesource.microsoft.com/#System.Web/StaticFileHandler.cs,559:

response.Cache.SetExpires(utcNow.AddDays(1));

[mcintyre321]

I also have projects which use ERVPP but they also serve content, so if I remove the VPP registration and handlers, I'll get even more issues raised on them.

In a perfect world, I would have time to add a StaticFileHandler which returns content-based etags into ERVPP. There's a project, Talifun.Web which contains a StaticFileHandler which might be better than the default StaticFileHandler. I have a feeling that it might not work as I think last time I tried it, it attempted to use the disk to make hashes of files, so it might need to be copied into ERVPP and then modified a little.

In this less than perfect world, maybe the easiest thing would be to publish a second package EmbeddedResourceVirtualPathProvider.Library (or a better name if you can think of one?) which doesn't do reg/handlers?

[mcintyre321]

Maybe something can be done using this approach: https://stackoverflow.com/a/37055207 to override the StaticFileHandlers bad behaviour

[evil-shrike]

Maybe something can be done using this approach: https://stackoverflow.com/a/37055207 to override the StaticFileHandlers bad behaviour

Sorry, I don't get how it could help. We have problems with not files served via ERVPP but files on disk

In this less than perfect world, maybe the easiest thing would be to publish a second package EmbeddedResourceVirtualPathProvider.Library

I think extracting a package with just code will solve all problems. I guess you don't want rename the current package for compat reason so this new package containing only the assembly should be named, right? Then I'd suggest EmbeddedResourceVirtualPathProvider.Core name, or .Provider, but .Library is also looks fine for me. Then existing EmbeddedResourceVirtualPathProvider package would contains dependencies on Core-package, WebActivatorEx and all content scripts, right? Looks good.

[mcintyre321]

Yup. If you can add the nuspec file, I'll update the CI build to push the additional package

[evil-shrike]

I'm afraid it'll require changing build settings as packages should be in separated subfolders now:

nuget/
  EmbeddedResourceVirtualPathProvider/
    content/
    lib/
    EmbeddedResourceVirtualPathProvider.nuspec
  EmbeddedResourceVirtualPathProvider.Core/
    EmbeddedResourceVirtualPathProvider.Core.nuspec

[mcintyre321]

I've updated the build to reflect the above.