Shazwazza / ClientDependency

DEPRECATED. A library for managing CSS & JavaScript dependencies and optimization in ASP.Net
139 stars 65 forks source link

This operation is not supported for a relative URI. at System.Uri.get_PathAndQuery() #147

Closed lesley-w closed 6 years ago

lesley-w commented 6 years ago

Umbraco 7

We have recently upgraded a website from v6 to v7.10 and are now seeing regular errors in the error logs:

2018-08-08 17:41:57,240 [25] ERROR Umbraco.Core.UmbracoApplicationBase [(null)] - An unhandled exception occurred

System.InvalidOperationException: This operation is not supported for a relative URI.
   at System.Uri.get_PathAndQuery()
   at ClientDependency.Core.CompositeFiles.Providers.BaseCompositeFileProcessingProvider.WritePathToStream(ClientDependencyType type, String path, HttpContextBase context, StreamWriter sw) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\Providers\BaseCompositeFileProcessingProvider.cs:line 209
   at ClientDependency.Core.CompositeFiles.Providers.CompositeFileProcessingProvider.<>c__DisplayClass2_0.<CombineFiles>b__0(String s) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\Providers\CompositeFileProcessingProvider.cs:line 74
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at ClientDependency.Core.CompositeFiles.Providers.CompositeFileProcessingProvider.CombineFiles(String[] filePaths, HttpContextBase context, ClientDependencyType type, List`1& fileDefs) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\Providers\CompositeFileProcessingProvider.cs:line 74
   at ClientDependency.Core.CompositeFiles.CompositeDependencyHandler.GetCombinedFiles(HttpContextBase context, String fileset, ClientDependencyType type, List`1& fDefs) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\CompositeDependencyHandler.cs:line 254
   at ClientDependency.Core.CompositeFiles.CompositeDependencyHandler.ProcessRequestInternal(HttpContextBase context, String fileset, ClientDependencyType type, Int32 version, Byte[] outputBytes, OutputCachedPage page) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\CompositeDependencyHandler.cs:line 185
   at ClientDependency.Core.CompositeFiles.CompositeDependencyHandler.System.Web.IHttpHandler.ProcessRequest(HttpContext context) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\CompositeDependencyHandler.cs:line 110
   at System.Web.HttpApplication.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute()
   at System.Web.HttpApplication.ExecuteStepImpl(IExecutionStep step)
   at System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously)

We have no idea what is causing the errors or how to reproduce it - which makes fixing it a problem.

lesley-w commented 6 years ago

We are also regularly seeing

2018-08-18 18:20:03,347 [22] ERROR Umbraco.Core.UmbracoApplicationBase [(null)] - An unhandled exception occurred

System.ArgumentException: No query string found in request
   at ClientDependency.Core.CompositeFiles.CompositeDependencyHandler.System.Web.IHttpHandler.ProcessRequest(HttpContext context) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\CompositeDependencyHandler.cs:line 49
   at System.Web.HttpApplication.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute()
   at System.Web.HttpApplication.ExecuteStepImpl(IExecutionStep step)
   at System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously)
lesley-w commented 6 years ago

Should add that as part of the upgrade we also moved the website into Azure

jespardk commented 6 years ago

We are getting the same. Clientdependency fails for all our custom libraries. Example:

2018-09-20 11:11:34,998 [P11300/D16/T89] ERROR Umbraco.Web.UI.CdfLogger - Could not load file contents from /App_Plugins/OpenPublic/Shared/PermissionsEditor/userGroupPermissions.interceptor.js.opw. EXCEPTION: This operation is not supported for a relative URI.
System.InvalidOperationException: This operation is not supported for a relative URI.
   at System.Uri.get_PathAndQuery()
   at ClientDependency.Core.CompositeFiles.Providers.BaseCompositeFileProcessingProvider.WritePathToStream(ClientDependencyType type, String path, HttpContextBase context, StreamWriter sw) in C:\Users\Shannon\Documents\_Projects\ClientDependency\ClientDependency\ClientDependency.Core\CompositeFiles\Providers\BaseCompositeFileProcessingProvider.cs:line 213

See failure here: https://github.com/Shazwazza/ClientDependency/blob/master/ClientDependency.Core/CompositeFiles/Providers/BaseCompositeFileProcessingProvider.cs#L217

uri.PathAndQuery.EnsureStartsWith("/").EnsureStartsWith("~") will never work with a relative url such as: "/App_Plugins/OpenPublic/Shared/PermissionsEditor/userGroupPermissions.interceptor.js.opw"

