From 4ae0a834d81a9ee580c29ee627eb96f691fb7037 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 17 Aug 2022 15:02:47 -0700 Subject: [PATCH 1/2] drop .NET Framework 4.5.2/4.6.1 and don't test in EOL platforms --- .circleci/config.yml | 53 +++++++++++-------- .ldrelease/config.yml | 8 +-- CONTRIBUTING.md | 4 +- README.md | 5 +- .../LaunchDarkly.EventSource.csproj | 4 +- .../LaunchDarkly.EventSource.Tests.csproj | 14 ++--- 6 files changed, 51 insertions(+), 37 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 722a64e..46df88c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,27 +7,20 @@ workflows: version: 2 test: jobs: - - test-netcore: - name: .NET Core 2.1 - docker-image: mcr.microsoft.com/dotnet/core/sdk:2.1-focal - test-target-framework: netcoreapp2.1 - - test-netcore: + - test_linux: name: .NET Core 3.1 docker-image: mcr.microsoft.com/dotnet/core/sdk:3.1-focal test-target-framework: netcoreapp3.1 - - test-netcore: - name: .NET 5.0 - docker-image: mcr.microsoft.com/dotnet/sdk:5.0-focal - test-target-framework: net5.0 - - test-windows-netframework: - name: .NET Framework 4.5.2 - test-target-framework: net452 - - test-windows-netframework: - name: .NET Framework 4.6.1 - test-target-framework: net461 + - test_linux: + name: .NET 6.0 + docker-image: mcr.microsoft.com/dotnet/sdk:6.0-focal + test-target-framework: net6.0 + - test_windows: + name: .NET Framework 4.6.2 + test-target-framework: net462 jobs: - test-netcore: + test_linux: parameters: docker-image: type: string @@ -43,10 +36,19 @@ jobs: name: install packages command: apt-get -q update && apt-get install -qy awscli - checkout - - run: dotnet build src/LaunchDarkly.EventSource -f netstandard2.0 - - run: dotnet test test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj + - run: + name: build + command: dotnet build src/LaunchDarkly.EventSource -f netstandard2.0 + - run: + name: run tests + command: | + dotnet test \ + -l "junit;LogFilePath=/tmp/circle-reports/unit-tests-commonsdk.xml" \ + test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj + - store_test_results: + path: /tmp/circle-reports - test-windows-netframework: + test_windows: parameters: test-target-framework: type: string @@ -57,5 +59,14 @@ jobs: TESTFRAMEWORK: <> steps: - checkout - - run: dotnet build src/LaunchDarkly.EventSource - - run: dotnet test test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj + - run: + name: build + command: dotnet build src/LaunchDarkly.EventSource + - run: + name: run tests + command: | + dotnet test \ + -l "junit;LogFilePath=/tmp/circle-reports/unit-tests-commonsdk.xml" \ + test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj + - store_test_results: + path: /tmp/circle-reports diff --git a/.ldrelease/config.yml b/.ldrelease/config.yml index b2d578d..a58dc66 100644 --- a/.ldrelease/config.yml +++ b/.ldrelease/config.yml @@ -6,16 +6,18 @@ publications: branches: - name: main - description: 4.x - for .NET SDK 6.0+ and Xamarin SDK 2.0+ + description: 5.x - for server-side .NET SDK 7.0+ and client-side .NET SDK 3.0+ + - name: 4.x + description: for .NET SDK 6.0+ and Xamarin SDK 2.0+ - name: 3.x description: for earlier .NET and Xamarin SDKs jobs: - template: - name: dotnet-linux + name: dotnet6-linux env: LD_RELEASE_DOCS_TARGET_FRAMEWORK: netstandard2.0 - LD_RELEASE_TEST_TARGET_FRAMEWORK: net5.0 + LD_RELEASE_TEST_TARGET_FRAMEWORK: net6.0 documentation: gitHubPages: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 113d4a1..3c8adc3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,10 +44,10 @@ To run all unit tests, for all targets: dotnet test test/LaunchDarkly.EventSource.Tests ``` -Or, to run tests only for the .NET Standard 2.0 target (using the .NET Core 2.1 runtime): +Or, to run tests only for the .NET Standard 2.0 target (using the .NET Core 3.1 runtime): ``` -dotnet test test/LaunchDarkly.EventSource.Tests -f netcoreapp2.1 +dotnet test test/LaunchDarkly.EventSource.Tests -f netcoreapp3.1 ``` Note that the unit tests can only be run in Debug configuration. There is an `InternalsVisibleTo` directive that allows the test code to access internal members of the library, and assembly strong-naming in the Release configuration interferes with this. diff --git a/README.md b/README.md index 06ce2f8..4ff1a13 100644 --- a/README.md +++ b/README.md @@ -12,9 +12,8 @@ The `LaunchDarkly.EventSource` package allows .NET developers to consume Server- This version of the library is built for the following targets: -* .NET Framework 4.5.2: runs on .NET Framework 4.5.x and above. -* .NET Core 2.1: runs on .NET Core 2.x and 3.x, or .NET 5. This target provides an adapter to the standard .NET Core logging framework, `Logs.CoreLogging`, which is not available in .NET Framework. -* .NET Standard 2.0: runs on .NET Core 2.x and 3.x or .NET 5, or within a library that is targeted to .NET Standard 2.x. +* .NET Framework 4.6.2: runs on .NET Framework 4.6.2 and above. +* .NET Standard 2.0: runs on .NET Core 3.x or .NET 6.0+, or within a library that is targeted to .NET Standard 2.x. The .NET build tools should automatically load the most appropriate build of the library for whatever platform your application or library is targeted to. diff --git a/src/LaunchDarkly.EventSource/LaunchDarkly.EventSource.csproj b/src/LaunchDarkly.EventSource/LaunchDarkly.EventSource.csproj index d2bcaa8..5485832 100644 --- a/src/LaunchDarkly.EventSource/LaunchDarkly.EventSource.csproj +++ b/src/LaunchDarkly.EventSource/LaunchDarkly.EventSource.csproj @@ -1,7 +1,7 @@  4.2.0 - netstandard2.0;net452 + netstandard2.0;net462 Apache-2.0 LaunchDarkly.EventSource portable @@ -21,7 +21,7 @@ - + diff --git a/test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj b/test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj index 8b06953..193d77d 100644 --- a/test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj +++ b/test/LaunchDarkly.EventSource.Tests/LaunchDarkly.EventSource.Tests.csproj @@ -4,20 +4,22 @@ single framework that we are testing; this allows us to test with older SDK versions that would error out if they saw any newer target frameworks listed here, even if we weren't running those. --> - netcoreapp2.1;net452;net5.0 + netcoreapp3.1;net462;net6.0 $(TESTFRAMEWORK) Copyright © 2017 Catamorphic, Co. - - - - + + + + + + - + From bdbce68feb293f4d79f300337654216d26f99f52 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 17 Aug 2022 15:31:52 -0700 Subject: [PATCH 2/2] remove support for non-UTF8 encodings since SSE doesn't allow them --- src/LaunchDarkly.EventSource/Configuration.cs | 8 -- .../ConfigurationBuilder.cs | 31 ++------ .../EventSourceService.cs | 17 +++-- src/LaunchDarkly.EventSource/MessageEvent.cs | 25 ++---- .../Resources.Designer.cs | 76 ++++++++++++++----- src/LaunchDarkly.EventSource/Resources.resx | 3 + .../EventSourceEncodingTest.cs | 13 ++-- 7 files changed, 86 insertions(+), 87 deletions(-) diff --git a/src/LaunchDarkly.EventSource/Configuration.cs b/src/LaunchDarkly.EventSource/Configuration.cs index 79d948b..cbee65d 100644 --- a/src/LaunchDarkly.EventSource/Configuration.cs +++ b/src/LaunchDarkly.EventSource/Configuration.cs @@ -74,13 +74,6 @@ public sealed class Configuration [Obsolete("Use ResponseStartTimeout")] public TimeSpan ConnectionTimeout => ResponseStartTimeout; - /// - /// The character encoding to use when reading the stream if the server did not specify - /// an encoding in a Content-Type header. - /// - /// - public Encoding DefaultEncoding { get; } - /// /// The HttpClient that will be used as the HTTP client, or null for a new HttpClient. /// @@ -183,7 +176,6 @@ internal Configuration(ConfigurationBuilder builder) (builder._logAdapter is null ? null : builder._logAdapter.Logger(Configuration.DefaultLoggerName)); BackoffResetThreshold = builder._backoffResetThreshold; - DefaultEncoding = builder._defaultEncoding ?? Encoding.UTF8; HttpClient = builder._httpClient; HttpMessageHandler = (builder._httpClient != null) ? null : builder._httpMessageHandler; InitialRetryDelay = builder._initialRetryDelay; diff --git a/src/LaunchDarkly.EventSource/ConfigurationBuilder.cs b/src/LaunchDarkly.EventSource/ConfigurationBuilder.cs index f157164..947b4d4 100644 --- a/src/LaunchDarkly.EventSource/ConfigurationBuilder.cs +++ b/src/LaunchDarkly.EventSource/ConfigurationBuilder.cs @@ -28,7 +28,6 @@ public class ConfigurationBuilder #region Private Fields internal readonly Uri _uri; - internal Encoding _defaultEncoding = Encoding.UTF8; internal TimeSpan _initialRetryDelay = Configuration.DefaultInitialRetryDelay; internal TimeSpan _backoffResetThreshold = Configuration.DefaultBackoffResetThreshold; internal string _lastEventId; @@ -89,21 +88,6 @@ public ConfigurationBuilder HttpRequestModifier(Action httpR return this; } - /// - /// Sets the character encoding to use when reading the stream if the server did not specify - /// an encoding in a Content-Type header. - /// - /// A System.Text.Encoding; if null, the default - /// is - /// the builder - /// - /// - public ConfigurationBuilder DefaultEncoding(Encoding defaultEncoding) - { - _defaultEncoding = defaultEncoding ?? Encoding.UTF8; - return this; - } - /// /// Sets the initial amount of time to wait before attempting to reconnect to the EventSource API. /// @@ -320,17 +304,14 @@ public ConfigurationBuilder Logger(Logger logger) /// preferable to store and process event data as UTF-8 byte arrays rather than /// strings. By default, EventSource will use the string type when /// processing the event stream; if you then use - /// to get the data, it will be converted to a byte array as needed. It will also - /// always use the string type internally if the stream's encoding is not - /// UTF-8. However, if the stream's encoding is UTF-8 and you have set - /// PreferDataAsUtf8Bytes to , the event data will - /// be stored internally as a UTF-8 byte array so that if you read + /// to get the data, it will be converted to a byte array as needed. However, if + /// you set PreferDataAsUtf8Bytes to , the event data + /// will be stored internally as a UTF-8 byte array so that if you read /// , you will get the same array with no /// extra copying or conversion. Therefore, for greatest efficiency you should set - /// this to if you intend to process the data as UTF-8 and - /// if you expect that the server will provide it in that encoding. If the server - /// turns out not to use that encoding, everything will still work the same except - /// that there will be more overhead from string conversion. + /// this to if you intend to process the data as UTF-8. Note + /// that Server-Sent Event streams always use UTF-8 encoding, as required by the + /// SSE specification. /// /// true if you intend to request the event /// data as UTF-8 bytes diff --git a/src/LaunchDarkly.EventSource/EventSourceService.cs b/src/LaunchDarkly.EventSource/EventSourceService.cs index 3773145..31c73f0 100644 --- a/src/LaunchDarkly.EventSource/EventSourceService.cs +++ b/src/LaunchDarkly.EventSource/EventSourceService.cs @@ -100,20 +100,25 @@ CancellationToken cancellationToken _logger.Debug("Response status: {0}", (int)response.StatusCode); ValidateResponse(response); - OnConnectionOpened(); - using (var stream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false)) { var encoding = DetectEncoding(response); - if (encoding == Encoding.UTF8 && _configuration.PreferDataAsUtf8Bytes) + if (encoding != Encoding.UTF8) + { + throw new EventSourceServiceCancelledException( + string.Format(Resources.ErrorWrongEncoding, encoding.HeaderName)); + } + OnConnectionOpened(); + + if (_configuration.PreferDataAsUtf8Bytes) { _logger.Debug("Reading UTF-8 stream without string conversion"); await ProcessResponseFromUtf8StreamAsync(processResponseLineUTF8, stream, cancellationToken); } else { - _logger.Debug("Reading stream with {0} encoding and string conversion", encoding.EncodingName); - using (var reader = new StreamReader(stream, encoding)) + _logger.Debug("Reading stream with string conversion"); + using (var reader = new StreamReader(stream, Encoding.UTF8)) { await ProcessResponseFromReaderAsync(processResponseLineString, reader, cancellationToken); } @@ -135,7 +140,7 @@ private Encoding DetectEncoding(HttpResponseMessage response) } catch (ArgumentException) { } } - return _configuration.DefaultEncoding ?? Encoding.UTF8; + return Encoding.UTF8; } protected virtual async Task ProcessResponseFromReaderAsync( diff --git a/src/LaunchDarkly.EventSource/MessageEvent.cs b/src/LaunchDarkly.EventSource/MessageEvent.cs index a2b81a7..a1c7332 100644 --- a/src/LaunchDarkly.EventSource/MessageEvent.cs +++ b/src/LaunchDarkly.EventSource/MessageEvent.cs @@ -19,27 +19,12 @@ namespace LaunchDarkly.EventSource /// /// Since strings in .NET use two-byte UTF-16 characters, if you have a large block of /// UTF-8 data it is considerably more efficient to process it in its original form - /// rather than converting it to or from a string. MessageEvent converts - /// transparently between these types depending on the original character encoding of - /// the stream; the properties - /// and ; and whether the caller reads - /// from the property or . - /// If you intend to process the data as UTF-8 bytes, and if you expect that the server - /// will provide UTF-8, you should set + /// rather than converting it to or from a string. stores + /// data as strings by default, but you set + /// it can store the raw UTF-8 data instead. In either case, MessageEvent will + /// convert types transparently so that you can read either + /// or . /// - /// - /// If the stream encoding is UTF-8, and you read the event data with the - /// property, the event data is stored as a - /// UTF-8 byte array when it is first read from the stream and it returns the same - /// array, without any further copying and without creating a string. - /// If the stream encoding is UTF-8, but you read the event data with the - /// property, the event data is originally read from - /// the stream as a UTF-8 byte array but is then converted to a string. - /// If the stream encoding is not UTF-8, the event data is originally read from - /// the stream as a string. will return the - /// same string; will create a new - /// UTF-8 byte array from it. - /// /// public struct MessageEvent { diff --git a/src/LaunchDarkly.EventSource/Resources.Designer.cs b/src/LaunchDarkly.EventSource/Resources.Designer.cs index 0d1e225..8c722f6 100644 --- a/src/LaunchDarkly.EventSource/Resources.Designer.cs +++ b/src/LaunchDarkly.EventSource/Resources.Designer.cs @@ -1,7 +1,6 @@ -//------------------------------------------------------------------------------ +//------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -10,35 +9,46 @@ namespace LaunchDarkly.EventSource { using System; - using System.Reflection; - [System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] - [System.Diagnostics.DebuggerNonUserCodeAttribute()] - [System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + /// + /// A strongly-typed resource class, for looking up localized strings, etc. + /// This class was generated by MSBuild using the GenerateResource task. + /// To add or remove a member, edit your .resx file then rerun MSBuild. + /// + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Build.Tasks.StronglyTypedResourceBuilder", "15.1.0.0")] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Resources { - private static System.Resources.ResourceManager resourceMan; + private static global::System.Resources.ResourceManager resourceMan; - private static System.Globalization.CultureInfo resourceCulture; + private static global::System.Globalization.CultureInfo resourceCulture; - [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] internal Resources() { } - [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] - internal static System.Resources.ResourceManager ResourceManager { + /// + /// Returns the cached ResourceManager instance used by this class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Resources.ResourceManager ResourceManager { get { - if (object.Equals(null, resourceMan)) { - System.Resources.ResourceManager temp = new System.Resources.ResourceManager("LaunchDarkly.EventSource.Resources", typeof(Resources).Assembly); + if (object.ReferenceEquals(resourceMan, null)) { + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("LaunchDarkly.EventSource.Resources", typeof(Resources).Assembly); resourceMan = temp; } return resourceMan; } } - [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] - internal static System.Globalization.CultureInfo Culture { + /// + /// Overrides the current thread's CurrentUICulture property for all + /// resource lookups using this strongly typed resource class. + /// + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] + internal static global::System.Globalization.CultureInfo Culture { get { return resourceCulture; } @@ -47,33 +57,57 @@ internal static System.Globalization.CultureInfo Culture { } } - internal static string ErrorReadTimeout { + /// + /// Looks up a localized string similar to Invalid attempt to call Start() while the connection state is {0}. + /// + internal static string ErrorAlreadyStarted { get { - return ResourceManager.GetString("ErrorReadTimeout", resourceCulture); + return ResourceManager.GetString("ErrorAlreadyStarted", resourceCulture); } } - internal static string ErrorAlreadyStarted { + /// + /// Looks up a localized string similar to HTTP response had no body. + /// + internal static string ErrorEmptyResponse { get { - return ResourceManager.GetString("ErrorAlreadyStarted", resourceCulture); + return ResourceManager.GetString("ErrorEmptyResponse", resourceCulture); } } + /// + /// Looks up a localized string similar to Unexpected HTTP status code {0} from server. + /// internal static string ErrorHttpStatus { get { return ResourceManager.GetString("ErrorHttpStatus", resourceCulture); } } + /// + /// Looks up a localized string similar to Read timeout elapsed with no new data from server; connection may have been silently dropped. + /// + internal static string ErrorReadTimeout { + get { + return ResourceManager.GetString("ErrorReadTimeout", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Unexpected HTTP content type "{0}"; should be "text/event-stream". + /// internal static string ErrorWrongContentType { get { return ResourceManager.GetString("ErrorWrongContentType", resourceCulture); } } - internal static string ErrorEmptyResponse { + /// + /// Looks up a localized string similar to Unexpected character encoding {0}; should be UTF-8. + /// + internal static string ErrorWrongEncoding { get { - return ResourceManager.GetString("ErrorEmptyResponse", resourceCulture); + return ResourceManager.GetString("ErrorWrongEncoding", resourceCulture); } } } diff --git a/src/LaunchDarkly.EventSource/Resources.resx b/src/LaunchDarkly.EventSource/Resources.resx index 4ffbfb4..82bf32d 100644 --- a/src/LaunchDarkly.EventSource/Resources.resx +++ b/src/LaunchDarkly.EventSource/Resources.resx @@ -129,6 +129,9 @@ Unexpected HTTP content type "{0}"; should be "text/event-stream" + + Unexpected character encoding {0}; should be UTF-8 + HTTP response had no body diff --git a/test/LaunchDarkly.EventSource.Tests/EventSourceEncodingTest.cs b/test/LaunchDarkly.EventSource.Tests/EventSourceEncodingTest.cs index 049e343..96999c2 100644 --- a/test/LaunchDarkly.EventSource.Tests/EventSourceEncodingTest.cs +++ b/test/LaunchDarkly.EventSource.Tests/EventSourceEncodingTest.cs @@ -1,5 +1,6 @@ using System.Text; using System.Threading.Tasks; +using LaunchDarkly.TestHelpers; using LaunchDarkly.TestHelpers.HttpTest; using Xunit; using Xunit.Abstractions; @@ -97,24 +98,22 @@ public void CanReceiveUtf8EventDataAsBytes(bool setExplicitEncoding) } } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void NonUtf8EncodingIsReadAsStrings(bool preferUtf8Data) + [Fact] + public void NonUtf8EncodingIsRejected() { using (var server = HttpServer.Start(MakeStreamHandler(Encoding.GetEncoding("iso-8859-1")))) { var config = Configuration.Builder(server.Uri) .LogAdapter(_testLogging) - .PreferDataAsUtf8Bytes(preferUtf8Data) .Build(); using (var es = new EventSource(config)) { var sink = new EventSink(es, _testLogging) { ExpectUtf8Data = false }; - _ = Task.Run(es.StartAsync); - sink.ExpectActions(expectedEventActions); + var errorAction = sink.ExpectAction(); + var ex = Assert.IsType(errorAction.Exception); + Assert.Matches(".*encoding.*8859.*", ex.Message); } } }