From 92628c4033e59b18ee80d06d15495b0f3f3fe357 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Sat, 21 Mar 2020 21:03:48 +0100 Subject: Clean up HTTP listener exception handling --- .../HttpServer/HttpListenerHost.cs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 655130fcf..49179a2da 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -527,22 +527,18 @@ namespace Emby.Server.Implementations.HttpServer } else { - await ErrorHandler(new FileNotFoundException(), httpReq, false).ConfigureAwait(false); + throw new FileNotFoundException(); } } - catch (Exception ex) when (ex is SocketException || ex is IOException || ex is OperationCanceledException) - { - await ErrorHandler(ex, httpReq, false).ConfigureAwait(false); - } - catch (SecurityException ex) - { - await ErrorHandler(ex, httpReq, false).ConfigureAwait(false); - } catch (Exception ex) { - var logException = !string.Equals(ex.GetType().Name, "SocketException", StringComparison.OrdinalIgnoreCase); - - await ErrorHandler(ex, httpReq, logException).ConfigureAwait(false); + bool ignoreStackTrace = + ex is SocketException || + ex is IOException || + ex is OperationCanceledException || + ex is SecurityException || + ex is FileNotFoundException; + await ErrorHandler(ex, httpReq, ignoreStackTrace).ConfigureAwait(false); } finally { -- cgit v1.2.3 From 842ec048284acfbe9711e87ea4fce10adfa890bb Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Sat, 21 Mar 2020 21:06:01 +0100 Subject: Do not handle exceptions manually when in development mode --- .../Emby.Server.Implementations.csproj | 1 + Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/Emby.Server.Implementations.csproj b/Emby.Server.Implementations/Emby.Server.Implementations.csproj index f8560ca85..ae4f0bcf0 100644 --- a/Emby.Server.Implementations/Emby.Server.Implementations.csproj +++ b/Emby.Server.Implementations/Emby.Server.Implementations.csproj @@ -32,6 +32,7 @@ + diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 49179a2da..e8ea8d033 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -23,6 +23,7 @@ using MediaBrowser.Model.Services; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using ServiceStack.Text.Jsv; @@ -48,6 +49,8 @@ namespace Emby.Server.Implementations.HttpServer private readonly string _baseUrlPrefix; private readonly Dictionary _serviceOperationsMap = new Dictionary(); private readonly List _webSocketConnections = new List(); + private readonly IHostEnvironment _hostEnvironment; + private IWebSocketListener[] _webSocketListeners = Array.Empty(); private bool _disposed = false; @@ -60,7 +63,8 @@ namespace Emby.Server.Implementations.HttpServer IJsonSerializer jsonSerializer, IXmlSerializer xmlSerializer, IHttpListener socketListener, - ILocalizationManager localizationManager) + ILocalizationManager localizationManager, + IHostEnvironment hostEnvironment) { _appHost = applicationHost; _logger = logger; @@ -72,6 +76,7 @@ namespace Emby.Server.Implementations.HttpServer _xmlSerializer = xmlSerializer; _socketListener = socketListener; _socketListener.WebSocketConnected = OnWebSocketConnected; + _hostEnvironment = hostEnvironment; _funcParseFn = t => s => JsvReader.GetParseFn(t)(s); @@ -532,6 +537,13 @@ namespace Emby.Server.Implementations.HttpServer } catch (Exception ex) { + // Do not handle exceptions manually when in development mode + // The framework-defined development exception page will be returned instead + if (_hostEnvironment.IsDevelopment()) + { + throw; + } + bool ignoreStackTrace = ex is SocketException || ex is IOException || -- cgit v1.2.3 From de634203d812922bb04359a32e59189fb0110791 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Thu, 2 Apr 2020 14:31:56 -0400 Subject: Put Boolean operators at beginning of lines instead of the end --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 62735b7cf..72667a314 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -548,11 +548,11 @@ namespace Emby.Server.Implementations.HttpServer } bool ignoreStackTrace = - ex is SocketException || - ex is IOException || - ex is OperationCanceledException || - ex is SecurityException || - ex is FileNotFoundException; + ex is SocketException + || ex is IOException + || ex is OperationCanceledException + || ex is SecurityException + || ex is FileNotFoundException; await ErrorHandler(ex, httpReq, ignoreStackTrace).ConfigureAwait(false); } finally -- cgit v1.2.3 From 71d8e66d9fbca1d448d8d4ec7c1fe9be55134076 Mon Sep 17 00:00:00 2001 From: Vasily Date: Mon, 6 Apr 2020 14:42:41 +0300 Subject: Add logging of URL being processed when logging an error This might help diagnosing stuff like "Operation was cancelled" --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 72667a314..c12862145 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.IO; using System.Linq; using System.Net.Sockets; @@ -239,19 +240,22 @@ namespace Emby.Server.Implementations.HttpServer } } - private async Task ErrorHandler(Exception ex, IRequest httpReq, bool logExceptionStackTrace) + private async Task ErrorHandler(Exception ex, IRequest httpReq, bool logExceptionStackTrace, string urlToLog) { + string urlSuffix = string.IsNullOrWhiteSpace(urlToLog) + ? string.Format(CultureInfo.InvariantCulture, "; URL being processed: {0}", urlToLog) + : ""; try { ex = GetActualException(ex); if (logExceptionStackTrace) { - _logger.LogError(ex, "Error processing request"); + _logger.LogError(ex, "Error processing request{Suffix}", urlSuffix); } else { - _logger.LogError("Error processing request: {Message}", ex.Message); + _logger.LogError("Error processing request: {Message}{Suffix}", ex.Message, urlSuffix); } var httpRes = httpReq.Response; @@ -271,7 +275,7 @@ namespace Emby.Server.Implementations.HttpServer } catch (Exception errorEx) { - _logger.LogError(errorEx, "Error this.ProcessRequest(context)(Exception while writing error to the response)"); + _logger.LogError(errorEx, "Error this.ProcessRequest(context)(Exception while writing error to the response){Suffix}", urlSuffix); } } @@ -553,7 +557,7 @@ namespace Emby.Server.Implementations.HttpServer || ex is OperationCanceledException || ex is SecurityException || ex is FileNotFoundException; - await ErrorHandler(ex, httpReq, ignoreStackTrace).ConfigureAwait(false); + await ErrorHandler(ex, httpReq, ignoreStackTrace, urlToLog).ConfigureAwait(false); } finally { -- cgit v1.2.3 From 61d9c9df5b4ad4b79679cccbb2a624447c824b67 Mon Sep 17 00:00:00 2001 From: Vasily Date: Sun, 12 Apr 2020 23:26:45 +0300 Subject: Addressing review feedback --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index c12862145..dd60e7a18 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -242,9 +242,7 @@ namespace Emby.Server.Implementations.HttpServer private async Task ErrorHandler(Exception ex, IRequest httpReq, bool logExceptionStackTrace, string urlToLog) { - string urlSuffix = string.IsNullOrWhiteSpace(urlToLog) - ? string.Format(CultureInfo.InvariantCulture, "; URL being processed: {0}", urlToLog) - : ""; + string urlSuffix = string.Format(CultureInfo.InvariantCulture, "; URL being processed: {0}", urlToLog); try { ex = GetActualException(ex); @@ -460,7 +458,7 @@ namespace Emby.Server.Implementations.HttpServer var stopWatch = new Stopwatch(); stopWatch.Start(); var httpRes = httpReq.Response; - string urlToLog = null; + string urlToLog = GetUrlToLog(urlString); string remoteIp = httpReq.RemoteIp; try @@ -506,8 +504,6 @@ namespace Emby.Server.Implementations.HttpServer return; } - urlToLog = GetUrlToLog(urlString); - if (string.Equals(localPath, _baseUrlPrefix + "/", StringComparison.OrdinalIgnoreCase) || string.Equals(localPath, _baseUrlPrefix, StringComparison.OrdinalIgnoreCase) || string.Equals(localPath, "/", StringComparison.OrdinalIgnoreCase) -- cgit v1.2.3 From 30f439287266a8afd6f6e7bea01ed08eb86bf0d7 Mon Sep 17 00:00:00 2001 From: Vasily Date: Sun, 12 Apr 2020 23:35:41 +0300 Subject: Fix condition flipped by https://github.com/jellyfin/jellyfin/pull/2635 --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index dd60e7a18..229e5d199 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -553,7 +553,7 @@ namespace Emby.Server.Implementations.HttpServer || ex is OperationCanceledException || ex is SecurityException || ex is FileNotFoundException; - await ErrorHandler(ex, httpReq, ignoreStackTrace, urlToLog).ConfigureAwait(false); + await ErrorHandler(ex, httpReq, !ignoreStackTrace, urlToLog).ConfigureAwait(false); } finally { -- cgit v1.2.3 From 058c35e739d4d3193e236b106cacd7ebc3926705 Mon Sep 17 00:00:00 2001 From: Vasily Date: Sun, 12 Apr 2020 23:40:34 +0300 Subject: Fix log highlithing --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 229e5d199..95df6fbc4 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Globalization; using System.IO; using System.Linq; using System.Net.Sockets; @@ -242,18 +241,17 @@ namespace Emby.Server.Implementations.HttpServer private async Task ErrorHandler(Exception ex, IRequest httpReq, bool logExceptionStackTrace, string urlToLog) { - string urlSuffix = string.Format(CultureInfo.InvariantCulture, "; URL being processed: {0}", urlToLog); try { ex = GetActualException(ex); if (logExceptionStackTrace) { - _logger.LogError(ex, "Error processing request{Suffix}", urlSuffix); + _logger.LogError(ex, "Error processing request; URL being processed: {Url}", urlToLog); } else { - _logger.LogError("Error processing request: {Message}{Suffix}", ex.Message, urlSuffix); + _logger.LogError("Error processing request: {Message}; URL being processed: {Url}", ex.Message, urlToLog); } var httpRes = httpReq.Response; @@ -273,7 +271,7 @@ namespace Emby.Server.Implementations.HttpServer } catch (Exception errorEx) { - _logger.LogError(errorEx, "Error this.ProcessRequest(context)(Exception while writing error to the response){Suffix}", urlSuffix); + _logger.LogError(errorEx, "Error this.ProcessRequest(context)(Exception while writing error to the response); URL being processed: {Url}", urlToLog); } } -- cgit v1.2.3 From 3bdb5e80a53fe8148048fc4424b1e92adc62ac3d Mon Sep 17 00:00:00 2001 From: Vasily Date: Mon, 13 Apr 2020 00:45:09 +0300 Subject: More consise error messages --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 95df6fbc4..5ae65a4e3 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -247,11 +247,11 @@ namespace Emby.Server.Implementations.HttpServer if (logExceptionStackTrace) { - _logger.LogError(ex, "Error processing request; URL being processed: {Url}", urlToLog); + _logger.LogError(ex, "Error processing request. URL: {Url}", urlToLog); } else { - _logger.LogError("Error processing request: {Message}; URL being processed: {Url}", ex.Message, urlToLog); + _logger.LogError("Error processing request: {Message}. URL: {Url}", ex.Message.TrimEnd('.'), urlToLog); } var httpRes = httpReq.Response; @@ -271,7 +271,7 @@ namespace Emby.Server.Implementations.HttpServer } catch (Exception errorEx) { - _logger.LogError(errorEx, "Error this.ProcessRequest(context)(Exception while writing error to the response); URL being processed: {Url}", urlToLog); + _logger.LogError(errorEx, "Error this.ProcessRequest(context)(Exception while writing error to the response). URL: {Url}", urlToLog); } } -- cgit v1.2.3 From 53380689ad00f00efc0c1790f1d25d08c95d7f2d Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Mon, 13 Apr 2020 13:17:46 -0400 Subject: Return correct status codes for authentication and authorization errors - Use AuthenticatonException to return 401 - Use SecurityException to return 403 - Update existing throws to throw the correct exception for the circumstance --- Emby.Server.Implementations/HttpServer/HttpListenerHost.cs | 5 ++++- .../HttpServer/Security/AuthService.cs | 7 ++++--- Emby.Server.Implementations/Library/UserManager.cs | 11 ++++------- Emby.Server.Implementations/Session/SessionManager.cs | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 5ae65a4e3..f496ff1ba 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -14,6 +14,7 @@ using Emby.Server.Implementations.Services; using MediaBrowser.Common.Extensions; using MediaBrowser.Common.Net; using MediaBrowser.Controller; +using MediaBrowser.Controller.Authentication; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Net; using MediaBrowser.Model.Events; @@ -230,7 +231,8 @@ namespace Emby.Server.Implementations.HttpServer switch (ex) { case ArgumentException _: return 400; - case SecurityException _: return 401; + case AuthenticationException _: return 401; + case SecurityException _: return 403; case DirectoryNotFoundException _: case FileNotFoundException _: case ResourceNotFoundException _: return 404; @@ -550,6 +552,7 @@ namespace Emby.Server.Implementations.HttpServer || ex is IOException || ex is OperationCanceledException || ex is SecurityException + || ex is AuthenticationException || ex is FileNotFoundException; await ErrorHandler(ex, httpReq, !ignoreStackTrace, urlToLog).ConfigureAwait(false); } diff --git a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs index 1360a5e0c..256b24924 100644 --- a/Emby.Server.Implementations/HttpServer/Security/AuthService.cs +++ b/Emby.Server.Implementations/HttpServer/Security/AuthService.cs @@ -2,6 +2,7 @@ using System; using System.Linq; +using System.Security.Authentication; using Emby.Server.Implementations.SocketSharp; using MediaBrowser.Common.Net; using MediaBrowser.Controller.Configuration; @@ -68,7 +69,7 @@ namespace Emby.Server.Implementations.HttpServer.Security if (user == null && auth.UserId != Guid.Empty) { - throw new SecurityException("User with Id " + auth.UserId + " not found"); + throw new AuthenticationException("User with Id " + auth.UserId + " not found"); } if (user != null) @@ -212,14 +213,14 @@ namespace Emby.Server.Implementations.HttpServer.Security { if (string.IsNullOrEmpty(token)) { - throw new SecurityException("Access token is required."); + throw new AuthenticationException("Access token is required."); } var info = GetTokenInfo(request); if (info == null) { - throw new SecurityException("Access token is invalid or expired."); + throw new AuthenticationException("Access token is invalid or expired."); } //if (!string.IsNullOrEmpty(info.UserId)) diff --git a/Emby.Server.Implementations/Library/UserManager.cs b/Emby.Server.Implementations/Library/UserManager.cs index 7b17cc913..f92cb6ae6 100644 --- a/Emby.Server.Implementations/Library/UserManager.cs +++ b/Emby.Server.Implementations/Library/UserManager.cs @@ -20,6 +20,7 @@ using MediaBrowser.Controller.Drawing; using MediaBrowser.Controller.Dto; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Net; using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Plugins; using MediaBrowser.Controller.Providers; @@ -324,21 +325,17 @@ namespace Emby.Server.Implementations.Library if (user.Policy.IsDisabled) { - throw new AuthenticationException( - string.Format( - CultureInfo.InvariantCulture, - "The {0} account is currently disabled. Please consult with your administrator.", - user.Name)); + throw new SecurityException($"The {user.Name} account is currently disabled. Please consult with your administrator."); } if (!user.Policy.EnableRemoteAccess && !_networkManager.IsInLocalNetwork(remoteEndPoint)) { - throw new AuthenticationException("Forbidden."); + throw new SecurityException("Forbidden."); } if (!user.IsParentalScheduleAllowed()) { - throw new AuthenticationException("User is not allowed access at this time."); + throw new SecurityException("User is not allowed access at this time."); } // Update LastActivityDate and LastLoginDate, then save diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs index de768333d..c93c02c48 100644 --- a/Emby.Server.Implementations/Session/SessionManager.cs +++ b/Emby.Server.Implementations/Session/SessionManager.cs @@ -1414,7 +1414,7 @@ namespace Emby.Server.Implementations.Session if (user == null) { AuthenticationFailed?.Invoke(this, new GenericEventArgs(request)); - throw new SecurityException("Invalid username or password entered."); + throw new AuthenticationException("Invalid username or password entered."); } if (!string.IsNullOrEmpty(request.DeviceId) -- cgit v1.2.3 From a8c3951c1798ced5c10631da7d9ca64731a12f26 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Mon, 13 Apr 2020 15:26:49 -0400 Subject: Only show developer exception page for 500 server exceptions Other response codes should be returned as normal --- .../HttpServer/HttpListenerHost.cs | 89 ++++++++++++---------- 1 file changed, 48 insertions(+), 41 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index f496ff1ba..4c69b35e2 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -241,40 +241,38 @@ namespace Emby.Server.Implementations.HttpServer } } - private async Task ErrorHandler(Exception ex, IRequest httpReq, bool logExceptionStackTrace, string urlToLog) + private async Task ErrorHandler(Exception ex, IRequest httpReq, int statusCode, string urlToLog) { - try - { - ex = GetActualException(ex); - - if (logExceptionStackTrace) - { - _logger.LogError(ex, "Error processing request. URL: {Url}", urlToLog); - } - else - { - _logger.LogError("Error processing request: {Message}. URL: {Url}", ex.Message.TrimEnd('.'), urlToLog); - } + bool ignoreStackTrace = + ex is SocketException + || ex is IOException + || ex is OperationCanceledException + || ex is SecurityException + || ex is AuthenticationException + || ex is FileNotFoundException; - var httpRes = httpReq.Response; - - if (httpRes.HasStarted) - { - return; - } + if (ignoreStackTrace) + { + _logger.LogError("Error processing request: {Message}. URL: {Url}", ex.Message.TrimEnd('.'), urlToLog); + } + else + { + _logger.LogError(ex, "Error processing request. URL: {Url}", urlToLog); + } - var statusCode = GetStatusCode(ex); - httpRes.StatusCode = statusCode; + var httpRes = httpReq.Response; - var errContent = NormalizeExceptionMessage(ex.Message); - httpRes.ContentType = "text/plain"; - httpRes.ContentLength = errContent.Length; - await httpRes.WriteAsync(errContent).ConfigureAwait(false); - } - catch (Exception errorEx) + if (httpRes.HasStarted) { - _logger.LogError(errorEx, "Error this.ProcessRequest(context)(Exception while writing error to the response). URL: {Url}", urlToLog); + return; } + + httpRes.StatusCode = statusCode; + + var errContent = NormalizeExceptionMessage(ex.Message); + httpRes.ContentType = "text/plain"; + httpRes.ContentLength = errContent.Length; + await httpRes.WriteAsync(errContent).ConfigureAwait(false); } private string NormalizeExceptionMessage(string msg) @@ -538,23 +536,32 @@ namespace Emby.Server.Implementations.HttpServer throw new FileNotFoundException(); } } - catch (Exception ex) + catch (Exception requestEx) { - // Do not handle exceptions manually when in development mode - // The framework-defined development exception page will be returned instead - if (_hostEnvironment.IsDevelopment()) + try { - throw; + var requestInnerEx = GetActualException(requestEx); + var statusCode = GetStatusCode(requestInnerEx); + + // Do not handle 500 server exceptions manually when in development mode + // The framework-defined development exception page will be returned instead + if (statusCode == 500 && _hostEnvironment.IsDevelopment()) + { + throw; + } + + await ErrorHandler(requestInnerEx, httpReq, statusCode, urlToLog).ConfigureAwait(false); } + catch (Exception handlerException) + { + var aggregateEx = new AggregateException("Error while handling request exception", requestEx, handlerException); + _logger.LogError(aggregateEx, "Error while handling exception in response to {Url}", urlToLog); - bool ignoreStackTrace = - ex is SocketException - || ex is IOException - || ex is OperationCanceledException - || ex is SecurityException - || ex is AuthenticationException - || ex is FileNotFoundException; - await ErrorHandler(ex, httpReq, !ignoreStackTrace, urlToLog).ConfigureAwait(false); + if (_hostEnvironment.IsDevelopment()) + { + throw aggregateEx; + } + } } finally { -- cgit v1.2.3 From 8b4b4b4127945354731036e13ca0b5366134958d Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Mon, 13 Apr 2020 16:10:55 -0400 Subject: Do not return the exception message to the client for AuthenticationExceptions --- .../HttpServer/HttpListenerHost.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'Emby.Server.Implementations/HttpServer/HttpListenerHost.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 4c69b35e2..211a0c1d9 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -269,25 +269,24 @@ namespace Emby.Server.Implementations.HttpServer httpRes.StatusCode = statusCode; - var errContent = NormalizeExceptionMessage(ex.Message); + var errContent = NormalizeExceptionMessage(ex) ?? string.Empty; httpRes.ContentType = "text/plain"; httpRes.ContentLength = errContent.Length; await httpRes.WriteAsync(errContent).ConfigureAwait(false); } - private string NormalizeExceptionMessage(string msg) + private string NormalizeExceptionMessage(Exception ex) { - if (msg == null) + // Do not expose the exception message for AuthenticationException + if (ex is AuthenticationException) { - return string.Empty; + return null; } // Strip any information we don't want to reveal - - msg = msg.Replace(_config.ApplicationPaths.ProgramSystemPath, string.Empty, StringComparison.OrdinalIgnoreCase); - msg = msg.Replace(_config.ApplicationPaths.ProgramDataPath, string.Empty, StringComparison.OrdinalIgnoreCase); - - return msg; + return ex.Message + ?.Replace(_config.ApplicationPaths.ProgramSystemPath, string.Empty, StringComparison.OrdinalIgnoreCase) + .Replace(_config.ApplicationPaths.ProgramDataPath, string.Empty, StringComparison.OrdinalIgnoreCase); } /// -- cgit v1.2.3