It was broken in this commit: https://github.com/Shazwazza/ClientDependency/commit/378e79381225b3394dd9c1585b8578c930b9deb2

This bug means we can not update clientdependency to resolve the security bug

Shazwazza commented 6 years ago

@jespardk please provide steps on how you to replicate the problem?

Shazwazza commented 6 years ago

@jespardk it worked before by 'fluke' which was part of the security problem, you are trying to load a opw file which is not a registered client file like css/js, if you add opw to that list in config it will work, or change your lib to be a .js file

jespardk commented 6 years ago

@Shazwazza We add the extensions through code. FileWriters.AddWriterForExtension(".opw", new EmbeddedResourceWriter()); The custom libs/files worked for ClientDependency 1.9.2 and we've done nothing in the project besides update to 1.9.7.

Regardless of the extensions, I can not see how "relativeuri".PathAndQuery would ever work? It is not supported by Uri

Demo:

var path = "/App_Plugins/OpenPublic/Shared/PermissionsEditor/userGroupPermissions.interceptor.js.opw";

          if (Uri.TryCreate(path, UriKind.RelativeOrAbsolute, out Uri uri))
          {
            uri.PathAndQuery.Dump();
          }

image

CasperTDK commented 6 years ago

Update: we've also tried adding the extension to the config file - it did not affect the PathAndQuery error Just to be explicit in what we've tried today: fileDependencyExtensions=".js,.css,.opw,.opwd"

Shazwazza commented 6 years ago

If it's a local file it should pass this boolean test: https://github.com/Shazwazza/ClientDependency/blob/master/ClientDependency.Core/CompositeFiles/Providers/BaseCompositeFileProcessingProvider.cs#L192

If it ends up here: https://github.com/Shazwazza/ClientDependency/blob/master/ClientDependency.Core/CompositeFiles/Providers/BaseCompositeFileProcessingProvider.cs#L213 then it's being detected as a remote file (i.e. with a full path like http://mysite/App_Plugins/OpenPublic/Shared/PermissionsEditor/userGroupPermissions.interceptor.js.opw ) which shouldn't be the case for a file like /App_Plugins/OpenPublic/Shared/PermissionsEditor/userGroupPermissions.interceptor.js.opw if that exists locally and the dependency is declared as such

CasperTDK commented 6 years ago

It is an embedded resources. So not a remote url nor a local file.

But if I change this && PathHelper.TryGetFileInfo(uri.PathAndQuery.EnsureStartsWith("/").EnsureStartsWith("~"), context, out var fi)) to this && PathHelper.TryGetFileInfo((uri.IsAbsoluteUri ? uri.PathAndQuery.EnsureStartsWith("/").EnsureStartsWith("~") : uri.OriginalString), context, out var fi))

Everything works!

Reason def = WriteFileToStream(sw, fi, type, path, context); uses our custom embedded resource writer - so everything is working as intended.

We just need to stop using Uri.QueryAndPath for relative urls (to avoid InvalidOperationException).

Shazwazza commented 6 years ago

Right ... would be nice to report that in the beginning so I can understand your scenario ;)

Point is, it shouldn't end up there. If it can be processed locally it needs to pass the CanProcessLocally test.

Shouldn't your virtual writer return true here ? https://github.com/Shazwazza/ClientDependency/blob/master/ClientDependency.Core/CompositeFiles/Providers/BaseCompositeFileProcessingProvider.cs#L141

CasperTDK commented 6 years ago

Sorry about that. I was being very focused on the InvalidOperation in the relativeUrl condition. I still think the pull request is valid for consideration :-)

I will look into CanProcessLocally right away - I see your point

CasperTDK commented 6 years ago

Well, this is complicated. There was a bug in CanProcessLocally -> FileExists on our end. Since 2015. Because the way ClientDependency has worked, the bug was never discovered and there have been no user-issues.

Pending recent changes, the FileExists bug now causes problems and hits the Uri.PathAndQuery issue on relative Uris

Shazwazza commented 6 years ago

Exactly and this is partly how the security problem came to be and another reason why we cannot change the Uri stuff since that is strictly used for absolute URLs.

CasperTDK commented 6 years ago

Aaah. Now it also makes sense for us. Thanks! 🥇

Shazwazza commented 6 years ago

Glad you got that sorted 🎉 :)

Shazwazza commented 6 years ago

I'm closing this issue since I believe this issue all along has been part of the problem resolved in the latest version.