Umbraco-cms: GetApplicationTrees throws exception after updating to MVC/Web API 5.2.7

[ronaldbarendse]

Microsoft just released a minor update for MVC 5 and Web API, but after updating from (NuGet version) 5.2.6 to 5.2.7, the following exception occurs when requesting GET /umbraco/backoffice/UmbracoTrees/ApplicationTree/GetApplicationTrees?application=content&tree=&isDialog=false, making the back-office unusable:

Unhandled controller exception occurred System.Web.Http.HttpResponseException: Processing of the HTTP request resulted in an exception. Please see the HTTP response returned by the 'Response' property of this exception for details.
 at System.Web.Http.ModelBinding.FormatterParameterBinding.ReadContentAsync(HttpRequestMessage request, Type type, IEnumerable`1 formatters, IFormatterLogger formatterLogger, CancellationToken cancellationToken)
 at System.Web.Http.ModelBinding.FormatterParameterBinding.ReadContentAsync(HttpRequestMessage request, Type type, IEnumerable`1 formatters, IFormatterLogger formatterLogger)
...

The request itself returns a 415 Unsupported Media Type with the following payload:

)]}',
{"Message":"The request contains an entity body but no Content-Type header. The inferred media type 'application/octet-stream' is not supported for this resource.","ExceptionMessage":"No MediaTypeFormatter is available to read an object of type 'FormDataCollection' from content with media type 'application/octet-stream'.","ExceptionType":"System.Net.Http.UnsupportedMediaTypeException","StackTrace":"   at System.Net.Http.HttpContentExtensions.ReadAsAsync[T](HttpContent content, Type type, IEnumerable`1 formatters, IFormatterLogger formatterLogger, CancellationToken cancellationToken)\r\n   at System.Web.Http.ModelBinding.FormatterParameterBinding.ReadContentAsync(HttpRequestMessage request, Type type, IEnumerable`1 formatters, IFormatterLogger formatterLogger, CancellationToken cancellationToken)"}

It's probably also worth noting the Microsoft.AspNet.Cors package is also installed and it only occurs only when the Microsoft.AspNet.WebApi.WebHost package (including dependent Microsoft.AspNet.WebApi) is updated to 5.2.7. Updating all other Microsoft.AspNet.WebApi.* packages to 5.2.7 does not introduce this exception. So there's probably some change in these 2 packages that causes this exception...

Reproduction

Specifics

• Umbraco 7.12.4
• GET /umbraco/backoffice/UmbracoTrees/ApplicationTree/GetApplicationTrees

Steps to reproduce

Update the Microsoft.AspNet.WebApi.WebHost package to 5.2.7.

[chrno1209]

I am having the same issue, had to rollback web-api to v5.2.6

[Shazwazza]

Thanks, we'll need to investigate. We have had to make fixes almost every time they release a new version due to some of the proxying we do between controllers.

[nul800sebastiaan]

Sounds annoying!

For now, clearly, you won't be able to upgrade this dependency.

I've marked this as "Up for grabs" so that you or someone else coming along could create a pull request for it.

[ronaldbarendse]

@nul800sebastiaan Maybe the next UmbracoCms.Core NuGet package/version can restrict the versions to 5.2.6, so they can't be updated untill the issue is fixed in a newer version (that removes the restriction again)?

As a workaround, the packages can be restricted by setting allowedVersions in the project packages.config, e.g.:

<package id="Microsoft.AspNet.Cors" version="5.2.6" targetFramework="net461" allowedVersions="[5.2.6]" />
<package id="Microsoft.AspNet.Mvc" version="5.2.6" targetFramework="net461" allowedVersions="[5.2.6]" />
<package id="Microsoft.AspNet.Razor" version="3.2.6" targetFramework="net461" allowedVersions="[3.2.6]" />
<package id="Microsoft.AspNet.WebApi" version="5.2.6" targetFramework="net461" allowedVersions="[5.2.6]" />
<package id="Microsoft.AspNet.WebApi.Client" version="5.2.6" targetFramework="net461" allowedVersions="[5.2.6]" />
<package id="Microsoft.AspNet.WebApi.Core" version="5.2.6" targetFramework="net461" allowedVersions="[5.2.6]" />
<package id="Microsoft.AspNet.WebApi.WebHost" version="5.2.6" targetFramework="net461" allowedVersions="[5.2.6]" />
<package id="Microsoft.AspNet.WebPages" version="3.2.6" targetFramework="net461" allowedVersions="[3.2.6]" />

[nul800sebastiaan]

@ronaldbarendse That's not an option, then we'd be limiting people to an exact version forever (or at least for the lifetime of this install). That's a workaround, a true fix is needed for this at some point.

