From 55200b708081b878f5d9aa2fe24e8b94b6ef8fb8 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 27 May 2020 12:44:22 -0300 Subject: [PATCH 1/7] prevent so with ResourceInclude and StyleInclude. --- .../Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs | 4 +++- src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs index 87e2ba3a2e..5f5c0211b8 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs @@ -13,6 +13,7 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions { private Uri? _baseUri; private IResourceDictionary? _loaded; + private bool _isLoading; /// /// Gets the loaded resource dictionary. @@ -21,8 +22,9 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions { get { - if (_loaded == null) + if (!_isLoading) { + _isLoading = true; var loader = new AvaloniaXamlLoader(); _loaded = (IResourceDictionary)loader.Load(Source, _baseUri); } diff --git a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs index 2b39263ee9..6bffaaa804 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs @@ -14,6 +14,7 @@ namespace Avalonia.Markup.Xaml.Styling { private readonly Uri _baseUri; private IStyle[]? _loaded; + private bool _isLoading; /// /// Initializes a new instance of the class. @@ -47,8 +48,10 @@ namespace Avalonia.Markup.Xaml.Styling { get { - if (_loaded == null) + if (!_isLoading) { + _isLoading = true; + var loader = new AvaloniaXamlLoader(); var loaded = (IStyle)loader.Load(Source, _baseUri); _loaded = new[] { loaded }; From 98591e7574b7da56a79b356eebea1b262686f583 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 27 May 2020 18:40:56 -0300 Subject: [PATCH 2/7] remove workaround. --- .../MarkupExtensions/ResourceInclude.cs | 8 +++----- src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs | 7 ++----- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs index 5f5c0211b8..2e8fd49702 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs @@ -12,8 +12,7 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions public class ResourceInclude : IResourceProvider { private Uri? _baseUri; - private IResourceDictionary? _loaded; - private bool _isLoading; + private IResourceDictionary? _loaded; /// /// Gets the loaded resource dictionary. @@ -22,9 +21,8 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions { get { - if (!_isLoading) - { - _isLoading = true; + if (_loaded is null) + { var loader = new AvaloniaXamlLoader(); _loaded = (IResourceDictionary)loader.Load(Source, _baseUri); } diff --git a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs index 6bffaaa804..00320ce4e9 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs @@ -13,8 +13,7 @@ namespace Avalonia.Markup.Xaml.Styling public class StyleInclude : IStyle, IResourceProvider { private readonly Uri _baseUri; - private IStyle[]? _loaded; - private bool _isLoading; + private IStyle[]? _loaded; /// /// Initializes a new instance of the class. @@ -48,10 +47,8 @@ namespace Avalonia.Markup.Xaml.Styling { get { - if (!_isLoading) + if (_loaded is null) { - _isLoading = true; - var loader = new AvaloniaXamlLoader(); var loaded = (IStyle)loader.Load(Source, _baseUri); _loaded = new[] { loaded }; From 455c8998766cb6c5806e095b965bcf12d785aaf5 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 27 May 2020 18:43:24 -0300 Subject: [PATCH 3/7] Add failing unit tests. --- .../MarkupExtensions/ResourceIncludeTests.cs | 30 +++++++++++++ .../StyleIncludeTests.cs | 44 +++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs index 7ab6c2de40..e827627938 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs @@ -44,6 +44,36 @@ namespace Avalonia.Markup.Xaml.UnitTests.MakrupExtensions } } + [Fact] + public void Missing_ResourceKey_In_ResourceInclude_Does_Not_Cause_StackOverflow() + { + var styleXaml = @" + + +"; + + using (StartWithResources(("test:style.xaml", styleXaml))) + { + var xaml = @" + + + + + + + + +"; + + var loader = new AvaloniaXamlLoader(); + var app = Application.Current; + + loader.Load(xaml, null, app); + } + } + private IDisposable StartWithResources(params (string, string)[] assets) { var assetLoader = new MockAssetLoader(assets); diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs new file mode 100644 index 0000000000..f4e96942a3 --- /dev/null +++ b/tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs @@ -0,0 +1,44 @@ +using System; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Markup.Xaml.UnitTests +{ + public class StyleIncludeTests : XamlTestBase + { + [Fact] + public void Missing_ResourceKey_In_StyleInclude_Does_Not_Cause_StackOverflow() + { + var styleXaml = @" +"; + + using (StartWithResources(("test:style.xaml", styleXaml))) + { + var xaml = @" + + + + +"; + + var loader = new AvaloniaXamlLoader(); + var app = Application.Current; + + loader.Load(xaml, null, app); + } + } + + private IDisposable StartWithResources(params (string, string)[] assets) + { + var assetLoader = new MockAssetLoader(assets); + var services = new TestServices(assetLoader: assetLoader); + return UnitTestApplication.Start(services); + } + } +} From 8f396d0570165066194b5f13fb3e42e705fa6c4d Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 27 May 2020 18:49:33 -0300 Subject: [PATCH 4/7] add fix for StackOverflow in resource loading. --- .../MarkupExtensions/ResourceInclude.cs | 15 ++++++++++++--- .../Avalonia.Markup.Xaml/Styling/StyleInclude.cs | 7 +++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs index 2e8fd49702..ce9e3fd4b1 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs @@ -12,7 +12,8 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions public class ResourceInclude : IResourceProvider { private Uri? _baseUri; - private IResourceDictionary? _loaded; + private IResourceDictionary? _loaded; + private bool _isLoading; /// /// Gets the loaded resource dictionary. @@ -22,9 +23,11 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions get { if (_loaded is null) - { + { + _isLoading = true; var loader = new AvaloniaXamlLoader(); _loaded = (IResourceDictionary)loader.Load(Source, _baseUri); + _isLoading = false; } return _loaded; @@ -48,7 +51,13 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions bool IResourceNode.TryGetResource(object key, out object? value) { - return Loaded.TryGetResource(key, out value); + if(!_isLoading) + { + return Loaded.TryGetResource(key, out value); + } + + value = null; + return false; } void IResourceProvider.AddOwner(IResourceHost owner) => Loaded.AddOwner(owner); diff --git a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs index 00320ce4e9..9d5b56ebb4 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs @@ -13,7 +13,8 @@ namespace Avalonia.Markup.Xaml.Styling public class StyleInclude : IStyle, IResourceProvider { private readonly Uri _baseUri; - private IStyle[]? _loaded; + private IStyle[]? _loaded; + private bool _isLoading; /// /// Initializes a new instance of the class. @@ -49,9 +50,11 @@ namespace Avalonia.Markup.Xaml.Styling { if (_loaded is null) { + _isLoading = true; var loader = new AvaloniaXamlLoader(); var loaded = (IStyle)loader.Load(Source, _baseUri); _loaded = new[] { loaded }; + _isLoading = false; } return _loaded?[0]!; @@ -84,7 +87,7 @@ namespace Avalonia.Markup.Xaml.Styling public bool TryGetResource(object key, out object? value) { - if (Loaded is IResourceProvider p) + if (!_isLoading && Loaded is IResourceProvider p) { return p.TryGetResource(key, out value); } From ed375b578008586617ec5380e9fc5dd03d6f4b79 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 27 May 2020 18:53:49 -0300 Subject: [PATCH 5/7] Allow tests to pass as we expect a KeyNotFound exceptions. --- .../MarkupExtensions/ResourceIncludeTests.cs | 10 +++++++++- .../StyleIncludeTests.cs | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs index e827627938..1514d5bd95 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/ResourceIncludeTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Avalonia.Controls; using Avalonia.Media; using Avalonia.UnitTests; @@ -70,7 +71,14 @@ namespace Avalonia.Markup.Xaml.UnitTests.MakrupExtensions var loader = new AvaloniaXamlLoader(); var app = Application.Current; - loader.Load(xaml, null, app); + try + { + loader.Load(xaml, null, app); + } + catch (KeyNotFoundException) + { + + } } } diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs index f4e96942a3..dca936d229 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/StyleIncludeTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Avalonia.UnitTests; using Xunit; @@ -30,7 +31,14 @@ namespace Avalonia.Markup.Xaml.UnitTests var loader = new AvaloniaXamlLoader(); var app = Application.Current; - loader.Load(xaml, null, app); + try + { + loader.Load(xaml, null, app); + } + catch (KeyNotFoundException) + { + + } } } From 46151fd35bd36860f4d2d0176ffea576e3caaf7c Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Wed, 27 May 2020 18:55:25 -0300 Subject: [PATCH 6/7] remove unintended change. --- .../Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs | 2 +- src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs index ce9e3fd4b1..1627053e67 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs @@ -22,7 +22,7 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions { get { - if (_loaded is null) + if (_loaded == null) { _isLoading = true; var loader = new AvaloniaXamlLoader(); diff --git a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs index 9d5b56ebb4..ea9042f779 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Styling/StyleInclude.cs @@ -48,7 +48,7 @@ namespace Avalonia.Markup.Xaml.Styling { get { - if (_loaded is null) + if (_loaded == null) { _isLoading = true; var loader = new AvaloniaXamlLoader(); From ab6ff87a73b9d707176de8f15d5cd81cf2f5a70c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 28 May 2020 12:16:09 +0200 Subject: [PATCH 7/7] Fix spacing. --- .../Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs index 1627053e67..0cedf4f364 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/ResourceInclude.cs @@ -51,7 +51,7 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions bool IResourceNode.TryGetResource(object key, out object? value) { - if(!_isLoading) + if (!_isLoading) { return Loaded.TryGetResource(key, out value); }