From d4fbe9a48511cd6b613735374ce7811cbff19dba Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Tue, 30 May 2023 12:53:42 -0700 Subject: [PATCH 1/5] Handle null session values This change does the following to handle null values (including nullable structs): 1. Allow null values to be passed into ISessionKeySerializer.TrySerialize 2. Check if null (or if Nullable<>) and serialize it 3. Remove special casing of null values in the SessionState default serializer (BinarySessionSerializer), which also requires a bump in version 4. The BinarySessionSerializer is updated to understand reading v1 or v2 values --- Microsoft.AspNetCore.SystemWebAdapters.sln | 2 +- samples/CoreApp/Program.cs | 6 +- samples/CoreApp/SessionExampleExtensions.cs | 19 ++- .../Serialization/ISessionKeySerializer.cs | 2 +- .../SessionState/BinarySessionSerializer.cs | 31 +++-- .../CompositeSessionKeySerializer.cs | 2 +- .../SessionState/JsonSessionKeySerializer.cs | 13 +- .../BinarySessionSerializerTests.cs | 126 +++++++++++++++-- .../JsonSessionKeySerializerTests.cs | 131 +++++++++++++++++- .../Wrapped/AspNetCoreSessionState.cs | 2 +- ...WebAdapters.FrameworkServices.Tests.csproj | 7 + 11 files changed, 300 insertions(+), 41 deletions(-) diff --git a/Microsoft.AspNetCore.SystemWebAdapters.sln b/Microsoft.AspNetCore.SystemWebAdapters.sln index 65b30ecbab..03ec13a21b 100644 --- a/Microsoft.AspNetCore.SystemWebAdapters.sln +++ b/Microsoft.AspNetCore.SystemWebAdapters.sln @@ -71,7 +71,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ModulesLibrary", "samples\M EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ModulesFramework", "samples\Modules\ModulesFramework\ModulesFramework.csproj", "{B262AD69-11F0-4AE0-949A-AEAA2300C061}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ModulesCore", "samples\Modules\ModulesCore\ModulesCore.csproj", "{F8B33C59-27CF-45DC-955C-2EBF9DA9DB7E}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ModulesCore", "samples\Modules\ModulesCore\ModulesCore.csproj", "{F8B33C59-27CF-45DC-955C-2EBF9DA9DB7E}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/samples/CoreApp/Program.cs b/samples/CoreApp/Program.cs index ba1ff9c280..710cfdb9b7 100644 --- a/samples/CoreApp/Program.cs +++ b/samples/CoreApp/Program.cs @@ -3,11 +3,7 @@ builder.Services.AddSystemWebAdapters() .WrapAspNetCoreSession() .AddSessionSerializer() - .AddCustomSerialization() - .AddJsonSessionSerializer(options => - { - options.RegisterKey("callCount"); - }); + .AddCustomSerialization(); builder.Services.AddDistributedMemoryCache(); diff --git a/samples/CoreApp/SessionExampleExtensions.cs b/samples/CoreApp/SessionExampleExtensions.cs index e81a169a20..3aef3a2442 100644 --- a/samples/CoreApp/SessionExampleExtensions.cs +++ b/samples/CoreApp/SessionExampleExtensions.cs @@ -10,6 +10,12 @@ internal static class SessionExampleExtensions public static ISystemWebAdapterBuilder AddCustomSerialization(this ISystemWebAdapterBuilder builder) { + builder.AddJsonSessionSerializer(options => + { + options.RegisterKey("callCount"); + options.RegisterKey("item"); + }); + builder.Services.AddSingleton(new ByteArraySerializer(SessionKey)); return builder; } @@ -59,6 +65,17 @@ static void SetValue(HttpContext context, byte[] data) return $"This endpoint has been hit {count} time(s) this session"; }); + + + builder.MapGet("/item", (HttpContextCore ctx) => + { + var context = (HttpContext)ctx; + + var result = context.Session!["item"]; + context.Session!["item"] = default(int); + + return $"Current value: {result}"; + }); } /// @@ -88,7 +105,7 @@ public bool TryDeserialize(string key, byte[] bytes, out object? obj) return false; } - public bool TrySerialize(string key, object value, out byte[] bytes) + public bool TrySerialize(string key, object? value, out byte[] bytes) { if (string.Equals(_key, key, StringComparison.Ordinal) && value is byte[] valueBytes) { diff --git a/src/Microsoft.AspNetCore.SystemWebAdapters.Abstractions/SessionState/Serialization/ISessionKeySerializer.cs b/src/Microsoft.AspNetCore.SystemWebAdapters.Abstractions/SessionState/Serialization/ISessionKeySerializer.cs index a6dd4cd660..3ed2f8af17 100644 --- a/src/Microsoft.AspNetCore.SystemWebAdapters.Abstractions/SessionState/Serialization/ISessionKeySerializer.cs +++ b/src/Microsoft.AspNetCore.SystemWebAdapters.Abstractions/SessionState/Serialization/ISessionKeySerializer.cs @@ -12,7 +12,7 @@ public interface ISessionKeySerializer /// Object to serialize., /// Bytes if successful. /// true if successful. If key is unknown, false will be returned. - bool TrySerialize(string key, object value, out byte[] bytes); + bool TrySerialize(string key, object? value, out byte[] bytes); /// /// Deserializes a session object for a given key. diff --git a/src/Services/SessionState/BinarySessionSerializer.cs b/src/Services/SessionState/BinarySessionSerializer.cs index cdb36dbe53..c8c4d66585 100644 --- a/src/Services/SessionState/BinarySessionSerializer.cs +++ b/src/Services/SessionState/BinarySessionSerializer.cs @@ -16,7 +16,15 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization; [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "CA1510:Use ArgumentNullException throw helper", Justification = "Source shared with .NET Framework that does not have the method")] internal partial class BinarySessionSerializer : ISessionSerializer { - private const byte Version = 1; + // Used to ensure format is as expected: + /// + /// Magic number used to track which version of the binary session serializer was used. + /// + /// 1 - Initial release (v1.0) + /// 2 - Delegates the handling of null to the serializers rather than special casing them (v1.2) + /// + /// + internal const byte Version = 2; private readonly SessionSerializerOptions _options; private readonly ISessionKeySerializer _serializer; @@ -53,21 +61,14 @@ public void Write(ISessionState state, BinaryWriter writer) { writer.Write(item); - if (state[item] is { } obj) + if (_serializer.TrySerialize(item, state[item], out var result)) { - if (_serializer.TrySerialize(item, obj, out var result)) - { - writer.Write7BitEncodedInt(result.Length); - writer.Write(result); - } - else - { - (unknownKeys ??= new()).Add(item); - writer.Write7BitEncodedInt(0); - } + writer.Write7BitEncodedInt(result.Length); + writer.Write(result); } else { + (unknownKeys ??= new()).Add(item); writer.Write7BitEncodedInt(0); } } @@ -100,9 +101,11 @@ public ISessionState Read(BinaryReader reader) throw new ArgumentNullException(nameof(reader)); } - if (reader.ReadByte() != Version) + var version = reader.ReadByte(); + + if (version != 1 && version != 2) { - throw new InvalidOperationException("Serialized session state has different version than expected"); + throw new InvalidOperationException($"Serialized session state has unsupported version: {version}"); } var state = new BinaryReaderSerializedSessionState(reader, _serializer); diff --git a/src/Services/SessionState/CompositeSessionKeySerializer.cs b/src/Services/SessionState/CompositeSessionKeySerializer.cs index a98eb7b3f3..112ad107aa 100644 --- a/src/Services/SessionState/CompositeSessionKeySerializer.cs +++ b/src/Services/SessionState/CompositeSessionKeySerializer.cs @@ -13,7 +13,7 @@ public CompositeSessionKeySerializer(IEnumerable serializ _serializers = serializers.ToArray(); } - public bool TrySerialize(string key, object value, out byte[] bytes) + public bool TrySerialize(string key, object? value, out byte[] bytes) { foreach (var serializer in _serializers) { diff --git a/src/Services/SessionState/JsonSessionKeySerializer.cs b/src/Services/SessionState/JsonSessionKeySerializer.cs index 4688ea8faf..698914cf4f 100644 --- a/src/Services/SessionState/JsonSessionKeySerializer.cs +++ b/src/Services/SessionState/JsonSessionKeySerializer.cs @@ -45,11 +45,20 @@ public bool TryDeserialize(string key, byte[] bytes, out object? obj) return false; } - public bool TrySerialize(string key, object value, out byte[] bytes) + public bool TrySerialize(string key, object? value, out byte[] bytes) { if (_options.Value.KnownKeys.TryGetValue(key, out var type)) { - if (type == value.GetType()) + if (value is null) + { + if (!type.IsValueType || (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))) + { + // Create a new one instead of caching since technically the array values could be overwritten + bytes = "null"u8.ToArray(); + return true; + } + } + else if (type == value.GetType() || (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>) && type.GenericTypeArguments[0] == value.GetType())) { try { diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs index 26265d7b81..7e406d99d0 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs @@ -29,14 +29,14 @@ public async Task SerializeEmpty() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 0, 0 }); } [Fact] public async Task DeserializeEmpty() { // Arrange - var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 0, 0 }; + var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -69,14 +69,14 @@ public async Task SerializeIsNewSession() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 1, 0, 0, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 1, 0, 0, 0, 0, 0 }); } [Fact] public async Task DeserializeIsNewSession() { // Arrange - var data = new byte[] { 1, 2, 105, 100, 1, 0, 0, 0, 0, 0 }; + var data = new byte[] { 2, 2, 105, 100, 1, 0, 0, 0, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -109,14 +109,14 @@ public async Task SerializeIsAbandoned() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 1, 0, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 1, 0, 0, 0, 0 }); } [Fact] public async Task DeserializeIsAbandoned() { // Arrange - var data = new byte[] { 1, 2, 105, 100, 0, 1, 0, 0, 0, 0 }; + var data = new byte[] { 2, 2, 105, 100, 0, 1, 0, 0, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -149,11 +149,33 @@ public async Task SerializeIsReadOnly() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 1, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 1, 0, 0, 0 }); } [Fact] public async Task DeserializeIsReadOnly() + { + // Arrange + var data = new byte[] { 2, 2, 105, 100, 0, 0, 1, 0, 0, 0 }; + using var ms = new MemoryStream(data); + + var serializer = CreateSerializer(); + + // Act + var result = await serializer.DeserializeAsync(ms, default); + + // Assert + Assert.Equal("id", result!.SessionID); + Assert.True(result.IsReadOnly); + Assert.False(result.IsAbandoned); + Assert.False(result.IsNewSession); + Assert.Equal(0, result.Timeout); + Assert.Equal(0, result.Count); + Assert.Empty(result.Keys); + } + + [Fact] + public async Task DeserializeIsReadOnlyV1() { // Arrange var data = new byte[] { 1, 2, 105, 100, 0, 0, 1, 0, 0, 0 }; @@ -189,14 +211,14 @@ public async Task SerializeTimeout() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 20, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 20, 0, 0 }); } [Fact] public async Task DeserializeTimeout() { // Arrange - var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 20, 0, 0 }; + var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 20, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -236,7 +258,84 @@ public async Task Serialize1Key() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }); + } + + [Fact] + public async Task Serialize1KeyNull() + { + // Arrange + var obj = default(object); + var state = new Mock(); + state.Setup(s => s["key1"]).Returns(obj); + state.Setup(s => s.SessionID).Returns("id"); + state.Setup(s => s.Keys).Returns(new[] { "key1" }); + state.Setup(s => s.Count).Returns(1); + + var keySerializer = new Mock(); + var bytes = new byte[] { 0 }; + keySerializer.Setup(k => k.TrySerialize("key1", obj, out bytes)).Returns(true); + + var serializer = CreateSerializer(keySerializer.Object); + using var ms = new MemoryStream(); + + // Act + await serializer.SerializeAsync(state.Object, ms, default); + + // Assert + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }); + } + + [Fact] + public async Task Deserialize1KeyV1() + { + // Arrange + var obj = new object(); + var keySerializer = new Mock(); + keySerializer.Setup(k => k.TryDeserialize("key1", Array.Empty(), out obj)).Returns(true); + + var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 0, 0 }; + using var ms = new MemoryStream(data); + + var serializer = CreateSerializer(keySerializer.Object); + + // Act + var result = await serializer.DeserializeAsync(ms, default); + + // Assert + Assert.Equal("id", result!.SessionID); + Assert.False(result.IsReadOnly); + Assert.False(result.IsAbandoned); + Assert.False(result.IsNewSession); + Assert.Equal(0, result.Timeout); + Assert.Equal(1, result.Count); + Assert.Equal(result.Keys, new[] { "key1" }); + Assert.Equal(obj, result["key1"]); + } + + [Fact] + public async Task Serialize1KeyNullable() + { + // Arrange + var obj = (int?)5; + var state = new Mock(); + state.Setup(s => s["key1"]).Returns(obj); + state.Setup(s => s.SessionID).Returns("id"); + state.Setup(s => s.Keys).Returns(new[] { "key1" }); + state.Setup(s => s.Count).Returns(1); + + var keySerializer = new Mock(); + var bytes = new byte[] { 0 }; + keySerializer.Setup(k => k.TrySerialize("key1", obj, out bytes)).Returns(true); + + var serializer = CreateSerializer(keySerializer.Object); + using var ms = new MemoryStream(); + + // Act + await serializer.SerializeAsync(state.Object, ms, default); + + // Assert + Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }); } [Fact] @@ -248,7 +347,7 @@ public async Task Deserialize1Key() var keySerializer = new Mock(); keySerializer.Setup(k => k.TryDeserialize("key1", bytes, out obj)).Returns(true); - var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }; + var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(keySerializer.Object); @@ -272,8 +371,9 @@ private static BinarySessionSerializer CreateSerializer(ISessionKeySerializer? k keySerializer ??= new Mock().Object; var logger = new Mock>(); + var options = new SessionSerializerOptions(); var optionContainer = new Mock>(); - optionContainer.Setup(o => o.Value).Returns(new SessionSerializerOptions()); + optionContainer.Setup(o => o.Value).Returns(options); return new BinarySessionSerializer(new Composite(keySerializer), optionContainer.Object, logger.Object); } @@ -290,7 +390,7 @@ public Composite(ISessionKeySerializer serializer) public bool TryDeserialize(string key, byte[] bytes, out object? obj) => _serializer.TryDeserialize(key, bytes, out obj); - public bool TrySerialize(string key, object value, out byte[] bytes) + public bool TrySerialize(string key, object? value, out byte[] bytes) => _serializer.TrySerialize(key, value, out bytes); } } diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/JsonSessionKeySerializerTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/JsonSessionKeySerializerTests.cs index 5bb29d7d18..d27788c22a 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/JsonSessionKeySerializerTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/JsonSessionKeySerializerTests.cs @@ -1,7 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Globalization; using System.Linq; +using System.Text; using Autofac.Extras.Moq; using AutoFixture; using Microsoft.Extensions.Options; @@ -83,7 +85,37 @@ public void SerializeTypeRegistered() // Assert Assert.True(result); - Assert.Equal(new byte[] { 123, 125 }, bytes); + Assert.Equal("{}"u8.ToArray(), bytes); + } + + [Fact] + public void HandleNullValues() + { + // Arrange + using var mock = AutoMock.GetLoose(); + var key = _fixture.Create(); + var value = _fixture.CreateMany().ToArray(); + + var options = new JsonSessionSerializerOptions + { + KnownKeys = + { + { key, typeof(Type1) }, + } + }; + mock.Mock>().Setup(o => o.Value).Returns(options); + + var serializer = mock.Create(); + + // Act + var serializeResult = serializer.TrySerialize(key, null, out var bytes); + var deserializeResult = serializer.TryDeserialize(key, bytes, out var deserialized); + + // Assert + Assert.True(serializeResult); + Assert.True(deserializeResult); + Assert.Equal("null"u8.ToArray(), bytes); + Assert.Null(deserialized); } [Fact] @@ -119,7 +151,7 @@ public void DeserializeTypeRegistered() // Arrange using var mock = AutoMock.GetLoose(); var key = _fixture.Create(); - var value = new byte[] { 123, 125 }; + var value = "{}"u8.ToArray(); var options = new JsonSessionSerializerOptions { @@ -166,6 +198,101 @@ public void SerializeIncorrectTypeRegistered() Assert.Empty(bytes); } + [InlineData(0)] + [InlineData(-1)] + [InlineData(1)] + [InlineData(100)] + [Theory] + public void PrimitiveSerializer(int primitive) + { + // Arrange + using var mock = AutoMock.GetLoose(); + var key = _fixture.Create(); + var value = _fixture.CreateMany().ToArray(); + + var options = new JsonSessionSerializerOptions + { + KnownKeys = + { + { key, typeof(int) }, + } + }; + mock.Mock>().Setup(o => o.Value).Returns(options); + + var serializer = mock.Create(); + + // Act + var serializeResult = serializer.TrySerialize(key, primitive, out var bytes); + var deserializeResult = serializer.TryDeserialize(key, bytes, out var deserialized); + + // Assert + Assert.True(serializeResult); + Assert.Equal(Encoding.UTF8.GetBytes(primitive.ToString(CultureInfo.InvariantCulture)), bytes); + Assert.True(deserializeResult); + Assert.Equal(primitive, deserialized); + } + + [Fact] + public void PrimitiveNullableNotSupportedSerializer() + { + // Arrange + using var mock = AutoMock.GetLoose(); + var key = _fixture.Create(); + var value = _fixture.CreateMany().ToArray(); + + var options = new JsonSessionSerializerOptions + { + KnownKeys = + { + { key, typeof(int) }, + } + }; + mock.Mock>().Setup(o => o.Value).Returns(options); + + var serializer = mock.Create(); + + // Act + var result = serializer.TrySerialize(key, default, out var bytes); + + // Assert + Assert.False(result); + } + + [InlineData(null)] + [InlineData(0)] + [InlineData(-1)] + [InlineData(1)] + [InlineData(100)] + [Theory] + public void PrimitiveNullableSerializer(int? primitive) + { + // Arrange + using var mock = AutoMock.GetLoose(); + var key = _fixture.Create(); + var value = _fixture.CreateMany().ToArray(); + + var options = new JsonSessionSerializerOptions + { + KnownKeys = + { + { key, typeof(int?) }, + } + }; + mock.Mock>().Setup(o => o.Value).Returns(options); + + var serializer = mock.Create(); + + // Act + var serializeResult = serializer.TrySerialize(key, primitive, out var bytes); + var deserializeResult = serializer.TryDeserialize(key, bytes, out var deserialized); + + // Assert + Assert.True(serializeResult); + Assert.Equal(Encoding.UTF8.GetBytes(primitive.HasValue ? primitive.Value.ToString(CultureInfo.InvariantCulture) : "null"), bytes); + Assert.True(deserializeResult); + Assert.Equal(primitive, deserialized); + } + private sealed class Type1 { } diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Wrapped/AspNetCoreSessionState.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Wrapped/AspNetCoreSessionState.cs index c45373ef21..2f7d3f06fc 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Wrapped/AspNetCoreSessionState.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Wrapped/AspNetCoreSessionState.cs @@ -301,7 +301,7 @@ public Composite(ISessionKeySerializer serializer) public bool TryDeserialize(string key, byte[] bytes, out object? obj) => _serializer.TryDeserialize(key, bytes, out obj); - public bool TrySerialize(string key, object value, out byte[] bytes) + public bool TrySerialize(string key, object? value, out byte[] bytes) => _serializer.TrySerialize(key, value, out bytes); } } diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj index 88e772e34f..f96152febe 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj @@ -3,6 +3,10 @@ net472 Microsoft.AspNetCore.SystemWebAdapters.Tests + + + + @@ -17,4 +21,7 @@ + + + \ No newline at end of file From a81d04213bc2810f25a8cb5bc846083c5f72e5c6 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Tue, 30 May 2023 14:02:53 -0700 Subject: [PATCH 2/5] simplify checks --- src/Services/SessionState/JsonSessionKeySerializer.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Services/SessionState/JsonSessionKeySerializer.cs b/src/Services/SessionState/JsonSessionKeySerializer.cs index 698914cf4f..ae4437ddb0 100644 --- a/src/Services/SessionState/JsonSessionKeySerializer.cs +++ b/src/Services/SessionState/JsonSessionKeySerializer.cs @@ -51,14 +51,14 @@ public bool TrySerialize(string key, object? value, out byte[] bytes) { if (value is null) { - if (!type.IsValueType || (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))) + if (!type.IsValueType || IsNullable(type)) { // Create a new one instead of caching since technically the array values could be overwritten bytes = "null"u8.ToArray(); return true; } } - else if (type == value.GetType() || (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>) && type.GenericTypeArguments[0] == value.GetType())) + else if (type == value.GetType() || IsNullableType(type, value.GetType())) { try { @@ -79,5 +79,11 @@ public bool TrySerialize(string key, object? value, out byte[] bytes) bytes = Array.Empty(); return false; } + + private static bool IsNullable(Type type) + => type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>); + + private static bool IsNullableType(Type type, Type nullableArg) + => IsNullable(type) && nullableArg == type.GenericTypeArguments[0]; } From cfc17ad1139d9ede5fa2cfd4a576c6151e60a803 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Tue, 30 May 2023 14:06:37 -0700 Subject: [PATCH 3/5] extra whitespace --- src/Services/SessionState/JsonSessionKeySerializer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Services/SessionState/JsonSessionKeySerializer.cs b/src/Services/SessionState/JsonSessionKeySerializer.cs index ae4437ddb0..f96f8265b2 100644 --- a/src/Services/SessionState/JsonSessionKeySerializer.cs +++ b/src/Services/SessionState/JsonSessionKeySerializer.cs @@ -22,7 +22,6 @@ public JsonSessionKeySerializer(IOptions options, [LoggerMessage(0, LogLevel.Error, "Unexpected JSON serialize/deserialization error for '{Key}' expected type '{Type}'")] private partial void LogException(Exception e, string key, string type); - [LoggerMessage(1, LogLevel.Warning, "Session key '{Key}' is registered as {RegisteredType} but was actually {FoundType}")] private partial void UnexpectedType(string key, Type registeredType, Type foundType); From a5b2ff46f8abc88d8010966583f1765fd818bd76 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Tue, 30 May 2023 14:45:44 -0700 Subject: [PATCH 4/5] add tests --- .../SessionState/BinarySessionSerializer.cs | 5 +--- .../BinarySessionSerializerTests.cs | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/Services/SessionState/BinarySessionSerializer.cs b/src/Services/SessionState/BinarySessionSerializer.cs index c8c4d66585..7c478d225c 100644 --- a/src/Services/SessionState/BinarySessionSerializer.cs +++ b/src/Services/SessionState/BinarySessionSerializer.cs @@ -162,10 +162,7 @@ public BinaryReaderSerializedSessionState(BinaryReader reader, ISessionKeySerial if (serializer.TryDeserialize(key, bytes, out var result)) { - if (result is not null) - { - this[key] = result; - } + this[key] = result; } else { diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs index 7e406d99d0..53652abf52 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs @@ -286,6 +286,33 @@ public async Task Serialize1KeyNull() Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }); } + [Fact] + public async Task Deserialize1KeyNull() + { + // Arrange + var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }; + var obj = new object(); + var value = new byte[] { 0 }; + + var keySerializer = new Mock(); + keySerializer.Setup(k => k.TryDeserialize("key1", value, out obj)).Returns(true); + + var serializer = CreateSerializer(keySerializer.Object); + + // Act + var result = await serializer.DeserializeAsync(new MemoryStream(data), default); + + // Assert + Assert.Equal("id", result!.SessionID); + Assert.False(result.IsReadOnly); + Assert.False(result.IsAbandoned); + Assert.False(result.IsNewSession); + Assert.Equal(0, result.Timeout); + Assert.Equal(1, result.Count); + Assert.Same(obj, result["key1"]); + Assert.Collection(result.Keys, k => Assert.Equal("key1", k)); + } + [Fact] public async Task Deserialize1KeyV1() { From 4514463cecbfa1a60ab9e17e0d3078b66a4e05aa Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 2 Jun 2023 11:28:49 -0700 Subject: [PATCH 5/5] revert version change --- Microsoft.AspNetCore.SystemWebAdapters.sln | 2 +- .../SessionState/BinarySessionSerializer.cs | 39 ++++++++----------- .../BinarySessionSerializerTests.cs | 37 ++++++++---------- ...WebAdapters.FrameworkServices.Tests.csproj | 3 -- 4 files changed, 34 insertions(+), 47 deletions(-) diff --git a/Microsoft.AspNetCore.SystemWebAdapters.sln b/Microsoft.AspNetCore.SystemWebAdapters.sln index 03ec13a21b..65b30ecbab 100644 --- a/Microsoft.AspNetCore.SystemWebAdapters.sln +++ b/Microsoft.AspNetCore.SystemWebAdapters.sln @@ -71,7 +71,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ModulesLibrary", "samples\M EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ModulesFramework", "samples\Modules\ModulesFramework\ModulesFramework.csproj", "{B262AD69-11F0-4AE0-949A-AEAA2300C061}" EndProject -Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ModulesCore", "samples\Modules\ModulesCore\ModulesCore.csproj", "{F8B33C59-27CF-45DC-955C-2EBF9DA9DB7E}" +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ModulesCore", "samples\Modules\ModulesCore\ModulesCore.csproj", "{F8B33C59-27CF-45DC-955C-2EBF9DA9DB7E}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/src/Services/SessionState/BinarySessionSerializer.cs b/src/Services/SessionState/BinarySessionSerializer.cs index 7c478d225c..2d7481075e 100644 --- a/src/Services/SessionState/BinarySessionSerializer.cs +++ b/src/Services/SessionState/BinarySessionSerializer.cs @@ -16,15 +16,7 @@ namespace Microsoft.AspNetCore.SystemWebAdapters.SessionState.Serialization; [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "CA1510:Use ArgumentNullException throw helper", Justification = "Source shared with .NET Framework that does not have the method")] internal partial class BinarySessionSerializer : ISessionSerializer { - // Used to ensure format is as expected: - /// - /// Magic number used to track which version of the binary session serializer was used. - /// - /// 1 - Initial release (v1.0) - /// 2 - Delegates the handling of null to the serializers rather than special casing them (v1.2) - /// - /// - internal const byte Version = 2; + private const byte Version = 1; private readonly SessionSerializerOptions _options; private readonly ISessionKeySerializer _serializer; @@ -62,16 +54,16 @@ public void Write(ISessionState state, BinaryWriter writer) writer.Write(item); if (_serializer.TrySerialize(item, state[item], out var result)) - { - writer.Write7BitEncodedInt(result.Length); - writer.Write(result); - } - else - { - (unknownKeys ??= new()).Add(item); - writer.Write7BitEncodedInt(0); + { + writer.Write7BitEncodedInt(result.Length); + writer.Write(result); + } + else + { + (unknownKeys ??= new()).Add(item); + writer.Write7BitEncodedInt(0); + } } - } if (unknownKeys is null) { @@ -101,11 +93,9 @@ public ISessionState Read(BinaryReader reader) throw new ArgumentNullException(nameof(reader)); } - var version = reader.ReadByte(); - - if (version != 1 && version != 2) + if (reader.ReadByte() != Version) { - throw new InvalidOperationException($"Serialized session state has unsupported version: {version}"); + throw new InvalidOperationException("Serialized session state has different version than expected"); } var state = new BinaryReaderSerializedSessionState(reader, _serializer); @@ -162,7 +152,10 @@ public BinaryReaderSerializedSessionState(BinaryReader reader, ISessionKeySerial if (serializer.TryDeserialize(key, bytes, out var result)) { - this[key] = result; + if (result is not null) + { + this[key] = result; + } } else { diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs index 53652abf52..a370522a4b 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SessionState/Serialization/BinarySessionSerializerTests.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.IO; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -29,14 +27,14 @@ public async Task SerializeEmpty() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 0, 0 }); } [Fact] public async Task DeserializeEmpty() { // Arrange - var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 0, 0 }; + var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -69,14 +67,14 @@ public async Task SerializeIsNewSession() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 1, 0, 0, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 1, 0, 0, 0, 0, 0 }); } [Fact] public async Task DeserializeIsNewSession() { // Arrange - var data = new byte[] { 2, 2, 105, 100, 1, 0, 0, 0, 0, 0 }; + var data = new byte[] { 1, 2, 105, 100, 1, 0, 0, 0, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -109,14 +107,14 @@ public async Task SerializeIsAbandoned() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 1, 0, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 1, 0, 0, 0, 0 }); } [Fact] public async Task DeserializeIsAbandoned() { // Arrange - var data = new byte[] { 2, 2, 105, 100, 0, 1, 0, 0, 0, 0 }; + var data = new byte[] { 1, 2, 105, 100, 0, 1, 0, 0, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -149,14 +147,14 @@ public async Task SerializeIsReadOnly() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 1, 0, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 1, 0, 0, 0 }); } [Fact] public async Task DeserializeIsReadOnly() { // Arrange - var data = new byte[] { 2, 2, 105, 100, 0, 0, 1, 0, 0, 0 }; + var data = new byte[] { 1, 2, 105, 100, 0, 0, 1, 0, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -175,7 +173,7 @@ public async Task DeserializeIsReadOnly() } [Fact] - public async Task DeserializeIsReadOnlyV1() + public async Task DeserializeIsReadOnlyEmptyNull() { // Arrange var data = new byte[] { 1, 2, 105, 100, 0, 0, 1, 0, 0, 0 }; @@ -211,14 +209,14 @@ public async Task SerializeTimeout() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 20, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 20, 0, 0 }); } [Fact] public async Task DeserializeTimeout() { // Arrange - var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 20, 0, 0 }; + var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 20, 0, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(); @@ -258,7 +256,7 @@ public async Task Serialize1Key() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }); } [Fact] @@ -283,14 +281,14 @@ public async Task Serialize1KeyNull() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }); } [Fact] public async Task Deserialize1KeyNull() { // Arrange - var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }; + var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }; var obj = new object(); var value = new byte[] { 0 }; @@ -362,7 +360,7 @@ public async Task Serialize1KeyNullable() await serializer.SerializeAsync(state.Object, ms, default); // Assert - Assert.Equal(ms.ToArray(), new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }); + Assert.Equal(ms.ToArray(), new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 0, 0 }); } [Fact] @@ -374,7 +372,7 @@ public async Task Deserialize1Key() var keySerializer = new Mock(); keySerializer.Setup(k => k.TryDeserialize("key1", bytes, out obj)).Returns(true); - var data = new byte[] { 2, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }; + var data = new byte[] { 1, 2, 105, 100, 0, 0, 0, 0, 1, 4, 107, 101, 121, 49, 1, 42, 0 }; using var ms = new MemoryStream(data); var serializer = CreateSerializer(keySerializer.Object); @@ -398,9 +396,8 @@ private static BinarySessionSerializer CreateSerializer(ISessionKeySerializer? k keySerializer ??= new Mock().Object; var logger = new Mock>(); - var options = new SessionSerializerOptions(); var optionContainer = new Mock>(); - optionContainer.Setup(o => o.Value).Returns(options); + optionContainer.Setup(o => o.Value).Returns(new SessionSerializerOptions()); return new BinarySessionSerializer(new Composite(keySerializer), optionContainer.Object, logger.Object); } diff --git a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj index f96152febe..22df7ab7fc 100644 --- a/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj +++ b/test/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices.Tests.csproj @@ -21,7 +21,4 @@ - - - \ No newline at end of file