[ronaldbarendse]

@nul800sebastiaan Umbraco is already limiting the versions to be below 6.0.0:

https://github.com/umbraco/Umbraco-CMS/blob/9ad69c11cbc46b6557a6c9b5a63603cc3dcfda3b/build/NuSpecs/UmbracoCms.Core.nuspec#L20-L21

This is probably because a new major version (in all likeliness) won't work with that version of Umbraco...

Because Microsoft.AspNet.WebApi.WebHost 5.2.7 (a transitive dependency of Microsoft.AspNet.WebApi) doesn't work with the current version of Umbraco, why not limit this in the NuGet package?

<dependency id="Microsoft.AspNet.WebApi.WebHost" version="[5.2.3,5.2.7)" />

If Umbraco is updated to also work with version 5.2.7 of this package, this can be removed again... In the mean time, nobody can (accidentally) break their Umbraco back-office in the next versions with a single Update-Package 👍

[ronaldbarendse]

For those that want to limit the Microsoft.AspNet.WebApi.WebHost version to be kept below 5.2.7, adding allowedVersions="[5.2.3,5.2.7)" to the package reference in packages.config should suffice (the other packages can be updated without errors, as far as I can tell). This should result in something like:

<package id="Microsoft.AspNet.WebApi.WebHost" version="5.2.6" targetFramework="net461" allowedVersions="[5.2.3,5.2.7)" />

If you already updated to 5.2.7, make sure you downgrade first using Update-Package Microsoft.AspNet.WebApi.WebHost -Version 5.2.6.

[nul800sebastiaan]

We ran some tests and this is not really a problem unless you manually update MVC, so few people will run into this. Let's fix the actual issue instead of this.

[nul800sebastiaan]

The real error behind this request is:

The request contains an entity body but no Content-Type header. The inferred media type 'application/octet-stream' is not supported for this resource.

I copied the curl output for this request from Chrome and replayed the request in Postman and indeed, we don't send up the header Content-Type: application/json; charset=utf-8. Once I add that header, the request returns the appropriate data.

Any thoughts about this @Shazwazza ?

[dawoe]

Is it only this request ? Or others as well...strange thing is that request doesn't contain a body as far as I can see

[nul800sebastiaan]

As far as I can tell it's only this request. The body is empty indeed, maybe that's what's tripping it up somehow.

[dawoe]

Really strange...because it's normal $http.get request from angular. Which is used in may places in the backend.

And as far as I can see this API method is only called in one place from the angular part https://github.com/umbraco/Umbraco-CMS/blob/dd6e764588d22ef2b7bce01fd504ece89834f181/src/Umbraco.Web.UI.Client/src/common/resources/tree.resource.js#L72

[Shazwazza]

@nul800sebastiaan w00t! Good work on checking the problem and I'm glad my assumption was wrong about a problem with the proxy, this means the issue is much cleaner/easier to fix.

I don't have time to jump into the code right now but will star this and try to have a look by tomorrow

[nul800sebastiaan]

Temporary fix: in Umbraco\Js\umbraco.security.js you can add a default header in the .config:"

$httpProvider.defaults.headers.get = { "Content-Type": "application/json; charset=utf-8" };

This seems to work and the backoffice seems fine for now. Not sure if this is the best idea though.

Make sure to bump the version number in ClientDependency.config when you make this change.

[ronaldbarendse]

As the exception is thrown while binding the model and the error mentioning FormDataCollection, it probably impacts all controllers using this model... Looking through the code, this is used (almost exclusively) in all the tree controllers, but the ApplicationTreeController is the first one to be called from Angular:

https://github.com/umbraco/Umbraco-CMS/blob/dd6e764588d22ef2b7bce01fd504ece89834f181/src/Umbraco.Web/Trees/ApplicationTreeController.cs#L34-L35

The strange thing is it's looking at the entity body for a GET request, so maybe adding a [FromUri] attribute to the parameter would remove this behaviour?

c# [HttpQueryStringFilter("queryStrings")] public async Task<SectionRootNode> GetApplicationTrees(string application, string tree, [FromUri]FormDataCollection queryStrings, bool onlyInitialized = true)

[nul800sebastiaan]

Unfortunately no, that doesn't fix it, it results in a No parameterless constructor defined for this object error.

[Shazwazza]

So the error is thrown here https://github.com/aspnet/AspNetWebStack/blob/master/src/System.Net.Http.Formatting/HttpContentExtensions.cs#L261

What is odd is that the history/blame of this file doesn't show a change for a very long time (like 6 years) but I think it turns out that it took that long to get this change out. This is the commit that changes the behaviour: https://github.com/aspnet/AspNetWebStack/commit/7bad1c5e3ef6f619c1ad39bfa18c0d8343a8e422

