Access Request.InputStream only when SOAP header present (#1115)

This commit fixes a bug where accessing the Request.InputStream
causes the entrie request to be buffered in memory. This has
two repercussions:

1. For many large requests, it can potentially cause out of memory
2. It can interfere with a custom IHostBufferPolicySelector used in
    web API to determine whether to buffer the input stream.

With this fix, the input stream is accessed only when
a SOAP content-type is present and has not already been
read bufferless.

Fixes #1113

sample/AspNetFullFrameworkSampleApp/App_Start/WebApiConfig.cs

@@ -4,7 +4,10 @@
 // See the LICENSE file in the project root for more information
 
 using System.Linq;
+using System.Web;
 using System.Web.Http;
+using System.Web.Http.Hosting;
+using System.Web.Http.WebHost;
 
 namespace AspNetFullFrameworkSampleApp
 {
@@ -16,6 +19,22 @@ public static void Register(HttpConfiguration configuration)
 			var appXmlType = configuration.Formatters.XmlFormatter.SupportedMediaTypes
 				.FirstOrDefault(t => t.MediaType == "application/xml");
 			configuration.Formatters.XmlFormatter.SupportedMediaTypes.Remove(appXmlType);
+
+			// don't buffer multipart data to web api
+			configuration.Services.Replace(typeof(IHostBufferPolicySelector), new NoBufferMultipartPolicySelector());
+		}
+	}
+
+	public class NoBufferMultipartPolicySelector : WebHostBufferPolicySelector
+	{
+		public override bool UseBufferedInputStream(object hostContext)
+		{
+			if (hostContext is HttpContextBase contextBase &&
+				contextBase.Request.ContentType != null &&
+				contextBase.Request.ContentType.Contains("multipart"))
+				return false;
+
+			return base.UseBufferedInputStream(hostContext);
 		}
 	}
 }

sample/AspNetFullFrameworkSampleApp/Asmx/Health.asmx.cs

@@ -12,5 +12,8 @@ public class Health : WebService
 	{
 		[WebMethod]
 		public string Ping() => "Ok";
+
+		[WebMethod]
+		public string Input(string input) => input;
 	}
 }

sample/AspNetFullFrameworkSampleApp/Controllers/WebApiController.cs

@@ -3,6 +3,9 @@
 // Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information
 
+using System.Net.Http;
+using System.Text;
+using System.Threading.Tasks;
 using System.Web.Http;
 
 namespace AspNetFullFrameworkSampleApp.Controllers
@@ -13,6 +16,16 @@ public class WebApiController : ApiController
 
 		public WebApiResponse Get() =>
 			new WebApiResponse { Content = "This is an example response from a web api controller" };
+
+		public async Task<IHttpActionResult> Post()
+		{
+			var multipart = await Request.Content.ReadAsMultipartAsync();
+			var result = new StringBuilder();
+			foreach(var content in multipart.Contents)
+				result.Append(await content.ReadAsStringAsync());
+
+			return Ok(result.ToString());
+		}
 	}
 
 	public class WebApiResponse

src/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs

@@ -111,8 +111,8 @@ private void ProcessBeginRequest(object sender)
 			}
 
 			var transactionName = $"{request.HttpMethod} {request.Unvalidated.Path}";
-			var soapAction = SoapRequest.ExtractSoapAction(request.Unvalidated.Headers, request.InputStream, _logger);
-			if (soapAction != null) transactionName += $" {soapAction}";
+			if (SoapRequest.TryExtractSoapAction(_logger, request, out var soapAction))
+				transactionName += $" {soapAction}";
 
 			var distributedTracingData = ExtractIncomingDistributedTracingData(request);
 			ITransaction transaction;

src/Elastic.Apm.AspNetFullFramework/Extensions/SoapRequest.cs

@@ -5,35 +5,60 @@
 using System;
 using System.Collections.Specialized;
 using System.IO;
