From fc92d40b3a9d0c1be354e0c98d71779b1fcf32f8 Mon Sep 17 00:00:00 2001 From: Wincent Date: Mon, 13 Mar 2017 10:45:16 +0100 Subject: [PATCH 01/12] Fixes jquery selector bug, http://issues.umbraco.org/issue/U4-8418 --- .../common/directives/components/events/events.directive.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/events/events.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/events/events.directive.js index 1b9dc090bb..6cf5f68461 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/events/events.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/events/events.directive.js @@ -163,13 +163,13 @@ angular.module('umbraco.directives') } // ignore clicks on dialog from old dialog service - var oldDialog = $(el).parents("#old-dialog-service"); + var oldDialog = $(event.target).parents("#old-dialog-service"); if (oldDialog.length === 1) { return; } // ignore clicks in tinyMCE dropdown(floatpanel) - var floatpanel = $(el).parents(".mce-floatpanel"); + var floatpanel = $(event.target).closest(".mce-floatpanel"); if (floatpanel.length === 1) { return; } From e432b7061d76cf38c033c2814de07e093791a5bc Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 14 Mar 2017 19:02:31 +0100 Subject: [PATCH 02/12] U4-9606 - New DumpOnTimeoutThreadAbort debug option --- src/Umbraco.Core/Configuration/CoreDebug.cs | 27 +++++++++++++++++++ src/Umbraco.Core/Logging/Logger.cs | 30 +++++++++++++++++---- src/Umbraco.Core/Umbraco.Core.csproj | 2 ++ 3 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/CoreDebug.cs diff --git a/src/Umbraco.Core/Configuration/CoreDebug.cs b/src/Umbraco.Core/Configuration/CoreDebug.cs new file mode 100644 index 0000000000..ff55be966b --- /dev/null +++ b/src/Umbraco.Core/Configuration/CoreDebug.cs @@ -0,0 +1,27 @@ +using System; + +namespace Umbraco.Core.Configuration +{ + internal static class CoreDebugExtensions + { + private static CoreDebug _coreDebug; + + public static CoreDebug CoreDebug(this UmbracoConfig config) + { + return _coreDebug ?? (_coreDebug = new CoreDebug()); + } + } + + internal class CoreDebug + { + public CoreDebug() + { + var appSettings = System.Configuration.ConfigurationManager.AppSettings; + DumpOnTimeoutThreadAbort = string.Equals("true", appSettings["Umbraco.CoreDebug.DumpOnTimeoutThreadAbort"], StringComparison.OrdinalIgnoreCase); + } + + // when true, the Logger creates a minidump of w3wp in ~/App_Data/MiniDump whenever it logs + // an error due to a ThreadAbortException that is due to a timeout. + public bool DumpOnTimeoutThreadAbort { get; private set; } + } +} diff --git a/src/Umbraco.Core/Logging/Logger.cs b/src/Umbraco.Core/Logging/Logger.cs index 66cad59733..30b1260305 100644 --- a/src/Umbraco.Core/Logging/Logger.cs +++ b/src/Umbraco.Core/Logging/Logger.cs @@ -7,6 +7,8 @@ using System.Threading; using System.Web; using log4net; using log4net.Config; +using Umbraco.Core.Configuration; +using Umbraco.Core.Diagnostics; namespace Umbraco.Core.Logging { @@ -57,21 +59,39 @@ namespace Umbraco.Core.Logging internal ILog LoggerFor(object getTypeFromInstance) { if (getTypeFromInstance == null) throw new ArgumentNullException("getTypeFromInstance"); - + return LogManager.GetLogger(getTypeFromInstance.GetType()); } - + public void Error(Type callingType, string message, Exception exception) { var logger = LogManager.GetLogger(callingType); if (logger == null) return; + var dump = false; + if (IsTimeoutThreadAbortException(exception)) { message += "\r\nThe thread has been aborted, because the request has timed out."; + dump = UmbracoConfig.For.CoreDebug().DumpOnTimeoutThreadAbort; } - logger.Error(message, exception); + if (dump) + { + try + { + var dumped = MiniDump.Dump(withException: true); + message += dumped + ? "\r\nA minidump was created in App_Data/MiniDump" + : "\r\nFailed to create a minidump"; + } + catch (Exception e) + { + message += string.Format("\r\nFailed to create a minidump ({0}: {1})", e.GetType().FullName, e.Message); + } + } + + logger.Error(message, exception); } private static bool IsTimeoutThreadAbortException(Exception exception) @@ -105,7 +125,7 @@ namespace Umbraco.Core.Logging if (showHttpTrace && HttpContext.Current != null) { HttpContext.Current.Trace.Warn(callingType.Name, string.Format(message, formatItems.Select(x => x.Invoke()).ToArray())); - } + } var logger = LogManager.GetLogger(callingType); if (logger == null || logger.IsWarnEnabled == false) return; @@ -122,7 +142,7 @@ namespace Umbraco.Core.Logging var logger = LogManager.GetLogger(callingType); if (logger == null || logger.IsWarnEnabled == false) return; var executedParams = formatItems.Select(x => x.Invoke()).ToArray(); - logger.WarnFormat((message) + ". Exception: " + e, executedParams); + logger.WarnFormat((message) + ". Exception: " + e, executedParams); } /// diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index e03dabfa5f..b22301bc8e 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -180,6 +180,7 @@ + @@ -300,6 +301,7 @@ + From f7f23015d02c7278a5f30d5ebacda152815d63ae Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 15 Mar 2017 09:39:35 +0100 Subject: [PATCH 03/12] U4-9606 - missing file --- src/Umbraco.Core/Diagnostics/MiniDump.cs | 119 +++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 src/Umbraco.Core/Diagnostics/MiniDump.cs diff --git a/src/Umbraco.Core/Diagnostics/MiniDump.cs b/src/Umbraco.Core/Diagnostics/MiniDump.cs new file mode 100644 index 0000000000..ded4b45667 --- /dev/null +++ b/src/Umbraco.Core/Diagnostics/MiniDump.cs @@ -0,0 +1,119 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Runtime.InteropServices; +using Umbraco.Core.IO; + +namespace Umbraco.Core.Diagnostics +{ + // taken from https://blogs.msdn.microsoft.com/dondu/2010/10/24/writing-minidumps-in-c/ + // and https://blogs.msdn.microsoft.com/dondu/2010/10/31/writing-minidumps-from-exceptions-in-c/ + // which itself got it from http://blog.kalmbach-software.de/2008/12/13/writing-minidumps-in-c/ + + internal static class MiniDump + { + [Flags] + public enum Option : uint + { + // From dbghelp.h: + Normal = 0x00000000, + WithDataSegs = 0x00000001, + WithFullMemory = 0x00000002, + WithHandleData = 0x00000004, + FilterMemory = 0x00000008, + ScanMemory = 0x00000010, + WithUnloadedModules = 0x00000020, + WithIndirectlyReferencedMemory = 0x00000040, + FilterModulePaths = 0x00000080, + WithProcessThreadData = 0x00000100, + WithPrivateReadWriteMemory = 0x00000200, + WithoutOptionalData = 0x00000400, + WithFullMemoryInfo = 0x00000800, + WithThreadInfo = 0x00001000, + WithCodeSegs = 0x00002000, + WithoutAuxiliaryState = 0x00004000, + WithFullAuxiliaryState = 0x00008000, + WithPrivateWriteCopyMemory = 0x00010000, + IgnoreInaccessibleMemory = 0x00020000, + ValidTypeFlags = 0x0003ffff, + } + + //typedef struct _MINIDUMP_EXCEPTION_INFORMATION { + // DWORD ThreadId; + // PEXCEPTION_POINTERS ExceptionPointers; + // BOOL ClientPointers; + //} MINIDUMP_EXCEPTION_INFORMATION, *PMINIDUMP_EXCEPTION_INFORMATION; + [StructLayout(LayoutKind.Sequential, Pack = 4)] // Pack=4 is important! So it works also for x64! + public struct MiniDumpExceptionInformation + { + public uint ThreadId; + public IntPtr ExceptionPointers; + [MarshalAs(UnmanagedType.Bool)] + public bool ClientPointers; + } + + //BOOL + //WINAPI + //MiniDumpWriteDump( + // __in HANDLE hProcess, + // __in DWORD ProcessId, + // __in HANDLE hFile, + // __in MINIDUMP_TYPE DumpType, + // __in_opt PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam, + // __in_opt PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam, + // __in_opt PMINIDUMP_CALLBACK_INFORMATION CallbackParam + // ); + + // Overload requiring MiniDumpExceptionInformation + [DllImport("dbghelp.dll", EntryPoint = "MiniDumpWriteDump", CallingConvention = CallingConvention.StdCall, CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true)] + private static extern bool MiniDumpWriteDump(IntPtr hProcess, uint processId, SafeHandle hFile, uint dumpType, ref MiniDumpExceptionInformation expParam, IntPtr userStreamParam, IntPtr callbackParam); + + // Overload supporting MiniDumpExceptionInformation == NULL + [DllImport("dbghelp.dll", EntryPoint = "MiniDumpWriteDump", CallingConvention = CallingConvention.StdCall, CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true)] + private static extern bool MiniDumpWriteDump(IntPtr hProcess, uint processId, SafeHandle hFile, uint dumpType, IntPtr expParam, IntPtr userStreamParam, IntPtr callbackParam); + + [DllImport("kernel32.dll", EntryPoint = "GetCurrentThreadId", ExactSpelling = true)] + private static extern uint GetCurrentThreadId(); + + public static bool Write(SafeHandle fileHandle, Option options, bool withException = false) + { + var currentProcess = Process.GetCurrentProcess(); + var currentProcessHandle = currentProcess.Handle; + var currentProcessId = (uint)currentProcess.Id; + + MiniDumpExceptionInformation exp; + + exp.ThreadId = GetCurrentThreadId(); + exp.ClientPointers = false; + exp.ExceptionPointers = IntPtr.Zero; + + if (withException) + exp.ExceptionPointers = Marshal.GetExceptionPointers(); + + var bRet = exp.ExceptionPointers == IntPtr.Zero + ? MiniDumpWriteDump(currentProcessHandle, currentProcessId, fileHandle, (uint) options, IntPtr.Zero, IntPtr.Zero, IntPtr.Zero) + : MiniDumpWriteDump(currentProcessHandle, currentProcessId, fileHandle, (uint) options, ref exp, IntPtr.Zero, IntPtr.Zero); + + return bRet; + } + + public static bool Dump(Option options = Option.WithFullMemory, bool withException = false) + { + // work around "stack trace is not available while minidump debugging", + // by making sure a local var (that we can inspect) contains the stack trace. + // getting the call stack before it is unwound would require a special exception + // filter everywhere in our code = not! + var stacktrace = withException ? Environment.StackTrace : string.Empty; + + var filepath = IOHelper.MapPath("~/App_Data/MiniDump"); + if (Directory.Exists(filepath) == false) + Directory.CreateDirectory(filepath); + + var filename = Path.Combine(filepath, string.Format("{0:yyyyMMddTHHmmss}.{1}.dmp", DateTime.UtcNow, Guid.NewGuid().ToString("N").Substring(0, 4))); + using (var stream = new FileStream(filename, FileMode.Create, FileAccess.ReadWrite, FileShare.Write)) + { + return Write(stream.SafeFileHandle, options, withException); + } + } + } +} From 27539d4aade350d780f79ae19716878d8135a4b3 Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 16 Mar 2017 14:04:28 +0100 Subject: [PATCH 04/12] U4-9606 - always minidump on Monitor.ReliableEnter exceptions --- src/Umbraco.Core/Logging/Logger.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Logging/Logger.cs b/src/Umbraco.Core/Logging/Logger.cs index 30b1260305..f575e3c117 100644 --- a/src/Umbraco.Core/Logging/Logger.cs +++ b/src/Umbraco.Core/Logging/Logger.cs @@ -73,7 +73,7 @@ namespace Umbraco.Core.Logging if (IsTimeoutThreadAbortException(exception)) { message += "\r\nThe thread has been aborted, because the request has timed out."; - dump = UmbracoConfig.For.CoreDebug().DumpOnTimeoutThreadAbort; + dump = UmbracoConfig.For.CoreDebug().DumpOnTimeoutThreadAbort || IsMonitorEnterThreadAbortException(exception); } if (dump) @@ -94,6 +94,15 @@ namespace Umbraco.Core.Logging logger.Error(message, exception); } + private static bool IsMonitorEnterThreadAbortException(Exception exception) + { + var abort = exception as ThreadAbortException; + if (abort == null) return false; + + var stacktrace = abort.StackTrace; + return stacktrace.Contains("System.Threading.Monitor.ReliableEnter"); + } + private static bool IsTimeoutThreadAbortException(Exception exception) { var abort = exception as ThreadAbortException; From 333b35d8b97546f20781b795feff524f83c8813b Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 16 Mar 2017 14:45:05 +0100 Subject: [PATCH 05/12] U4-9606 - hard cap of 8 minidump files --- src/Umbraco.Core/Diagnostics/MiniDump.cs | 44 ++++++++++++++++-------- src/Umbraco.Core/Logging/Logger.cs | 5 +++ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/Umbraco.Core/Diagnostics/MiniDump.cs b/src/Umbraco.Core/Diagnostics/MiniDump.cs index ded4b45667..e8c2e82f94 100644 --- a/src/Umbraco.Core/Diagnostics/MiniDump.cs +++ b/src/Umbraco.Core/Diagnostics/MiniDump.cs @@ -12,6 +12,8 @@ namespace Umbraco.Core.Diagnostics internal static class MiniDump { + private static readonly object LockO = new object(); + [Flags] public enum Option : uint { @@ -75,7 +77,7 @@ namespace Umbraco.Core.Diagnostics [DllImport("kernel32.dll", EntryPoint = "GetCurrentThreadId", ExactSpelling = true)] private static extern uint GetCurrentThreadId(); - public static bool Write(SafeHandle fileHandle, Option options, bool withException = false) + private static bool Write(SafeHandle fileHandle, Option options, bool withException = false) { var currentProcess = Process.GetCurrentProcess(); var currentProcessHandle = currentProcess.Handle; @@ -99,20 +101,34 @@ namespace Umbraco.Core.Diagnostics public static bool Dump(Option options = Option.WithFullMemory, bool withException = false) { - // work around "stack trace is not available while minidump debugging", - // by making sure a local var (that we can inspect) contains the stack trace. - // getting the call stack before it is unwound would require a special exception - // filter everywhere in our code = not! - var stacktrace = withException ? Environment.StackTrace : string.Empty; - - var filepath = IOHelper.MapPath("~/App_Data/MiniDump"); - if (Directory.Exists(filepath) == false) - Directory.CreateDirectory(filepath); - - var filename = Path.Combine(filepath, string.Format("{0:yyyyMMddTHHmmss}.{1}.dmp", DateTime.UtcNow, Guid.NewGuid().ToString("N").Substring(0, 4))); - using (var stream = new FileStream(filename, FileMode.Create, FileAccess.ReadWrite, FileShare.Write)) + lock (LockO) { - return Write(stream.SafeFileHandle, options, withException); + // work around "stack trace is not available while minidump debugging", + // by making sure a local var (that we can inspect) contains the stack trace. + // getting the call stack before it is unwound would require a special exception + // filter everywhere in our code = not! + var stacktrace = withException ? Environment.StackTrace : string.Empty; + + var filepath = IOHelper.MapPath("~/App_Data/MiniDump"); + if (Directory.Exists(filepath) == false) + Directory.CreateDirectory(filepath); + + var filename = Path.Combine(filepath, string.Format("{0:yyyyMMddTHHmmss}.{1}.dmp", DateTime.UtcNow, Guid.NewGuid().ToString("N").Substring(0, 4))); + using (var stream = new FileStream(filename, FileMode.Create, FileAccess.ReadWrite, FileShare.Write)) + { + return Write(stream.SafeFileHandle, options, withException); + } + } + } + + public static bool OkToDump() + { + lock (LockO) + { + var filepath = IOHelper.MapPath("~/App_Data/MiniDump"); + if (Directory.Exists(filepath) == false) return true; + var count = Directory.GetFiles(filepath, "*.dmp").Length; + return count < 8; } } } diff --git a/src/Umbraco.Core/Logging/Logger.cs b/src/Umbraco.Core/Logging/Logger.cs index f575e3c117..d0bfcbfca0 100644 --- a/src/Umbraco.Core/Logging/Logger.cs +++ b/src/Umbraco.Core/Logging/Logger.cs @@ -73,7 +73,12 @@ namespace Umbraco.Core.Logging if (IsTimeoutThreadAbortException(exception)) { message += "\r\nThe thread has been aborted, because the request has timed out."; + + // dump if configured, or if stacktrace contains Monitor.ReliableEnter dump = UmbracoConfig.For.CoreDebug().DumpOnTimeoutThreadAbort || IsMonitorEnterThreadAbortException(exception); + + // dump if it is ok to dump (might have a cap on number of dump...) + dump &= MiniDump.OkToDump(); } if (dump) From fffd508bb2e932b97530a377a50cfedff2409621 Mon Sep 17 00:00:00 2001 From: Shannon Date: Sat, 18 Mar 2017 15:44:57 +1100 Subject: [PATCH 06/12] missing null check --- src/umbraco.cms/businesslogic/Packager/data.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/umbraco.cms/businesslogic/Packager/data.cs b/src/umbraco.cms/businesslogic/Packager/data.cs index 7781f1790f..536be6d435 100644 --- a/src/umbraco.cms/businesslogic/Packager/data.cs +++ b/src/umbraco.cms/businesslogic/Packager/data.cs @@ -302,7 +302,11 @@ namespace umbraco.cms.businesslogic.packager XmlHelper.SetAttribute(Source, xmlDef, "enableSkins", package.EnableSkins.ToString()); XmlHelper.SetAttribute(Source, xmlDef, "skinRepoGuid", package.SkinRepoGuid.ToString()); XmlHelper.SetAttribute(Source, xmlDef, "iconUrl", package.IconUrl); - XmlHelper.SetAttribute(Source, xmlDef, "umbVersion", package.UmbracoVersion.ToString(3)); + if (package.UmbracoVersion != null) + { + XmlHelper.SetAttribute(Source, xmlDef, "umbVersion", package.UmbracoVersion.ToString(3)); + } + var licenseNode = xmlDef.SelectSingleNode("license"); if (licenseNode == null) From 94b7c36d62808f41c0560d45866e8f7fbe433dea Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 22 Mar 2017 12:16:33 +0100 Subject: [PATCH 07/12] U4-6811 - fix preview set deletion --- .../umbraco/preview/PreviewContent.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs index 2499c1c817..44faec0868 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs @@ -99,7 +99,7 @@ namespace umbraco.presentation.preview //Inject preview xml parentId = document.Level == 1 ? -1 : document.ParentId; var previewXml = document.ToPreviewXml(XmlContent); - if (document.ContentEntity.Published == false + if (document.ContentEntity.Published == false && ApplicationContext.Current.Services.ContentService.HasPublishedVersion(document.Id)) previewXml.Attributes.Append(XmlContent.CreateAttribute("isDraft")); XmlContent = content.GetAddOrUpdateXmlNode(XmlContent, document.Id, document.Level, parentId, previewXml); @@ -187,13 +187,9 @@ namespace umbraco.presentation.preview private static void CleanPreviewDirectory(int userId, DirectoryInfo dir) { - foreach (FileInfo file in dir.GetFiles(userId + "_*.config")) - { - DeletePreviewFile(userId, file); - } // also delete any files accessed more than 10 minutes ago var now = DateTime.Now; - foreach (FileInfo file in dir.GetFiles("*.config")) + foreach (var file in dir.GetFiles("*.config")) { if ((now - file.LastAccessTime).TotalMinutes > 10) DeletePreviewFile(userId, file); @@ -208,7 +204,11 @@ namespace umbraco.presentation.preview } catch (Exception ex) { - LogHelper.Error(string.Format("Couldn't delete preview set: {0} - User {1}", file.Name, userId), ex); + // for *some* reason deleting the file can fail, + // and it will work later on (long-lasting locks, etc), + // so just ignore the exception + + //LogHelper.Error(string.Format("Couldn't delete preview set: {0} - User {1}", file.Name, userId), ex); } } From d5c963917027d57214a316c20d713cb060ef41cc Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 22 Mar 2017 13:58:40 +0100 Subject: [PATCH 08/12] U4-9658 - detect umbraco.config rogue content types, and fix --- .../XmlPublishedCache/XmlPublishedContent.cs | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs index 202fe551c7..4e251d746e 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Xml; using System.Xml.Serialization; using System.Xml.XPath; +using umbraco; using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Models; @@ -315,9 +316,9 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache private void InitializeNode() { - InitializeNode(_xmlNode, UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema, _isPreviewing, - out _id, out _key, out _template, out _sortOrder, out _name, out _writerName, - out _urlName, out _creatorName, out _creatorId, out _writerId, out _docTypeAlias, out _docTypeId, out _path, + InitializeNode(_xmlNode, UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema, _isPreviewing, + out _id, out _key, out _template, out _sortOrder, out _name, out _writerName, + out _urlName, out _creatorName, out _creatorId, out _writerId, out _docTypeAlias, out _docTypeId, out _path, out _version, out _createDate, out _updateDate, out _level, out _isDraft, out _contentType, out _properties, PublishedContentType.Get); @@ -345,7 +346,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache //return if this is null if (xmlNode == null) - { + { return; } @@ -407,8 +408,8 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache } //dictionary to store the property node data - var propertyNodes = new Dictionary(); - + var propertyNodes = new Dictionary(); + foreach (XmlNode n in xmlNode.ChildNodes) { var e = n as XmlElement; @@ -432,7 +433,20 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache } //lookup the content type and create the properties collection - contentType = getPublishedContentType(PublishedItemType.Content, docTypeAlias); + try + { + contentType = getPublishedContentType(PublishedItemType.Content, docTypeAlias); + + } + catch (Exception e) + { + if (e.TargetSite.Name == "CreatePublishedContentType") + { + content.Instance.RefreshContentFromDatabase(); + throw new Exception(string.Format("The content cache failed to find a content type with alias \"{0}\". This usually indicates that the content cache is corrupt; the content cache has been rebuilt in an attempt to self-fix the issue.", docTypeAlias)); + } + throw; + } properties = new Dictionary(StringComparer.OrdinalIgnoreCase); //fill in the property collection From 631404743fb532d75ca0a380f74f34c19d416d36 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 22 Mar 2017 14:39:11 +0100 Subject: [PATCH 09/12] U4-9610 - fix nullref in BulkPublishController --- src/Umbraco.Core/Publishing/PublishStatus.cs | 7 +-- .../Publishing/PublishingStrategy.cs | 43 ++++++++------- src/umbraco.cms/businesslogic/web/Document.cs | 52 ++++++++----------- 3 files changed, 49 insertions(+), 53 deletions(-) diff --git a/src/Umbraco.Core/Publishing/PublishStatus.cs b/src/Umbraco.Core/Publishing/PublishStatus.cs index 3436e9070e..a2fdf1d6cd 100644 --- a/src/Umbraco.Core/Publishing/PublishStatus.cs +++ b/src/Umbraco.Core/Publishing/PublishStatus.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using Umbraco.Core.Events; using Umbraco.Core.Models; using Umbraco.Core.Services; @@ -12,7 +13,8 @@ namespace Umbraco.Core.Publishing { public PublishStatus(IContent content, PublishStatusType statusType, EventMessages eventMessages) : base(content, statusType, eventMessages) - { + { + InvalidProperties = Enumerable.Empty(); } /// @@ -20,8 +22,7 @@ namespace Umbraco.Core.Publishing /// public PublishStatus(IContent content, EventMessages eventMessages) : this(content, PublishStatusType.Success, eventMessages) - { - } + { } public IContent ContentItem { diff --git a/src/Umbraco.Core/Publishing/PublishingStrategy.cs b/src/Umbraco.Core/Publishing/PublishingStrategy.cs index c6d4f19baf..37630f2638 100644 --- a/src/Umbraco.Core/Publishing/PublishingStrategy.cs +++ b/src/Umbraco.Core/Publishing/PublishingStrategy.cs @@ -30,7 +30,7 @@ namespace Umbraco.Core.Publishing /// Publishes a single piece of Content /// /// to publish - /// Id of the User issueing the publish operation + /// Id of the User issueing the publish operation internal Attempt PublishInternal(IContent content, int userId) { var evtMsgs = _eventMessagesFactory.Get(); @@ -99,26 +99,26 @@ namespace Umbraco.Core.Publishing /// By default this is set to true which means that it will publish any content item in the list that is completely unpublished and /// not visible on the front-end. If set to false, this will only publish content that is live on the front-end but has new versions /// that have yet to be published. - /// + /// /// /// - /// + /// /// This method becomes complex once we start to be able to cancel events or stop publishing a content item in any way because if a /// content item is not published then it's children shouldn't be published either. This rule will apply for the following conditions: /// * If a document fails to be published, do not proceed to publish it's children if: /// ** The document does not have a publish version /// ** The document does have a published version but the includeUnpublishedDocuments = false - /// + /// /// In order to do this, we will order the content by level and begin by publishing each item at that level, then proceed to the next - /// level and so on. If we detect that the above rule applies when the document publishing is cancelled we'll add it to the list of + /// level and so on. If we detect that the above rule applies when the document publishing is cancelled we'll add it to the list of /// parentsIdsCancelled so that it's children don't get published. - /// + /// /// Its important to note that all 'root' documents included in the list *will* be published regardless of the rules mentioned - /// above (unless it is invalid)!! By 'root' documents we are referring to documents in the list with the minimum value for their 'level'. - /// In most cases the 'root' documents will only be one document since under normal circumstance we only publish one document and + /// above (unless it is invalid)!! By 'root' documents we are referring to documents in the list with the minimum value for their 'level'. + /// In most cases the 'root' documents will only be one document since under normal circumstance we only publish one document and /// its children. The reason we have to do this is because if a user is publishing a document and it's children, it is implied that /// the user definitely wants to publish it even if it has never been published before. - /// + /// /// internal IEnumerable> PublishWithChildrenInternal( IEnumerable content, int userId, bool includeUnpublishedDocuments = true) @@ -132,7 +132,7 @@ namespace Umbraco.Core.Publishing //group by levels and iterate over the sorted ascending level. //TODO: This will cause all queries to execute, they will not be lazy but I'm not really sure being lazy actually made - // much difference because we iterate over them all anyways?? Morten? + // much difference because we iterate over them all anyways?? Morten? // Because we're grouping I think this will execute all the queries anyways so need to fetch it all first. var fetchedContent = content.ToArray(); @@ -149,7 +149,7 @@ namespace Umbraco.Core.Publishing var levelGroups = fetchedContent.GroupBy(x => x.Level); foreach (var level in levelGroups.OrderBy(x => x.Key)) { - //set the first level flag, used to ensure that all documents at the first level will + //set the first level flag, used to ensure that all documents at the first level will //be published regardless of the rules mentioned in the remarks. if (!firstLevel.HasValue) { @@ -200,7 +200,10 @@ namespace Umbraco.Core.Publishing _logger.Info( string.Format("Content '{0}' with Id '{1}' will not be published because some of it's content is not passing validation rules.", item.Name, item.Id)); - statuses.Add(Attempt.Fail(new PublishStatus(item, PublishStatusType.FailedContentInvalid, evtMsgs))); + statuses.Add(Attempt.Fail(new PublishStatus(item, PublishStatusType.FailedContentInvalid, evtMsgs) + { + InvalidProperties = ((ContentBase)item).LastInvalidProperties + })); //Does this document apply to our rule to cancel it's children being published? CheckCancellingOfChildPublishing(item, parentsIdsCancelled, includeUnpublishedDocuments); @@ -272,11 +275,11 @@ namespace Umbraco.Core.Publishing /// /// /// See remarks on method: PublishWithChildrenInternal - /// + /// private void CheckCancellingOfChildPublishing(IContent content, List parentsIdsCancelled, bool includeUnpublishedDocuments) { //Does this document apply to our rule to cancel it's children being published? - //TODO: We're going back to the service layer here... not sure how to avoid this? And this will add extra overhead to + //TODO: We're going back to the service layer here... not sure how to avoid this? And this will add extra overhead to // any document that fails to publish... var hasPublishedVersion = ApplicationContext.Current.Services.ContentService.HasPublishedVersion(content.Id); @@ -306,8 +309,8 @@ namespace Umbraco.Core.Publishing //NOTE: This previously always returned true so I've left it that way. It returned true because (from Morten)... // ... if one item couldn't be published it wouldn't be correct to return false. // in retrospect it should have returned a list of with Ids and Publish Status - // come to think of it ... the cache would still be updated for a failed item or at least tried updated. - // It would call the Published event for the entire list, but if the Published property isn't set to True it + // come to think of it ... the cache would still be updated for a failed item or at least tried updated. + // It would call the Published event for the entire list, but if the Published property isn't set to True it // wouldn't actually update the cache for that item. But not really ideal nevertheless... return true; } @@ -345,7 +348,7 @@ namespace Umbraco.Core.Publishing var evtMsgs = _eventMessagesFactory.Get(); //Fire UnPublishing event - if (UnPublishing.IsRaisedEventCancelled( + if (UnPublishing.IsRaisedEventCancelled( new PublishEventArgs(content, evtMsgs), this)) { _logger.Info( @@ -388,8 +391,8 @@ namespace Umbraco.Core.Publishing //NOTE: This previously always returned true so I've left it that way. It returned true because (from Morten)... // ... if one item couldn't be published it wouldn't be correct to return false. // in retrospect it should have returned a list of with Ids and Publish Status - // come to think of it ... the cache would still be updated for a failed item or at least tried updated. - // It would call the Published event for the entire list, but if the Published property isn't set to True it + // come to think of it ... the cache would still be updated for a failed item or at least tried updated. + // It would call the Published event for the entire list, but if the Published property isn't set to True it // wouldn't actually update the cache for that item. But not really ideal nevertheless... return true; } @@ -405,7 +408,7 @@ namespace Umbraco.Core.Publishing public override void PublishingFinalized(IContent content) { var evtMsgs = _eventMessagesFactory.Get(); - Published.RaiseEvent( + Published.RaiseEvent( new PublishEventArgs(content, false, false, evtMsgs), this); } diff --git a/src/umbraco.cms/businesslogic/web/Document.cs b/src/umbraco.cms/businesslogic/web/Document.cs index bd21517d80..3ed457d776 100644 --- a/src/umbraco.cms/businesslogic/web/Document.cs +++ b/src/umbraco.cms/businesslogic/web/Document.cs @@ -874,18 +874,15 @@ namespace umbraco.cms.businesslogic.web [Obsolete("Don't use! Only used internally to support the legacy events", false)] internal IEnumerable> PublishWithSubs(int userId, bool includeUnpublished) { - PublishEventArgs e = new PublishEventArgs(); + var e = new PublishEventArgs(); FireBeforePublish(e); - IEnumerable> publishedResults = Enumerable.Empty>(); + if (e.Cancel) return Enumerable.Empty>(); - if (!e.Cancel) - { - publishedResults = ApplicationContext.Current.Services.ContentService - .PublishWithChildrenWithStatus(ContentEntity, userId, includeUnpublished); + var publishedResults = ApplicationContext.Current.Services.ContentService + .PublishWithChildrenWithStatus(ContentEntity, userId, includeUnpublished); - FireAfterPublish(e); - } + FireAfterPublish(e); return publishedResults; } @@ -893,7 +890,6 @@ namespace umbraco.cms.businesslogic.web [Obsolete("Don't use! Only used internally to support the legacy events", false)] internal Attempt SaveAndPublish(int userId) { - var result = Attempt.Fail(new PublishStatus(ContentEntity, PublishStatusType.FailedCancelledByEvent, new EventMessages())); foreach (var property in GenericProperties) { ContentEntity.SetValue(property.PropertyType.Alias, property.Value); @@ -901,32 +897,28 @@ namespace umbraco.cms.businesslogic.web var saveArgs = new SaveEventArgs(); FireBeforeSave(saveArgs); + if (saveArgs.Cancel) return Attempt.Fail(new PublishStatus(ContentEntity, PublishStatusType.FailedCancelledByEvent, new EventMessages())); - if (!saveArgs.Cancel) - { - var publishArgs = new PublishEventArgs(); - FireBeforePublish(publishArgs); + var publishArgs = new PublishEventArgs(); + FireBeforePublish(publishArgs); + if (publishArgs.Cancel) return Attempt.Fail(new PublishStatus(ContentEntity, PublishStatusType.FailedCancelledByEvent, new EventMessages())); - if (!publishArgs.Cancel) - { - //NOTE: The 'false' parameter will cause the PublishingStrategy events to fire which will ensure that the cache is refreshed. - result = ApplicationContext.Current.Services.ContentService - .SaveAndPublishWithStatus(ContentEntity, userId); - base.VersionDate = ContentEntity.UpdateDate; - this.UpdateDate = ContentEntity.UpdateDate; + //NOTE: The 'false' parameter will cause the PublishingStrategy events to fire which will ensure that the cache is refreshed. + var result = ApplicationContext.Current.Services.ContentService + .SaveAndPublishWithStatus(ContentEntity, userId); + VersionDate = ContentEntity.UpdateDate; + UpdateDate = ContentEntity.UpdateDate; - //NOTE: This is just going to call the CMSNode Save which will launch into the CMSNode.BeforeSave and CMSNode.AfterSave evenths - // which actually do dick all and there's no point in even having them there but just in case for some insane reason someone - // has bound to those events, I suppose we'll need to keep this here. - base.Save(); + //NOTE: This is just going to call the CMSNode Save which will launch into the CMSNode.BeforeSave and CMSNode.AfterSave evenths + // which actually do dick all and there's no point in even having them there but just in case for some insane reason someone + // has bound to those events, I suppose we'll need to keep this here. + base.Save(); - //Launch the After Save event since we're doing 2 things in one operation: Saving and publishing. - FireAfterSave(saveArgs); + //Launch the After Save event since we're doing 2 things in one operation: Saving and publishing. + FireAfterSave(saveArgs); - //Now we need to fire the After publish event - FireAfterPublish(publishArgs); - } - } + //Now we need to fire the After publish event + FireAfterPublish(publishArgs); return result; } From 3b22c0cdd7774e6a6d99098b55ca49330a3dde56 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Mar 2017 11:06:06 +1100 Subject: [PATCH 10/12] Catches correct expected exception type, we'll log for other non expected errors. --- .../umbraco/preview/PreviewContent.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs index 44faec0868..9ba5f91bd3 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/preview/PreviewContent.cs @@ -202,13 +202,15 @@ namespace umbraco.presentation.preview { file.Delete(); } - catch (Exception ex) + catch (IOException) { // for *some* reason deleting the file can fail, // and it will work later on (long-lasting locks, etc), // so just ignore the exception - - //LogHelper.Error(string.Format("Couldn't delete preview set: {0} - User {1}", file.Name, userId), ex); + } + catch (Exception ex) + { + LogHelper.Error(string.Format("Couldn't delete preview set: {0} - User {1}", file.Name, userId), ex); } } From 6c1e271fa8f1298c408b489e751cf7b0e60c801d Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Mar 2017 11:27:46 +1100 Subject: [PATCH 11/12] Fixes styling and fixing error message quotes to make it more clear --- src/Umbraco.Web.UI/umbraco/dialogs/publish.aspx | 2 +- src/Umbraco.Web/WebServices/BulkPublishController.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web.UI/umbraco/dialogs/publish.aspx b/src/Umbraco.Web.UI/umbraco/dialogs/publish.aspx index e86108616d..d6b6936144 100644 --- a/src/Umbraco.Web.UI/umbraco/dialogs/publish.aspx +++ b/src/Umbraco.Web.UI/umbraco/dialogs/publish.aspx @@ -80,7 +80,7 @@
-
+
  • diff --git a/src/Umbraco.Web/WebServices/BulkPublishController.cs b/src/Umbraco.Web/WebServices/BulkPublishController.cs index cc9a153e07..e477db539a 100644 --- a/src/Umbraco.Web/WebServices/BulkPublishController.cs +++ b/src/Umbraco.Web/WebServices/BulkPublishController.cs @@ -46,16 +46,16 @@ namespace Umbraco.Web.WebServices /*var result = ((ContentService) Services.ContentService) .PublishWithChildrenInternal(content, UmbracoUser.Id, includeUnpublished) .ToArray();*/ - var result = doc.PublishWithSubs(UmbracoUser.Id, includeUnpublished); + var result = doc.PublishWithSubs(UmbracoUser.Id, includeUnpublished).ToArray(); return Json(new { success = result.All(x => x.Success), - message = GetMessageForStatuses(result.Select(x => x.Result), content) + message = GetMessageForStatuses(result.Select(x => x.Result).ToArray(), content) }); } } - private string GetMessageForStatuses(IEnumerable statuses, IContent doc) + private string GetMessageForStatuses(PublishStatus[] statuses, IContent doc) { //if all are successful then just say it was successful if (statuses.All(x => ((int) x.StatusType) < 10)) @@ -91,12 +91,12 @@ namespace Umbraco.Web.WebServices return "Cannot publish document with a status of " + status.StatusType; case PublishStatusType.FailedCancelledByEvent: return ui.Text("publish", "contentPublishedFailedByEvent", - string.Format("{0} ({1})", status.ContentItem.Name, status.ContentItem.Id), UmbracoUser); + string.Format("'{0}' ({1})", status.ContentItem.Name, status.ContentItem.Id), UmbracoUser); case PublishStatusType.FailedContentInvalid: return ui.Text("publish", "contentPublishedFailedInvalid", new []{ - string.Format("{0} ({1})", status.ContentItem.Name, status.ContentItem.Id), - string.Join(",", status.InvalidProperties.Select(x => x.Alias)) + string.Format("'{0}' ({1})", status.ContentItem.Name, status.ContentItem.Id), + string.Format("'{0}'", string.Join(", ", status.InvalidProperties.Select(x => x.Alias))) }, UmbracoUser); default: From e7a540e717124187c68e6c0deb52aaafa5479109 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 23 Mar 2017 12:15:14 +1100 Subject: [PATCH 12/12] Proper Exception handling --- .../PublishedContent/PublishedContentType.cs | 2 +- .../XmlPublishedCache/XmlPublishedContent.cs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs index d05960b08f..63164cf271 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentType.cs @@ -179,7 +179,7 @@ namespace Umbraco.Core.Models.PublishedContent } if (contentType == null) - throw new Exception(string.Format("ContentTypeService failed to find a {0} type with alias \"{1}\".", + throw new InvalidOperationException(string.Format("ContentTypeService failed to find a {0} type with alias \"{1}\".", itemType.ToString().ToLower(), alias)); return new PublishedContentType(contentType); diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs index 4e251d746e..1b0a428610 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlPublishedContent.cs @@ -438,14 +438,16 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache contentType = getPublishedContentType(PublishedItemType.Content, docTypeAlias); } - catch (Exception e) + catch (InvalidOperationException e) { - if (e.TargetSite.Name == "CreatePublishedContentType") - { - content.Instance.RefreshContentFromDatabase(); - throw new Exception(string.Format("The content cache failed to find a content type with alias \"{0}\". This usually indicates that the content cache is corrupt; the content cache has been rebuilt in an attempt to self-fix the issue.", docTypeAlias)); - } - throw; + content.Instance.RefreshContentFromDatabase(); + + + throw new InvalidOperationException( + string.Format("{0}. This usually indicates that the content cache is corrupt; the content cache has been rebuilt in an attempt to self-fix the issue.", + //keep the original message but don't use this as an inner exception because we want the above message to be displayed, if we use the inner exception + //we can keep the stack trace but the header message will be the original message and the one we really want to display will be hidden below the fold. + e.Message)); } properties = new Dictionary(StringComparer.OrdinalIgnoreCase);