Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PortablePdbReader/ExceptionDetailsProvider may throw an exception which causes problems in callers like DeveloperExceptionPageMiddleware #14609

Closed
cremor opened this issue Oct 1, 2019 · 1 comment · Fixed by #14612
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware

Comments

@cremor
Copy link
Contributor

cremor commented Oct 1, 2019

Describe the bug

PortablePdbReader.GetPdbPath() calls PEReader.ReadDebugDirectory() but doesn't handle exceptions that this call might throw. This in turn will then cause problems for callers higher up in the call stack, like DeveloperExceptionPageMiddleware or Hellang.Middleware.ProblemDetails which call it via ExceptionDetailsProvider.GetDetails.

To Reproduce

Steps to reproduce the behavior:

  1. Using version 2.2 or 3.0 of ASP.NET Core
  2. Add a reference to Oracle.ManagedDataAccess to the project (the current version is 19.3.1, future versions might not be affected)
  3. Add the packages Microsoft.Extensions.StackTrace.Sources and Microsoft.Extensions.TypeNameHelper.Sources (via https://dotnet.myget.org/F/aspnetcore-dev/api/v3/index.json)
  4. Run this code:
try
{
    // Trigger an exception inside Oracle.ManagedDataAccess.dll
    using (var conn = new OracleConnection())
    {
        conn.Open();
    }
}
catch (Exception ex)
{
    var provider = new ExceptionDetailsProvider(env.ContentRootFileProvider, 10);
    foreach (var detail in provider.GetDetails(ex))
    {
        // Would do something with 'detail' here.
    }
}
  1. See this exception in the DeveloperExceptionPage:
System.BadImageFormatException: Invalid directory size.
   at System.Reflection.PortableExecutable.PEReader.ReadDebugDirectory()
   at Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetPdbPath(String assemblyPath) in C:\Users\Username\.nuget\packages\microsoft.extensions.stacktrace.sources\3.0.0-preview-181114-06\contentFiles\cs\netstandard1.0\Microsoft.Extensions.StackTrace.Sources\StackFrame\StackFrame\PortablePdbReader.cs:line 99
   at Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetMetadataReader(String assemblyPath) in C:\Users\Username\.nuget\packages\microsoft.extensions.stacktrace.sources\3.0.0-preview-181114-06\contentFiles\cs\netstandard1.0\Microsoft.Extensions.StackTrace.Sources\StackFrame\StackFrame\PortablePdbReader.cs:line 72
   at Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.PopulateStackFrame(StackFrameInfo frameInfo, MethodBase method, Int32 IlOffset) in C:\Users\Username\.nuget\packages\microsoft.extensions.stacktrace.sources\3.0.0-preview-181114-06\contentFiles\cs\netstandard1.0\Microsoft.Extensions.StackTrace.Sources\StackFrame\StackFrame\PortablePdbReader.cs:line 27
   at Microsoft.Extensions.StackTrace.Sources.StackTraceHelper.GetFrames(Exception exception) in C:\Users\Username\.nuget\packages\microsoft.extensions.stacktrace.sources\3.0.0-preview-181114-06\contentFiles\cs\netstandard1.0\Microsoft.Extensions.StackTrace.Sources\StackFrame\StackFrame\StackTraceHelper.cs:line 61
   at Microsoft.Extensions.StackTrace.Sources.ExceptionDetailsProvider.GetDetails(Exception exception)+MoveNext() in C:\Users\Username\.nuget\packages\microsoft.extensions.stacktrace.sources\3.0.0-preview-181114-06\contentFiles\cs\netstandard1.0\Microsoft.Extensions.StackTrace.Sources\ExceptionDetails\ExceptionDetails\ExceptionDetailsProvider.cs:line 30
  1. Remove the try-catch block around the code that triggers the exception.
  2. See that the DeveloperExceptionPage is empty. Instead the following exception is written to the output window:
Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware: Error: An exception was thrown attempting to display the error page.

System.BadImageFormatException: Invalid directory size.
   at System.Reflection.PortableExecutable.PEReader.ReadDebugDirectory()
   at Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetPdbPath(String assemblyPath)
   at Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetMetadataReader(String assemblyPath)
   at Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.PopulateStackFrame(StackFrameInfo frameInfo, MethodBase method, Int32 IlOffset)
   at Microsoft.Extensions.StackTrace.Sources.StackTraceHelper.GetFrames(Exception exception)
   at Microsoft.Extensions.StackTrace.Sources.ExceptionDetailsProvider.GetDetails(Exception exception)+MoveNext()
   at Microsoft.AspNetCore.Diagnostics.RazorViews.ErrorPage.ExecuteAsync()
   at Microsoft.Extensions.RazorViews.BaseView.ExecuteAsync(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

Expected behavior

No exception should be thrown while getting the details of another exception. If the exception details can't be collected, show as many details as possible, but don't fail.

Additional context

It is important to use Oracle.ManagedDataAccess to reproduce the problem. Oracle.ManagedDataAccess.Core will not cause the problem! It looks like Oracle.ManagedDataAccess is obfuscated and I assume that causes the exception in PEReader.ReadDebugDirectory().
To explain why I use the non-core version of Oracle.ManagedDataAccess: I initially found this in an solution that runs ASP.NET Core 2.2 on .NET Framework 4.7.1

Output of dotnet --info:

.NET Core SDK (gemäß "global.json"):
 Version:   3.0.100
 Commit:    04339c3a26

Laufzeitumgebung:
 OS Name:     Windows
 OS Version:  10.0.17763
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.0.100\

Host (useful for support):
  Version: 3.0.0
  Commit:  7d57652f33

.NET Core SDKs installed:
  2.1.802 [C:\Program Files\dotnet\sdk]
  2.2.402 [C:\Program Files\dotnet\sdk]
  3.0.100 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
@mkArtakMSFT mkArtakMSFT added the External This is an issue in a component not contained in this repository. It is open for tracking purposes. label Oct 1, 2019
@analogrelay
Copy link
Contributor

It seems reasonable to do a special-case try..catch around this and just print out less detail. Looks like @khellang is looking at that :).

@analogrelay analogrelay removed the External This is an issue in a component not contained in this repository. It is open for tracking purposes. label Oct 1, 2019
@analogrelay analogrelay added this to the 5.0.0-preview1 milestone Oct 1, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2019
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants