From e626f51a6a4bee41ae1dbe7a2aa63e3b7853e454 Mon Sep 17 00:00:00 2001 From: Jark Reijerink Date: Tue, 16 May 2017 13:24:45 +0100 Subject: [PATCH] Serialize and deserialize with the correct encoding, fixes #119 --- src/HttpMessage/HttpServerResponse.cs | 5 +- .../ServerResponseParsers/ContentParser.cs | 5 +- .../Rest/FluentRestRouteHandlerTests.cs | 17 +++ .../RestRouteHandlerTests.XmlAcceptCharset.cs | 134 ++++++++++++++++++ .../TestHelpers/FluentHttpServerTests.cs | 98 ++++++++----- .../WebServer.UnitTests.csproj | 2 + src/WebServer/EncodingCache.cs | 38 ++--- src/WebServer/Http/ContentSerializer.cs | 70 +++++---- .../Rest/RestControllerMethodInfo.cs | 2 +- ...RestControllerMethodWithContentExecutor.cs | 5 +- .../Rest/RestToHttpResponseConverter.cs | 2 +- 11 files changed, 274 insertions(+), 104 deletions(-) create mode 100644 src/WebServer.UnitTests/Rest/FluentRestRouteHandlerTests.cs create mode 100644 src/WebServer.UnitTests/Rest/RestRouteHandlerTests.XmlAcceptCharset.cs diff --git a/src/HttpMessage/HttpServerResponse.cs b/src/HttpMessage/HttpServerResponse.cs index b5dfe6e..f85a75a 100644 --- a/src/HttpMessage/HttpServerResponse.cs +++ b/src/HttpMessage/HttpServerResponse.cs @@ -181,10 +181,7 @@ public byte[] ToBytes() public override string ToString() { #if DEBUG - // This is just used for debugging purposes and will not be available when running in release mode. Problem with - // this method is that it uses Encoding to decode the content which is a fairly complicated process. For debugging - // purposes I'm using UTF-8 which is working most of the time. In real life you want to use the charset provided, or - // some default encoding as explained in the HTTP specs. + // This is just used for debugging purposes and will not be available when running in release mode. return HttpServerResponseParser.Default.ConvertToString(this); #else return $"{HttpVersion} {ResponseStatus} including {Headers.Count()} headers."; diff --git a/src/HttpMessage/ServerResponseParsers/ContentParser.cs b/src/HttpMessage/ServerResponseParsers/ContentParser.cs index 3a74b78..aaa5147 100644 --- a/src/HttpMessage/ServerResponseParsers/ContentParser.cs +++ b/src/HttpMessage/ServerResponseParsers/ContentParser.cs @@ -5,7 +5,7 @@ namespace Restup.HttpMessage.ServerResponseParsers { internal class ContentParser : IHttpResponsePartParser { - private static Encoding DEFAULT_CONTENT_ENCODING = Encoding.UTF8; + private static readonly Encoding DEFAULT_CONTENT_ENCODING = Encoding.UTF8; public byte[] ParseToBytes(HttpServerResponse response) { @@ -17,7 +17,8 @@ public string ParseToString(HttpServerResponse response) if (response.Content == null) return string.Empty; - return DEFAULT_CONTENT_ENCODING.GetString(response.Content); + var encoding = Encoding.GetEncoding(response.ContentCharset); + return (encoding ?? DEFAULT_CONTENT_ENCODING).GetString(response.Content); } } } diff --git a/src/WebServer.UnitTests/Rest/FluentRestRouteHandlerTests.cs b/src/WebServer.UnitTests/Rest/FluentRestRouteHandlerTests.cs new file mode 100644 index 0000000..396f9fd --- /dev/null +++ b/src/WebServer.UnitTests/Rest/FluentRestRouteHandlerTests.cs @@ -0,0 +1,17 @@ +using Restup.Webserver.Rest; +using Restup.Webserver.UnitTests.TestHelpers; + +namespace Restup.Webserver.UnitTests.Rest +{ + public class FluentRestRouteHandlerTests : FluentHttpServerTests + { + public FluentRestRouteHandlerTests ControllersIsRegistered(string urlPrefix, params object[] arguments) where T : class + { + var restRouteHandler = new RestRouteHandler(); + restRouteHandler.RegisterController(arguments); + RegisterRouteHandler(urlPrefix, restRouteHandler); + + return this; + } + } +} \ No newline at end of file diff --git a/src/WebServer.UnitTests/Rest/RestRouteHandlerTests.XmlAcceptCharset.cs b/src/WebServer.UnitTests/Rest/RestRouteHandlerTests.XmlAcceptCharset.cs new file mode 100644 index 0000000..1251994 --- /dev/null +++ b/src/WebServer.UnitTests/Rest/RestRouteHandlerTests.XmlAcceptCharset.cs @@ -0,0 +1,134 @@ +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.Serialization; +using System.Text; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Restup.HttpMessage.Headers.Request; +using Restup.HttpMessage.Models.Schemas; +using Restup.Webserver.Attributes; +using Restup.Webserver.Models.Contracts; +using Restup.Webserver.Models.Schemas; +using Restup.Webserver.UnitTests.TestHelpers; + +namespace Restup.Webserver.UnitTests.Rest +{ + [TestClass] + public class RestRouteHandlerTests_XmlAcceptCharset : RestRouteHandlerTests + { + [TestMethod] + [DataRow("utf-8")] + [DataRow("utf-16")] + public void WhenAGetRequestWithAcceptMediaTypeXmlAndAnAcceptEncodingIsReceived_ThenTheResponseIsCorrectlyEncoded(string charset) + { + new FluentRestRouteHandlerTests() + .Given + .ControllersIsRegistered("/api", new ProductStore()) + .When + .RequestHasArrived("/api/products", + acceptCharsets: new[] { charset }, + acceptMediaTypes: new[] { "application/xml" }) + .Then + .AssertLastResponse(x => x.ContentCharset, charset) + .AssertLastResponseContent($"{Product.DefaultId}{Product.DefaultName}{Product.DefaultCurrency}"); + } + + [TestMethod] + [DataRow("utf-8")] + [DataRow("utf-16")] + public void WhenAPostRequestWithAcceptMediaTypeXmlAndAnAcceptEncodingIsReceived_ThenTheRequestIsCorrectlyReceived(string charset) + { + var productStore = new ProductStore(); + var id = 2; + var name = "new name"; + var currency = "£"; + + var encoding = Encoding.GetEncoding(charset); + + new FluentRestRouteHandlerTests() + .Given + .ControllersIsRegistered("/api", productStore) + .When + .RequestHasArrived("/api/products", + method: HttpMethod.POST, + acceptCharsets: new[] { charset }, + acceptMediaTypes: new[] { "application/xml" }, + contentType: "application/xml", + contentCharset: charset, + content: encoding.GetBytes($"{id}{name}{currency}")) + .Then + .AssertLastResponseHasNoHeaderOf() + .AssertLastResponse(x => x.ResponseStatus, HttpResponseStatus.Created); + + Assert.AreEqual(1, productStore.PostedProducts.Count); + + var postedProduct = productStore.PostedProducts.Single(); + Assert.AreEqual(id, postedProduct.Id); + Assert.AreEqual(name, postedProduct.Name); + Assert.AreEqual(currency, postedProduct.Currency); + } + + [RestController(InstanceCreationType.PerCall)] + public class XmlTestController + { + private readonly ProductStore productStore; + + public XmlTestController(ProductStore productStore) + { + this.productStore = productStore; + } + + [UriFormat("/products")] + public IGetResponse GetProduct() + { + return new GetResponse(GetResponse.ResponseStatus.OK, productStore.Get()); + } + + [UriFormat("/products")] + public IPostResponse PostProduct([FromContent] Product product) + { + productStore.Post(product); + return new PostResponse(PostResponse.ResponseStatus.Created); + } + } + + public class ProductStore + { + public ConcurrentQueue PostedProducts = new ConcurrentQueue(); + + public Product Get() + { + return new Product(Product.DefaultId, Product.DefaultName, Product.DefaultCurrency); + } + + public void Post(Product product) + { + PostedProducts.Enqueue(product); + } + } + + public class Product + { + public const int DefaultId = 1; + public const string DefaultName = "default"; + public const string DefaultCurrency = "£"; + + public int Id { get; set; } + public string Name { get; set; } + public string Currency { get; set; } + + // xml serialisation + private Product() + { + + } + + public Product(int id, string name, string currency) + { + Id = id; + Name = name; + Currency = currency; + } + } + } +} \ No newline at end of file diff --git a/src/WebServer.UnitTests/TestHelpers/FluentHttpServerTests.cs b/src/WebServer.UnitTests/TestHelpers/FluentHttpServerTests.cs index 63185bb..09e87d8 100644 --- a/src/WebServer.UnitTests/TestHelpers/FluentHttpServerTests.cs +++ b/src/WebServer.UnitTests/TestHelpers/FluentHttpServerTests.cs @@ -1,15 +1,19 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using Microsoft.VisualStudio.TestTools.UnitTesting; using Restup.HttpMessage; using Restup.HttpMessage.Models.Contracts; using Restup.HttpMessage.Models.Schemas; using Restup.Webserver.Http; +using Restup.Webserver.Models.Contracts; namespace Restup.Webserver.UnitTests.TestHelpers { - public class FluentHttpServerTests + public class FluentHttpServerTests : FluentHttpServerTests { } + + public class FluentHttpServerTests where T : FluentHttpServerTests { private HttpServer _httpServer; private readonly List _responses; @@ -17,9 +21,9 @@ public class FluentHttpServerTests private readonly Dictionary _routeHandlers; private readonly HttpServerConfiguration _configuration; - public FluentHttpServerTests Given => this; - public FluentHttpServerTests When => this; - public FluentHttpServerTests Then => this; + public T Given => (T) this; + public T When => (T) this; + public T Then => (T) this; public FluentHttpServerTests() { @@ -29,130 +33,146 @@ public FluentHttpServerTests() _routeHandlers = new Dictionary(); } - public FluentHttpServerTests ListeningOnDefaultRoute() + public T ListeningOnDefaultRoute() { var routeHandler = new EchoRouteHandler(); _routeHandlers[string.Empty] = routeHandler; _configuration.RegisterRoute(routeHandler); _httpServer = new HttpServer(_configuration); - return this; + return (T) this; } - public FluentHttpServerTests ListeningOnRoute(string urlPrefix) + public T ListeningOnRoute(string urlPrefix) { var routeHandler = new EchoRouteHandler(); _routeHandlers[urlPrefix] = routeHandler; + RegisterRouteHandler(urlPrefix, routeHandler); + return (T) this; + } + + protected void RegisterRouteHandler(string urlPrefix, IRouteHandler routeHandler) + { _configuration.RegisterRoute(urlPrefix, routeHandler); _httpServer = new HttpServer(_configuration); - return this; } - public FluentHttpServerTests RequestHasArrived(string uri, IEnumerable acceptEncodings = null, + public T RequestHasArrived(string uri, IEnumerable acceptEncodings = null, byte[] content = null, string origin = null, HttpMethod? method = HttpMethod.GET, HttpMethod? accessControlRequestMethod = null, - IEnumerable accessControlRequestHeaders = null, string contentType = null) + IEnumerable accessControlRequestHeaders = null, string contentType = null, IEnumerable acceptCharsets = null, + string contentCharset = null, IEnumerable acceptMediaTypes = null) { var httpServerRequest = Utils.CreateHttpRequest(uri: new Uri(uri, UriKind.Relative), - acceptEncodings: acceptEncodings, content: content, origin: origin, method: method, + acceptEncodings: acceptEncodings, content: content, origin: origin, method: method, accessControlRequestMethod: accessControlRequestMethod, accessControlRequestHeaders: accessControlRequestHeaders, - contentType: contentType); + contentType: contentType, acceptCharsets: acceptCharsets, contentTypeCharset: contentCharset, acceptMediaTypes: acceptMediaTypes); var response = _httpServer.HandleRequestAsync(httpServerRequest).Result; _responses.Add(response); - return this; + return (T) this; } - public FluentHttpServerTests AssertLastResponse(Func actualFunc, T expected) + public T AssertLastResponse(Func actualFunc, TValue expected) { var response = _responses.Last(); var actual = actualFunc(response); Assert.AreEqual(expected, actual); - return this; + return (T) this; + } + + public T AssertLastResponseContent(string expected) + { + var response = _responses.Last(); + var encoding = Encoding.GetEncoding(response.ContentCharset); + var actual = encoding.GetString(response.Content); + + Assert.AreEqual(expected, actual); + return (T)this; } - public FluentHttpServerTests AssertLastResponse(Func actualFunc, T1 expected) where T : IHttpHeader + public T AssertLastResponse(Func actualFunc, TValue expected) where THeader : IHttpHeader { var response = _responses.Last(); - var header = response.Headers.OfType().First(); + var header = response.Headers.OfType().First(); var actual = actualFunc(header); Assert.AreEqual(expected, actual); - return this; + return (T) this; } - public FluentHttpServerTests AssertLastResponse(string expected) where T : IHttpHeader + public T AssertLastResponse(string expected) where THeader : IHttpHeader { - return AssertLastResponse(x => x.Value, expected); + return AssertLastResponse(x => x.Value, expected); } - public FluentHttpServerTests AssertLastResponseHasNoHeaderOf() where T : IHttpHeader + public T AssertLastResponseHasNoHeaderOf() where THeader : IHttpHeader { var response = _responses.Last(); - var anyHeaderOfTypeT = response.Headers.OfType().Any(); + var anyHeaderOfTypeT = response.Headers.OfType().Any(); Assert.IsFalse(anyHeaderOfTypeT); - return this; + return (T) this; } - public FluentHttpServerTests AssertRouteHandlerRequest(Func actualFunc, T expected) + public T AssertRouteHandlerRequest(Func actualFunc, TValue expected) { var routeHandler = _routeHandlers.Single().Value; var request = routeHandler.Requests.Last(); var actual = actualFunc(request); Assert.AreEqual(expected, actual); - return this; + return (T) this; } - public FluentHttpServerTests AssertRouteHandlerReceivedRequest() + public T AssertRouteHandlerReceivedRequest() { var routeHandler = _routeHandlers.Single().Value; Assert.AreEqual(1, routeHandler.Requests.Count()); - return this; + return (T) this; } - public FluentHttpServerTests AssertRouteHandlerReceivedNoRequests() + public T AssertRouteHandlerReceivedNoRequests() { var routeHandler = _routeHandlers.Single().Value; Assert.AreEqual(0, routeHandler.Requests.Count()); - return this; + return (T) this; } - public FluentHttpServerTests AssertRouteHandlerRequest(string prefix, Func actualFunc, T expected) + public T AssertRouteHandlerRequest(string prefix, Func actualFunc, TValue expected) { var routeHandler = _routeHandlers[prefix]; var request = routeHandler.Requests.Last(); var actual = actualFunc(request); Assert.AreEqual(expected, actual); - return this; + return (T) this; } - public FluentHttpServerTests AssertRouteHandlerReceivedRequest(string prefix) + public T AssertRouteHandlerReceivedRequest(string prefix) { var routeHandler = _routeHandlers[prefix]; Assert.AreEqual(1, routeHandler.Requests.Count()); - return this; + return (T) this; } - public FluentHttpServerTests AssertRouteHandlerReceivedNoRequests(string prefix) + public T AssertRouteHandlerReceivedNoRequests(string prefix) { var routeHandler = _routeHandlers[prefix]; Assert.AreEqual(0, routeHandler.Requests.Count()); - return this; + return (T) this; } - public FluentHttpServerTests CorsIsEnabled() + public T CorsIsEnabled() { _configuration.EnableCors(); _httpServer = new HttpServer(_configuration); - return this; + return (T) this; } - public FluentHttpServerTests CorsIsEnabled(Action builder) + public T CorsIsEnabled(Action builder) { _configuration.EnableCors(builder); _httpServer = new HttpServer(_configuration); - return this; + return (T) this; } } } \ No newline at end of file diff --git a/src/WebServer.UnitTests/WebServer.UnitTests.csproj b/src/WebServer.UnitTests/WebServer.UnitTests.csproj index 7549111..56b181b 100644 --- a/src/WebServer.UnitTests/WebServer.UnitTests.csproj +++ b/src/WebServer.UnitTests/WebServer.UnitTests.csproj @@ -98,8 +98,10 @@ + + diff --git a/src/WebServer/EncodingCache.cs b/src/WebServer/EncodingCache.cs index 225a26a..576d4ff 100644 --- a/src/WebServer/EncodingCache.cs +++ b/src/WebServer/EncodingCache.cs @@ -1,5 +1,5 @@ using System; -using System.Collections.Generic; +using System.Collections.Concurrent; using System.Text; namespace Restup.Webserver @@ -13,13 +13,11 @@ static EncodingCache() Default = new EncodingCache(); } - private Dictionary _cache; - private object _cacheLock; + private readonly ConcurrentDictionary _cache; internal EncodingCache() { - _cache = new Dictionary(StringComparer.OrdinalIgnoreCase); - _cacheLock = new object(); + _cache = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); } internal Encoding GetEncoding(string charset) @@ -29,27 +27,19 @@ internal Encoding GetEncoding(string charset) return null; } - if (!_cache.ContainsKey(charset)) - { - lock (_cacheLock) - { - if (!_cache.ContainsKey(charset)) - { - Encoding charsetEncoding = null; - try - { - charsetEncoding = Encoding.GetEncoding(charset); - } - catch - { - } + return _cache.GetOrAdd(charset, ResolveEncoding); + } - _cache[charset] = charsetEncoding; - } - } + private static Encoding ResolveEncoding(string charset) + { + try + { + return Encoding.GetEncoding(charset); + } + catch + { + return null; } - - return _cache[charset]; } } } diff --git a/src/WebServer/Http/ContentSerializer.cs b/src/WebServer/Http/ContentSerializer.cs index 3c6876d..32a00c6 100644 --- a/src/WebServer/Http/ContentSerializer.cs +++ b/src/WebServer/Http/ContentSerializer.cs @@ -1,68 +1,76 @@ -using Newtonsoft.Json; -using Restup.Webserver.Models.Schemas; -using System; +using System; using System.IO; +using System.Text; +using System.Xml; using System.Xml.Serialization; +using Newtonsoft.Json; +using Restup.Webserver.Models.Schemas; namespace Restup.Webserver.Http { internal class ContentSerializer { - internal object FromContent(string content, MediaType contentMediaType, Type contentType) + internal object FromContent(byte[] content, MediaType contentMediaType, Type contentType, Encoding contentEncoding) { - if (contentMediaType == MediaType.JSON) - { - return JsonConvert.DeserializeObject(content, contentType); - } - else if (contentMediaType == MediaType.XML) + switch (contentMediaType) { - return XmlDeserializeObject(content, contentType); + case MediaType.JSON: + var contentAsString = contentEncoding.GetString(content); + return JsonConvert.DeserializeObject(contentAsString, contentType); + case MediaType.XML: + return XmlDeserializeObject(content, contentType, contentEncoding); } throw new NotImplementedException(); } - internal byte[] ToAcceptContent(object contentObject, RestServerRequest req) + internal byte[] ToAcceptContent(object contentObject, MediaType acceptMediaType, Encoding acceptEncoding) { if (contentObject == null) { return new byte[0]; } - - if (req.AcceptMediaType == MediaType.JSON) + + switch (acceptMediaType) { - return req.AcceptEncoding.GetBytes(JsonConvert.SerializeObject(contentObject)); - } - else if (req.AcceptMediaType == MediaType.XML) - { - return req.AcceptEncoding.GetBytes(XmlSerializeObject(contentObject)); + case MediaType.JSON: + return acceptEncoding.GetBytes(JsonConvert.SerializeObject(contentObject)); + case MediaType.XML: + return XmlSerializeObject(contentObject, acceptEncoding); } - return new byte[0]; + throw new NotImplementedException(); } - private static string XmlSerializeObject(object toSerialize) + private static byte[] XmlSerializeObject(object toSerialize, Encoding acceptEncoding) { - XmlSerializer xmlSerializer = new XmlSerializer(toSerialize.GetType()); + var xmlSerializer = new XmlSerializer(toSerialize.GetType()); - using (StringWriter textWriter = new StringWriter()) + using (var memoryStream = new MemoryStream()) { - xmlSerializer.Serialize(textWriter, toSerialize); - return textWriter.ToString(); + // setting the encoding in the XmlWriter will ensure the correct encoding is output in the xml + using (var xmlWriter = XmlWriter.Create(memoryStream, new XmlWriterSettings { Encoding = acceptEncoding })) + { + xmlSerializer.Serialize(xmlWriter, toSerialize); + } + + return memoryStream.ToArray(); } } - private static object XmlDeserializeObject(string content, Type toType) + private static object XmlDeserializeObject(byte[] content, Type toType, Encoding contentEncoding) { var serializer = new XmlSerializer(toType); - object result; - - using (TextReader reader = new StringReader(content)) + using (var memoryStream = new MemoryStream(content)) { - result = serializer.Deserialize(reader); + // note: setting the content encoding here will not guarantee that this encoding will be used to read the xml + // once the reader hits the encoding="" attribute it will immediately switch to that encoding, but this + // is still the right thing to do because the encoding="" might not have been specified + using (var reader = new StreamReader(memoryStream, contentEncoding)) + { + return serializer.Deserialize(reader); + } } - - return result; } } } diff --git a/src/WebServer/Rest/RestControllerMethodInfo.cs b/src/WebServer/Rest/RestControllerMethodInfo.cs index 1444b44..450b0ec 100644 --- a/src/WebServer/Rest/RestControllerMethodInfo.cs +++ b/src/WebServer/Rest/RestControllerMethodInfo.cs @@ -117,7 +117,7 @@ where p.GetCustomAttribute() == null if (!ParametersHaveValidType(methodParameters.Select(p => p.ParameterType))) { - throw new InvalidOperationException("Can't use method parameters with a custom type."); + throw new InvalidOperationException($"Can't use method parameters with a custom type, use the {typeof(FromContentAttribute)} if you meant to include it in the body of the request."); } var parameterValueGetters = methodParameters.Select(x => GetParameterGetter(x, MatchUri)).ToArray(); diff --git a/src/WebServer/Rest/RestControllerMethodWithContentExecutor.cs b/src/WebServer/Rest/RestControllerMethodWithContentExecutor.cs index b6cb41d..2b506d3 100644 --- a/src/WebServer/Rest/RestControllerMethodWithContentExecutor.cs +++ b/src/WebServer/Rest/RestControllerMethodWithContentExecutor.cs @@ -28,9 +28,10 @@ protected override object ExecuteAnonymousMethod(RestControllerMethodInfo info, if (request.HttpServerRequest.Content != null) { contentObj = _contentSerializer.FromContent( - request.ContentEncoding.GetString(request.HttpServerRequest.Content), + request.HttpServerRequest.Content, request.ContentMediaType, - info.ContentParameterType); + info.ContentParameterType, + request.ContentEncoding); } } catch (JsonReaderException) diff --git a/src/WebServer/Rest/RestToHttpResponseConverter.cs b/src/WebServer/Rest/RestToHttpResponseConverter.cs index a1caa07..dac5f90 100644 --- a/src/WebServer/Rest/RestToHttpResponseConverter.cs +++ b/src/WebServer/Rest/RestToHttpResponseConverter.cs @@ -61,7 +61,7 @@ private HttpServerResponse GetDefaultContentResponse(IContentRestResponse respon { defaultResponse.ContentType = GetMediaTypeAsString(restReq.AcceptMediaType); defaultResponse.ContentCharset = restReq.AcceptCharset; - defaultResponse.Content = _contentSerializer.ToAcceptContent(response.ContentData, restReq); + defaultResponse.Content = _contentSerializer.ToAcceptContent(response.ContentData, restReq.AcceptMediaType, restReq.AcceptEncoding); } return defaultResponse;