which shows it is targeted at the 3.2.7 tag (which is the 5.2.7 webapi release)

By default, FormDataCollection isn't meant to be used with GET which is why we have this https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/WebApi/Filters/HttpQueryStringFilterAttribute.cs#L13 ... but that only formats a value after it has attempted to be bound. This probably did work fine in older versions of WebApi but not so much now. The underlying reason this error is thrown is because no MediaTypeFormatter found for the FormDataCollection in this request type and this filter is more of a hack. What needs to happen is either a custom (readonly) media type formatter is created and registered for this type for where it is used or a custom model binder is used. This is how it should have been done before but i guess we learn new things over the past many years :)

I don't think we should be appending the Content-Type header to every request since I think that may cause more problems especially if other package devs are relying on the default behavior of angular. I've also tried adding the defaults in our interceptor like $httpProvider.defaults.headers.get = { "Content-Type": "application/json; charset=utf-8" }; but it doesn't work on my machine since it seems that this version of angular still strips off the content type if the data is empty, see https://stackoverflow.com/questions/16194442/angular-content-type-is-not-being-sent-with-http so if we went down that route we also have to add an empty string to the body which isn't pretty. Else we can update all calls to the tree to use this syntax, which does work but it's still forcing a hack to work around the default WebApi behaviour on the server.

return umbRequestHelper.resourcePromise($http({
    method: 'GET',
    url: umbRequestHelper.getApiUrl("treeApplicationApiBaseUrl", "GetApplicationTrees", query),
    data: '',
    headers: {
        'Content-Type': 'application/json'
    }
}), 'Failed to retrieve data for application tree ' + options.section);

So here's a PR that fixes how we deal with FormDataCollection on the server side for GET requests, I've tested with the WebApi version we ship with along with 5.2.7 and everything is good. I've checked all references of FormDataCollection where it needs to be model bound and ensured the model binder is applied there.

https://github.com/umbraco/Umbraco-CMS/pull/3851

[ronaldbarendse]

@Shazwazza Nice to see these bugs eventually lead to more cleaner code 😄 I've added some comments to the PR, have a look!

[shearer3000]

hi - will this PR make 7.13 or 7.13.1?
thanks

[alamandra007]

I was facing the same issue and applied suggested fix to umbraco.resources.js file, what brought back to live all the top level nodes of my back-end trees.

return umbRequestHelper.resourcePromise($http({
    method: 'GET',
    url: umbRequestHelper.getApiUrl("treeApplicationApiBaseUrl", "GetApplicationTrees", query),
    data: '',
    headers: {
        'Content-Type': 'application/json'
    }
}), 'Failed to retrieve data for application tree ' + options.section);

However to resurrect all upper levels nodes of my back-end trees I had to perform additional sibling similar tweak.

return umbRequestHelper.resourcePromise($http({
    method: 'GET',
    url: getTreeNodesUrl(options.node),
    data: '',
    headers: {
        'Content-Type': 'application/json'
    }
}), 'Failed to retrieve data for child nodes ' + options.node.nodeId);

To have context menus working on all back-end tree nodes I had to make one tweak as well.

return umbRequestHelper.resourcePromise($http({
    method: 'GET',
    url: getTreeMenuUrl(node),
    data: '',
    headers: {
        'Content-Type': 'application/json'
    }
}), 'Failed to retrieve data for a node\'s menu ' + node.id);

Cheers!

[ronaldbarendse]

Too bad both the 'work-around' (limiting the dependency upper-version) or @Shazwazza's fix didn't make Umbraco 7.13 (and 7.13.1) release...

Both have a PR and limiting the upper-version of Microsoft.AspNet.WebApi.WebHost is a quick-fix that doesn't require further code changes or extensive testing: so why is this not even applied as temporary solution? 😕

[Shazwazza]

I think it was not merged because we weren't sure if we were waiting on a reply https://github.com/umbraco/Umbraco-CMS/pull/3851#issuecomment-447720821

If you are happy with that code then i think it's fine to merge if @nul800sebastiaan is happy with it and it's tested.

[DanDiplo]

Guess I'm one of the "few people" to run in to this - lost an hour of my life to wondering what was going on 😄 I know it's difficult keeping track, but I think it's worth considering testing Umbraco with updates of core MVC/Web API components as a) there is a reason Microsoft update them and b) this reason can sometimes be security related.

[ghost]

I can see this missed the 7.13.2 release...

When is it due to be released?

Thanks!

[CasperTDK]

Any news? 👍

[Shazwazza]

Have merged, so should be in 7.14