+using System.Web;
 using System.Xml;
 using Elastic.Apm.Logging;
 
 namespace Elastic.Apm.AspNetFullFramework.Extensions
 {
+	/// <summary>
+	/// Extract details about a SOAP request from a HTTP request
+	/// </summary>
 	internal static class SoapRequest
 	{
 		private const string SoapActionHeaderName = "SOAPAction";
 		private const string ContentTypeHeaderName = "Content-Type";
 		private const string SoapAction12ContentType = "application/soap+xml";
 
 		/// <summary>
-		/// Extracts the soap action from the header if exists only with Soap 1.1
+		/// Try to extract a Soap 1.1 or Soap 1.2 action from the request.
 		/// </summary>
-		/// <param name="headers">The request headers</param>
-		/// <param name="requestStream">The request stream</param>
-		/// <param name="logger">The logger.</param>
-		public static string ExtractSoapAction(NameValueCollection headers, Stream requestStream, IApmLogger logger)
+		/// <param name="logger">The logger</param>
+		/// <param name="request">The request</param>
+		/// <param name="soapAction">The extracted soap action. <c>null</c> if no soap action is extracted</param>
+		/// <returns><c>true</c> if a soap action can be extracted, <c>false</c> otherwise.</returns>
+		public static bool TryExtractSoapAction(IApmLogger logger, HttpRequest request, out string soapAction)
 		{
 			try
 			{
-				return GetSoap11Action(headers) ?? GetSoap12Action(headers, requestStream);
+				var headers = request.Unvalidated.Headers;
+				soapAction = GetSoap11Action(headers);
+				if (soapAction != null) return true;
+
+				// if the input stream has already been read bufferless, we can't inspect it
+				if (request.ReadEntityBodyMode == ReadEntityBodyMode.Bufferless)
+				{
+					soapAction = null;
+					return false;
+				}
+
+				if (IsSoap12Action(headers))
+				{
+					// use request.GetBufferedInputStream() which causes the framework to buffer what is read
+					// so that subsequent reads can read from the beginning.
+					// ASMX SOAP services by default deserialize the SOAP message in the input stream into
+					// the parameters for the method.
+					soapAction = GetSoap12ActionFromInputStream(request.GetBufferedInputStream());
+					if (soapAction != null) return true;
+				}
 			}
 			catch (Exception e)
 			{
-				logger.Error()?.LogException(e, "Error reading soap action header");
+				logger.Error()?.LogException(e, "Error extracting soap action");
 			}
 
-			return null;
+			soapAction = null;
+			return false;
 		}
 
 		/// <summary>
@@ -51,34 +76,13 @@ private static string GetSoap11Action(NameValueCollection headers)
 			return null;
 		}
 
-		/// <summary>
-		/// Lightweight parser that extracts the soap action from the xml body only with Soap 1.2
-		/// </summary>
-		/// <param name="headers">the request headers</param>
-		/// <param name="requestStream">the request stream</param>
-		private static string GetSoap12Action(NameValueCollection headers, Stream requestStream)
+		private static bool IsSoap12Action(NameValueCollection headers)
 		{
-			//[{"key":"Content-Type","value":"application/soap+xml; charset=utf-8"}]
 			var contentType = headers.Get(ContentTypeHeaderName);
-			if (contentType is null || !contentType.Contains(SoapAction12ContentType))
-				return null;
-
-			var stream = requestStream;
-			if (!stream.CanSeek)
-				return null;
-
-			try
-			{
-				var action = GetSoap12ActionInternal(stream);
-				return action;
-			}
-			finally
-			{
-				stream.Seek(0, SeekOrigin.Begin);
-			}
+			return contentType != null && contentType.Contains(SoapAction12ContentType);
 		}
 
-		internal static string GetSoap12ActionInternal(Stream stream)
+		internal static string GetSoap12ActionFromInputStream(Stream stream)
 		{
 			try
 			{

test/Elastic.Apm.AspNetFullFramework.Tests/Soap/Soap12Tests.cs → test/Elastic.Apm.AspNetFullFramework.Tests/Soap/SoapParsingTests.cs

@@ -1,15 +1,13 @@
-using System;
-using System.Collections.Generic;
-using System.IO;
+using System.IO;
 using System.Text;
 using Elastic.Apm.AspNetFullFramework.Extensions;
 using FluentAssertions;
 using Xunit;
 
-namespace Elastic.Apm.Tests.Soap
+namespace Elastic.Apm.AspNetFullFramework.Tests.Soap
 {
 
-	public class Soap12Tests
+	public class SoapParsingTests
 	{
 		#region Samples
 		// Example 1: SOAP message containing a SOAP header block and a SOAP body
@@ -77,7 +75,7 @@ public class Soap12Tests
 ;
 
 		/// <summary>
-		/// This message is secured using WS-Security 
+		/// This message is secured using WS-Security
 		/// In fact, from a SOAP perspective, WS-Security is something that protects the contents of the message publishing one only method "EncryptedData".
 		/// Projects using WS-Security should log the actual methdod called deeper in the processing pipeline
 		/// </summary>
@@ -176,7 +174,7 @@ public class Soap12Tests
 		[InlineData(Sample2, "GetStockPrice")]
 		[InlineData(SampleWithComments, "GetStockPrice")]
 		[InlineData(SoapSampleOnlyBody, "GetStockPrice")]
-		[InlineData(SoapWithWsSecurity, "EncryptedData")] //special  
+		[InlineData(SoapWithWsSecurity, "EncryptedData")] //special
 		[InlineData(PartialMessage, "GetStockPrice")]
 		[InlineData(NotSoap, null)]
 		[InlineData(NotXml, null)]
@@ -186,7 +184,7 @@ public class Soap12Tests
 		public void Soap12Parser_ParsesHeaderAndBody(string soap, string expectedAction)
 		{
 			var requestStream = new MemoryStream(Encoding.UTF8.GetBytes(soap));
-			var action = SoapRequest.GetSoap12ActionInternal(requestStream);
+			var action = SoapRequest.GetSoap12ActionFromInputStream(requestStream);
 
 			action.Should().Be(expectedAction);
 		}

test/Elastic.Apm.AspNetFullFramework.Tests/Soap/SoapRequestTests.cs

@@ -0,0 +1,64 @@
+// Licensed to Elasticsearch B.V under
+// one or more agreements.
+// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
+// See the LICENSE file in the project root for more information
+
+using System.Linq;
+using System.Net.Http;
+using System.Net.Http.Headers;
+using System.Text;
+using System.Threading.Tasks;
+using FluentAssertions;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Elastic.Apm.AspNetFullFramework.Tests.Soap
+{
+	[Collection(Consts.AspNetFullFrameworkTestsCollection)]
+	public class SoapRequestTests : TestsBase
+	{
+		public SoapRequestTests(ITestOutputHelper xUnitOutputHelper)
+			: base(xUnitOutputHelper) { }
+
+		/// <summary>
+		/// Tests that the reading of the input stream to get the action name for a SOAP 1.2 request
+		/// does not cause an exception to be thrown when the framework deserializes the input stream
+		/// to parse the parameters for the web method.
+		/// </summary>
+		[AspNetFullFrameworkFact]
+		public async Task Name_Should_Should_Not_Throw_Exception_When_Asmx_Soap12_Request()
+		{
+			var pathData = SampleAppUrlPaths.CallSoapServiceProtocolV12;
+			var action = "Input";
+
+			var input = @"This is the input";
+			var request = new HttpRequestMessage(HttpMethod.Post, pathData.Uri)
+			{
+				Content = new StringContent($@"<?xml version=""1.0"" encoding=""utf-8""?>
+				<soap12:Envelope xmlns:xsi=""http://www.w3.org/2001/XMLSchema-instance"" xmlns:xsd=""http://www.w3.org/2001/XMLSchema"" xmlns:soap12=""http://www.w3.org/2003/05/soap-envelope"">
+					<soap12:Body>
+					<{action} xmlns=""http://tempuri.org/"">
+						<input>{input}</input>
+					</{action}>
+					</soap12:Body>
+				</soap12:Envelope>", Encoding.UTF8, "application/soap+xml")
+			};
+
+			request.Headers.Accept.Clear();
+			request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*"));
+
+			var response = await HttpClient.SendAsync(request);
+			response.IsSuccessStatusCode.Should().BeTrue();
+
+			var responseText = await response.Content.ReadAsStringAsync();
+			responseText.Should().Contain(input);
+
+			await WaitAndCustomVerifyReceivedData(receivedData =>
+			{
+				receivedData.Transactions.Count.Should().Be(1);
+				var transaction = receivedData.Transactions.First();
+				transaction.Name.Should().Be($"POST {pathData.Uri.AbsolutePath} {action}");
+			});
+		}
+	}
+}

test/Elastic.Apm.AspNetFullFramework.Tests/WebApiTests.cs

@@ -0,0 +1,43 @@
+// Licensed to Elasticsearch B.V under
+// one or more agreements.
+// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
+// See the LICENSE file in the project root for more information
+
+using System.IO;
+using System.Net.Http;
+using System.Net.Http.Headers;
+using System.Text;
+using System.Threading.Tasks;
+using Elastic.Apm.Tests.TestHelpers;
+using FluentAssertions;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Elastic.Apm.AspNetFullFramework.Tests
+{
+	[Collection(Consts.AspNetFullFrameworkTestsCollection)]
+	public class WebApiTests : TestsBase
+	{
+		public WebApiTests(ITestOutputHelper xUnitOutputHelper) : base(xUnitOutputHelper) { }
+
+		// https://github.com/elastic/apm-agent-dotnet/issues/1113
+		[AspNetFullFrameworkFact]
+		public async Task MultipartData_Should_Not_Throw()
+		{
+			var pathData = SampleAppUrlPaths.WebApiPage;
+			using var request = new HttpRequestMessage(HttpMethod.Post, pathData.Uri);
+
+			using var plainInputTempFile = TempFile.CreateWithContents("this is plain input");
+			using var jsonTempFile = TempFile.CreateWithContents("{\"input\":\"this is json input\"}");
+			using var multiPartContent = new MultipartFormDataContent
+			{
+				{ new StreamContent(new FileStream(plainInputTempFile.Path, FileMode.Open, FileAccess.Read)), "plain", "plain" },
+				{ new StreamContent(new FileStream(jsonTempFile.Path, FileMode.Open, FileAccess.Read)), "json", "json" },
+			};
+
+			request.Content = multiPartContent;
+			using var response = await HttpClient.SendAsync(request).ConfigureAwait(false);
+			response.IsSuccessStatusCode.Should().BeTrue();
+		}
+	}
+}