Merge pull request #89 from launchdarkly/eb/sc-165127/only-utf8

remove support for non-UTF8 encodings since SSE doesn't allow them

Author: eli-darkly

src/LaunchDarkly.EventSource/Configuration.cs

@@ -74,13 +74,6 @@ public sealed class Configuration
         [Obsolete("Use ResponseStartTimeout")]
         public TimeSpan ConnectionTimeout => ResponseStartTimeout;
 
-        /// <summary>
-        /// The character encoding to use when reading the stream if the server did not specify
-        /// an encoding in a <c>Content-Type</c> header.
-        /// </summary>
-        /// <seealso cref="ConfigurationBuilder.DefaultEncoding(Encoding)"/>
-        public Encoding DefaultEncoding { get; }
-
         /// <summary>
         /// The HttpClient that will be used as the HTTP client, or null for a new HttpClient.
         /// </summary>
@@ -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;

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<HttpRequestMessage> httpR
             return this;
         }
 
-        /// <summary>
-        /// Sets the character encoding to use when reading the stream if the server did not specify
-        /// an encoding in a <c>Content-Type</c> header.
-        /// </summary>
-        /// <param name="defaultEncoding">A <c>System.Text.Encoding</c>; if null, the default
-        /// is <see cref="Encoding.UTF8"/></param>
-        /// <returns>the builder</returns>
-        /// <seealso cref="MessageEvent"/>
-        /// <seealso cref="PreferDataAsUtf8Bytes"/>
-        public ConfigurationBuilder DefaultEncoding(Encoding defaultEncoding)
-        {
-            _defaultEncoding = defaultEncoding ?? Encoding.UTF8;
-            return this;
-        }
-
         /// <summary>
         /// Sets the initial amount of time to wait before attempting to reconnect to the EventSource API.
         /// </summary>
@@ -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, <c>EventSource</c> will use the <c>string</c> type when
         /// processing the event stream; if you then use <see cref="MessageEvent.DataUtf8Bytes"/>
-        /// to get the data, it will be converted to a byte array as needed. It will also
-        /// always use the <c>string</c> type internally if the stream's encoding is not
-        /// UTF-8. However, if the stream's encoding is UTF-8 <c>and</c> you have set
-        /// <c>PreferDataAsUtf8Bytes</c> to <see langword="true"/>, 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 <c>PreferDataAsUtf8Bytes</c> to <see langword="true"/>, the event data
+        /// will be stored internally as a UTF-8 byte array so that if you read
         /// <see cref="MessageEvent.DataUtf8Bytes"/>, you will get the same array with no
         /// extra copying or conversion. Therefore, for greatest efficiency you should set
-        /// this to <see langword="true"/> 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 <see langword="true"/> 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.
         /// </remarks>
         /// <param name="preferDataAsUtf8Bytes">true if you intend to request the event
         /// data as UTF-8 bytes</param>

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(

src/LaunchDarkly.EventSource/MessageEvent.cs

@@ -19,27 +19,12 @@ namespace LaunchDarkly.EventSource
     /// <para>
     /// 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. <c>MessageEvent</c> converts
-    /// transparently between these types depending on the original character encoding of
-    /// the stream; the <see cref="Configuration"/> properties <see cref="Configuration.DefaultEncoding"/>
-    /// and <see cref="Configuration.PreferDataAsUtf8Bytes"/>; and whether the caller reads
-    /// from the property <see cref="MessageEvent.Data"/> or <see cref="MessageEvent.DataUtf8Bytes"/>.
-    /// 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. <see cref="EventSource"/> stores
+    /// data as strings by default, but you set <see cref="ConfigurationBuilder.PreferDataAsUtf8Bytes(bool)"/>
+    /// it can store the raw UTF-8 data instead. In either case, <c>MessageEvent</c> will
+    /// convert types transparently so that you can read either <see cref="MessageEvent.Data"/>
+    /// or <see cref="MessageEvent.DataUtf8Bytes"/>.
     /// </para>
-    /// <list type="bullet">
-    /// <item> If the stream encoding is UTF-8, and you read the event data with the
-    /// <see cref="MessageEvent.DataUtf8Bytes"/> 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 <c>string</c>. </item>
-    /// <item> If the stream encoding is UTF-8, but you read the event data with the
-    /// <see cref="MessageEvent.Data"/> property, the event data is originally read from
-    /// the stream as a UTF-8 byte array but is then converted to a <c>string</c>. </item>
-    /// <item> If the stream encoding is not UTF-8, the event data is originally read from
-    /// the stream as a <c>string</c>. <see cref="MessageEvent.Data"/> will return the
-    /// same <c>string</c>; <see cref="MessageEvent.DataUtf8Bytes"/> will create a new
-    /// UTF-8 byte array from it.</item>
-    /// </list>
     /// </remarks>
     public struct MessageEvent
     {

src/LaunchDarkly.EventSource/Resources.Designer.cs

@@ -129,6 +129,9 @@
   <data name="ErrorWrongContentType" xml:space="preserve">
     <value>Unexpected HTTP content type "{0}"; should be "text/event-stream"</value>
   </data>
+  <data name="ErrorWrongEncoding" xml:space="preserve">
+    <value>Unexpected character encoding {0}; should be UTF-8</value>
+  </data>
   <data name="ErrorEmptyResponse" xml:space="preserve">
     <value>HTTP response had no body</value>
   </data>

src/LaunchDarkly.EventSource/Resources.resx

@@ -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<EventSourceServiceCancelledException>(errorAction.Exception);
+                    Assert.Matches(".*encoding.*8859.*", ex.Message);
                 }
             }
         }