http://ayende.comAyende @ RahienCopyright (C) Ayende Rahien 2004 - 2021 (c) 202660Path.Combine("~/Content/css", file) This combines with a backslash delimiter, when the other segments contain forward slashes. Better to use VirtualPathUtility.Combine(): http://msdn.microsoft.com/en-us/library/system.web.virtualpathutility.combine.aspx Also if you put a rooted path into the files parameter, you will get a YSOD eg: http://localhost/css?files=c:\boot.inihttp://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment32http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment32Mon, 04 Jul 2011 09:07:27 GMTCheck out: http://mscd.codeplex.comhttp://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment31http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment31Thu, 09 Jun 2011 22:07:38 GMTMany proxies do not cache URLs with ?querystring in them. It is better to make files a part of the actual URL like proposed above. The only negative is there is a shorter length limitation.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment30http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment30Thu, 09 Jun 2011 14:03:58 GMTI have the same concerns as Alistair - if you're using MVC you could create a route that took something like this: /CSS/{files}.{version}.css Make 'files' an underscore delimited list of the files, which you split and use in your controller code above. 'Version' doesn't even need to be used server side - it could just be a number to do cache busting. However one thing I might like as an addition is a variable that allows you to serve the full (for development) or minified version.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment29http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment29Thu, 09 Jun 2011 06:04:47 GMTNevermind, the 404 will occur for outside of the correct path. So I suppose that could be an issue.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment28http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment28Wed, 08 Jun 2011 18:56:01 GMT@tobi, The code is exiting with an `HttpNotFound` on the first file that doesn't exist on disk. So in your scenario, it would exit with a 404 once it hit the `0.css` file. It won't even attempt to process the other 9,999,999 files. So I don't believe this could overload it. I have to agree that not handling the duplicate file bit could be an issue, though.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment27http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment27Wed, 08 Jun 2011 18:53:27 GMTAnd I forgot that you have to normalize casing and filenames may not contain ".." etc. because that would also allow multiple URLs for the same content. Disallow duplicates.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment26http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment26Wed, 08 Jun 2011 17:42:25 GMT@Ayende: right, that's why I said it was probably a minor issue ;) This being said, unless I'm missing something, what goes into the cache is the combination of files (so yes it would take a lot of these to flood the cache) but my point was that it's keyed by the Files querystring parameter, and it's not handling multiple inclusions of the same file. So if you wanted to mitigate that maybe just taking distinct paths and/or rejecting requests with duplicates would help. If you consider the threat to be serious. Which we would in a framework but you probably would justifiably dismiss in this specific context.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment25http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment25Wed, 08 Jun 2011 17:42:12 GMTOverload it like this: from i in Enumerable.Range(0, 10000000) let files = new [] { "normal.css", i + ".css" } select "/css?files=" + files.Concat(","); And request all of those URLs. Solution: Return 404 if any file does not exist and, if files is not sorted, redirect to the canonical URL.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment24http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment24Wed, 08 Jun 2011 12:52:28 GMTPath validation is hard. The best approach I've seen is CAS (http://msdn.microsoft.com/en-us/library/930b76w0(v=VS.90).aspx): var perm = new FileIOPermission(FileIOPermissionAccess.Read, rootPhysicalPath); perm.PermitOnly(); Lots of the discussions around CAS are for sandboxing plugins, but I've found it very useful in cases like this.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment23http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment23Wed, 08 Jun 2011 11:29:22 GMTTobi, How could you do that?http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment22http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment22Wed, 08 Jun 2011 09:06:57 GMTBertrand, That is just another way to say DDoS, and there are other ways to mitigate against that. You couldn't really flood the cache, since the only things that goes into the cache are the actual files on disk. And even if you flood the cache, nothing bad would happen, things would just not be cached.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment21http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment21Wed, 08 Jun 2011 09:06:23 GMTInterested in why you use == false instead of !. I assume that is to increase readability. Is that something you do throughout your code? http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment20http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment20Tue, 07 Jun 2011 23:48:57 GMTYeah there are some ugly caching implications of this code. Specifically it violates: http://code.google.com/speed/page-speed/docs/caching.html#LeverageProxyCaching So some proxies won't cache this resource as there is a querystring in the url. Additionally, how do you handle refreshing peoples cache? Presumably with something like &version=1?http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment19http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment19Tue, 07 Jun 2011 23:30:04 GMTYou could even overload the server by causing it to process the css over and over.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment18http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment18Tue, 07 Jun 2011 20:34:24 GMTIf I'm not mistaken, there is a minor issue that you may flood the cache by querying many different combinations of existing files (and maybe also requiring the same file more than once). While the file dependency call might mitigate this somehow, adding a crypto validation hash of sorts might help if that becomes a problem (unlikely but eh). Never a fan of crypto stuff on the querystring but I thought I would mention it.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment17http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment17Tue, 07 Jun 2011 19:24:46 GMT@Jesse I don't like the fact that the client can determine which files are going to be merged. It's only a security bug waiting to be exploited. Of course Ayende has checked the path, but another dev could easily remove the check (unintended) and all seems to be ok and working, but then the server is left wide open ... I would not perse (hard)code it into the controller. Via a route or some other mapping (hashing,lookup) would be fine too, but certainly not on the client. // Ryan http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment16http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment16Tue, 07 Jun 2011 17:40:34 GMTI saw the call to MapPath but was unsure about its contract. And that is what bothers me: I do not feel sure about this. PS: I would appreciate if the Name/Email input was remembered by the comment form.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment15http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment15Tue, 07 Jun 2011 15:59:21 GMT@Patrick, I'm not sure why it would be safer. The code disallows anything that is not exactly ~/Content/css. So using incorrect case would simply return 404. It's a pretty restrictive whitelist, which is all that is needed.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment14http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment14Tue, 07 Jun 2011 14:01:56 GMTYou can take this line from inside the loop. var pathAllowed = Server.MapPath(Url.Content("~/Content/css" http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment13http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment13Tue, 07 Jun 2011 14:01:49 GMTIt would look safer to me with... normalizeFile.ToLower().StartsWith(pathAllowed.ToLower()) ... since path are case insensitive. I hate treating path as string! http://codebetter.com/patricksmacchia/2008/12/28/what-is-microsoft-waiting-for-providing-a-descent-path-api/http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment11http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment11Tue, 07 Jun 2011 13:51:37 GMTTobi, Did you see the call to MapPath, as part of its work, it will normalize the path properly.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment10http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment10Tue, 07 Jun 2011 13:32:13 GMTI like this very much, and I would disagree with Ryan about hardcoding your file names in your controller. I saw an interesting alternative from Mads Kristensen from a talk he gave at Mix (http://channel9.msdn.com/events/MIX/MIX11/FRM04). What I liked about his solution was the hashing of the files to create a unique filename. So the view calls a static function that creates the file name, and if the CSS or JS ever changes, the file name reflects that and the new version is sent to the browser as needed.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment9http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment9Tue, 07 Jun 2011 13:00:23 GMTAnother bump for Combres; it does this and a WHOLE lot more.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment8http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment8Tue, 07 Jun 2011 12:25:17 GMTI don't like the files parameter. I would hardcode the css files you need in code. And even make another action method if you need to include different css files. public ActionResult CssOne() { return Merge("base.css", "one.css"); } public ActionResult CssTwo() { return Merge("base.css", "two.css"); } private ActionResult Merge(param string[] files) { // your implementation } http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment7http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment7Tue, 07 Jun 2011 12:08:32 GMT@tobi, I think that validation is pretty much best practice. It doesn't stop you from writing "files=../css/resetcss.css", but it doesn't matter since you will still be in the correct directory. And it can be nice to be able to write "files=layout/foo.css".http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment6http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment6Tue, 07 Jun 2011 11:59:47 GMTYes, this code is nice. Didn't know about the cache/file dependencies. I've always done that coding myself. I've looked at dotless, but thought it lacked a good caching strategy. The only problem I can see with this code is that files that are imported with the @import directive will not be added as dependencies. It would be nice if dotless could include this in the HttpHandler. I prefer using @import instead of putting the filenames in the querystring.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment5http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment5Tue, 07 Jun 2011 11:53:33 GMT"If you aren’t in the CSS directory, you won’t be returned." Are you sure "\..\" is stripped from the path? I don't like that path validation at all. I would restrict the input file names to letters, numbers and "-._". That way you white list and are safe.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment4http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment4Tue, 07 Jun 2011 10:32:18 GMTI'm using very similar code and noticed that browsers occasionally will still re-ask for the files and IIS wouldn't serve them from the cache. So adding checks for If-ModifiedXXX headers will allow you to return 304 instead of sending the entire file stream.http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment3http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment3Tue, 07 Jun 2011 10:22:16 GMTYou really should look into Combres too - it does all this work for you, except that it's completely configurable and functions differently in Debug vs. Release - in Debug mode you get all the separate files loaded separately; in Release mode they get minified, combined, and sent as one. It does JavaScript too, and the minifier etc. is all configurable. http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment2http://ayende.com/13314/elegant-code-raccoon-blog-s-css-controller#comment2Tue, 07 Jun 2011 10:06:06 GMT