Mvc: ObjectResult executor should log the type of the value being written out

[kichalla]

We log here https://github.com/aspnet/Mvc/blob/release/src/Microsoft.AspNetCore.Mvc.Core/Internal/ObjectResultExecutor.cs#L139

and it shows up like Executing ObjectResult, writing value Microsoft.AspNetCore.Mvc.ControllerContext. in the log

Related :

• return Json("this is a json string");
  Executing JsonResult, writing value this is a json string.
• return Json(null);
  Executing JsonResult, writing value .
• return Json(new Account() { MyProperty = "pr1", MyProperty2 = "pr2" });
  Executing JsonResult, writing value RoutingMiddleware.Controllers.Account.

[javiercn]

We should never do the first case. We don't have a way of knowing if the information is confidential, so we shouldn't add it to logs by default.

[Eilon]

There isn't a _huge_ security concern here because we log a ton of stuff already anyway, any of which might already have security concerns. E.g. even a URL might have some secret code in it.

The bigger concern is that anything related to live data values is unbounded in terms of length, and thus might bloat log files.

So, logging just the type is probably all we would do.

[rynowak]

We log the Convert.ToString(...) output of the parameters/models. This will generally be the model typ e name, but allows you to customize it if desired.

[rynowak]

We really have no excuse not to fix this for 2.1

[mistakenot]

I've had a stab at fixing this. First attempt at a contribution to this project, so please be forgiving of anything I've missed :) You can see my diff at https://github.com/mistakenot/Mvc/commit/25019a0b2d78813cc2abaf065f26edba6678efce.

Let me know if it is any help.

[Eilon]

@mistakenot thanks for taking a crack at this! If you send a PR it will probably make it easier for us to review. Thanks!

[Eilon]

@rynowak there's a PR for this that I assigned to you to review.

[rynowak]

9db92dc6a7bfc22a62ff12053244201c38390da5 and db2d9ee56f9a7ca0f1a264821e0608b70047337c

[oofpez]

Seems related: I am having an issue where my entire api response body is logged!

Microsoft.AspNetCore.Mvc.Formatters.Json.Internal.JsonResultExecutor: Information: Executing JsonResult, writing value { ... }

{ ... } containing the entire json model.

how can I turn this off? It only seems to happen in the one controller method, where they response is returned like this:
return Json(JsonConvert.DeserializeObject(stringValue));

as opposed to return Json( object) which outputs the type.

[Eilon]

@oofpez hmm that certainly sounds odd. Can you log a new issue with detailed steps to reproduce this?

[kichalla]

I was able to repro this issue in 2.0.5 version of MVC, but it is fixed in latest version (dev) though

[pranavkm]

Doesn't look like this is resolved. Testing with 2.1.0-preview1-28257, here's what I see:

info: Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor[1]
      Executing ObjectResult, writing value of type 'Microsoft.AspNetCore.Mvc.ControllerContext'.

Reopening so we can track this for preview2.

[rynowak]

info: Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor[1]
      Executing ObjectResult, writing value of type 'Microsoft.AspNetCore.Mvc.ControllerContext'.

@pranavkm This logs the TYPE of the value, which is what expect. What am I missing???
https://github.com/aspnet/Mvc/blob/930664de6ef5b6fe1d547ded61745962253774eb/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs#L936

[pranavkm]

The problem's here - https://github.com/aspnet/Mvc/blob/930664de6ef5b6fe1d547ded61745962253774eb/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ObjectResultExecutor.cs#L128

To be clearer, we're passing in the context rather than the value to the log method.