Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use RecyclableMemoryStream #16949

Merged
merged 23 commits into from
Nov 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
See https://github.com/OrchardCMS/OrchardCore/pull/16057 for more information.
-->
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.1.0" />
<PackageVersion Include="Microsoft.IO.RecyclableMemoryStream" Version="3.0.1" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
<PackageVersion Include="MimeKit" Version="4.8.0" />
<PackageVersion Include="MiniProfiler.AspNetCore.Mvc" Version="4.3.8" />
Expand Down
44 changes: 24 additions & 20 deletions src/OrchardCore.Modules/OrchardCore.Media/Recipes/MediaStep.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Text;
using System.Text.Json.Nodes;
using Microsoft.Extensions.FileProviders;
using Microsoft.Extensions.Localization;
Expand Down Expand Up @@ -46,35 +47,38 @@ protected override async Task HandleAsync(RecipeExecutionContext context)

Stream stream = null;

if (!string.IsNullOrWhiteSpace(file.Base64))
try
{
stream = new MemoryStream(Convert.FromBase64String(file.Base64));
}
else if (!string.IsNullOrWhiteSpace(file.SourcePath))
{
var fileInfo = context.RecipeDescriptor.FileProvider.GetRelativeFileInfo(context.RecipeDescriptor.BasePath, file.SourcePath);
if (!string.IsNullOrWhiteSpace(file.Base64))
{
stream = Base64.DecodedToStream(file.Base64);
}
else if (!string.IsNullOrWhiteSpace(file.SourcePath))
{
var fileInfo = context.RecipeDescriptor.FileProvider.GetRelativeFileInfo(context.RecipeDescriptor.BasePath, file.SourcePath);

stream = fileInfo.CreateReadStream();
}
else if (!string.IsNullOrWhiteSpace(file.SourceUrl))
{
var httpClient = _httpClientFactory.CreateClient();
stream = fileInfo.CreateReadStream();
}
else if (!string.IsNullOrWhiteSpace(file.SourceUrl))
{
var httpClient = _httpClientFactory.CreateClient();

var response = await httpClient.GetAsync(file.SourceUrl);
var response = await httpClient.GetAsync(file.SourceUrl);

if (response.IsSuccessStatusCode)
{
stream = await response.Content.ReadAsStreamAsync();
if (response.IsSuccessStatusCode)
{
stream = await response.Content.ReadAsStreamAsync();
}
}
}

if (stream != null)
{
try
if (stream != null)
{
await _mediaFileStore.CreateFileFromStreamAsync(file.TargetPath, stream, true);
}
finally
}
finally
{
if (stream != null)
{
await stream.DisposeAsync();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Diagnostics;
using System.Text;
using System.Text.Json;
using Dapper;
using Fluid;
Expand Down Expand Up @@ -43,7 +44,10 @@ public AdminController(
[Admin("Queries/Sql/Query", "QueriesRunSql")]
public Task<IActionResult> Query(string query)
{
query = string.IsNullOrWhiteSpace(query) ? "" : System.Text.Encoding.UTF8.GetString(Convert.FromBase64String(query));
query = string.IsNullOrWhiteSpace(query)
? ""
: Base64.FromUTF8Base64String(query);

return Query(new AdminQueryViewModel
{
DecodedQuery = query,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics;
using System.Globalization;
using System.Text;
using System.Text.Encodings.Web;
using System.Text.Json;
using System.Text.Json.Nodes;
Expand Down Expand Up @@ -466,7 +467,9 @@ public async Task<IActionResult> Query(string indexName, string query)
return await Query(new AdminQueryViewModel
{
IndexName = indexName,
DecodedQuery = string.IsNullOrWhiteSpace(query) ? string.Empty : System.Text.Encoding.UTF8.GetString(Convert.FromBase64String(query))
DecodedQuery = string.IsNullOrWhiteSpace(query)
? string.Empty
: Base64.FromUTF8Base64String(query)
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics;
using System.Globalization;
using System.Text;
using System.Text.Encodings.Web;
using System.Text.Json;
using System.Text.Json.Nodes;
Expand Down Expand Up @@ -355,7 +356,10 @@ public async Task<ActionResult> Delete(LuceneIndexSettingsViewModel model)

public Task<IActionResult> Query(string indexName, string query)
{
query = string.IsNullOrWhiteSpace(query) ? "" : System.Text.Encoding.UTF8.GetString(Convert.FromBase64String(query));
query = string.IsNullOrWhiteSpace(query)
? ""
: Base64.FromUTF8Base64String(query);

return Query(new AdminQueryViewModel { IndexName = indexName, DecodedQuery = query });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public async Task<IActionResult> Index(string sitemapId, CancellationToken cance

document.Declaration = new XDeclaration("1.0", "utf-8", null);

var stream = new MemoryStream();
using var stream = MemoryStreamFactory.GetStream();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not using the Site Map feature, so I can't say whether this is actually a problem, but this looks wrong. With this change, the stream is disposed as it's being returned for use later (by the File result).

Copy link
Member Author

@MikeAlhayek MikeAlhayek Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvarblow it look suspicious to me too. Feel free to test it out, and submit an issue and PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is indeed a problem. I am fixing it.

await document.SaveAsync(stream, SaveOptions.None, cancellationToken);

if (stream.Length >= ErrorLength)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public async Task<IActionResult> ResetPasswordPOST()

if (ModelState.IsValid)
{
var token = Encoding.UTF8.GetString(Convert.FromBase64String(model.ResetToken));
var token = Base64.FromUTF8Base64String(model.ResetToken);

if (await _userService.ResetPasswordAsync(model.UsernameOrEmail, token, model.NewPassword, ModelState.AddModelError))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,15 @@ public async Task<IActionResult> ServiceEndpoint([ModelBinder(BinderType = typeo
};

// Save to an intermediate MemoryStream to preserve the encoding declaration.
using var stream = new MemoryStream();
using var stream = MemoryStreamFactory.GetStream();
using (var w = XmlWriter.Create(stream, settings))
{
var result = _writer.MapMethodResponse(methodResponse);
result.Save(w);
}

var content = Encoding.UTF8.GetString(stream.ToArray());
var content = Encoding.UTF8.GetString(stream.GetBuffer(), 0, (int)stream.Length);

return Content(content, "text/xml");
}

Expand Down
68 changes: 68 additions & 0 deletions src/OrchardCore/OrchardCore.Abstractions/Base64.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
using System.Text;

namespace OrchardCore;

public static class Base64
{
/// <summary>
/// Converts a base64 encoded UTF8 string to the original value.
/// </summary>
/// <param name="base64">The base64 encoded string.</param>
/// <returns>The decoded string.</returns>
/// <remarks>This method is equivalent to <c>Encoding.UTF8.GetString(Convert.FromBase64String(base64))</c> but uses a buffer pool to decode the string.</remarks>
public static string FromUTF8Base64String(string base64)
{
ArgumentNullException.ThrowIfNull(base64);

// Due to padding the deserialized buffer could be smaller than this value.
var maxBufferLength = GetDeserializedBase64Length(base64.Length);

using var memoryStream = MemoryStreamFactory.GetStream(maxBufferLength);
var span = memoryStream.GetSpan(maxBufferLength);

if (!Convert.TryFromBase64String(base64, span, out var bytesWritten))
{
throw new FormatException("Invalid Base64 string.");
}

return Encoding.UTF8.GetString(span.Slice(0, bytesWritten));
}

/// <summary>
/// Converts a base64 encoded string to a stream.
/// </summary>
/// <param name="base64">The base64 encoded string.</param>
/// <remarks>The resulting <see cref="Stream"/> should be disposed once used.</remarks>
/// <returns>The decoded stream.</returns>
/// <exception cref="FormatException"></exception>
public static Stream DecodedToStream(string base64)
{
ArgumentNullException.ThrowIfNull(base64);

// Due to padding the deserialized buffer could be smaller than this value.
var maxBufferLength = GetDeserializedBase64Length(base64.Length);

var memoryStream = MemoryStreamFactory.GetStream(maxBufferLength);
var span = memoryStream.GetSpan(maxBufferLength);

if (!Convert.TryFromBase64String(base64, span, out var bytesWritten))
{
throw new FormatException("Invalid Base64 string.");
}

memoryStream.Advance(bytesWritten);

return memoryStream;
}

/// <summary>
/// Gets the maximum buffer length required to decode a base64 string.
/// </summary>
/// <param name="base64Length">The length value to decode.</param>
/// <returns>The size of the decoded buffer.</returns>
public static int GetDeserializedBase64Length(int base64Length)
{
// Do the multiplication first to prevent precision loss.
return base64Length * 3 / 4;
}
}
25 changes: 25 additions & 0 deletions src/OrchardCore/OrchardCore.Abstractions/MemoryStreamFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
using Microsoft.IO;

namespace OrchardCore;

public static class MemoryStreamFactory
{
private static readonly RecyclableMemoryStreamManager _manager = new();

static MemoryStreamFactory()
{
var options = new RecyclableMemoryStreamManager.Options
{
BlockSize = 4 * 1024, // 4 KB
AggressiveBufferReturn = true
};

_manager = new RecyclableMemoryStreamManager(options);
}

public static RecyclableMemoryStream GetStream(string tag = null)
=> _manager.GetStream(tag);

public static RecyclableMemoryStream GetStream(int requiredSize, string tag = null)
=> _manager.GetStream(tag, requiredSize);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

<ItemGroup>
<PackageReference Include="JsonPath.Net" />
<PackageReference Include="Microsoft.IO.RecyclableMemoryStream" />
<PackageReference Include="ZString" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,17 @@ public class BlobFileStore : IFileStore
private readonly IClock _clock;
private readonly BlobContainerClient _blobContainer;
private readonly IContentTypeProvider _contentTypeProvider;

private readonly string _basePrefix;

public BlobFileStore(BlobStorageOptions options, IClock clock, IContentTypeProvider contentTypeProvider)
public BlobFileStore(
BlobStorageOptions options,
IClock clock,
IContentTypeProvider contentTypeProvider)
{
_options = options;
_clock = clock;
_contentTypeProvider = contentTypeProvider;

_blobContainer = new BlobContainerClient(_options.ConnectionString, _options.ContainerName);

if (!string.IsNullOrEmpty(_options.BasePath))
Expand Down Expand Up @@ -436,6 +439,7 @@ private async Task CreateDirectoryAsync(string path)

// Create a directory marker file to make this directory appear when listing directories.
using var stream = new MemoryStream(MarkerFileContent);

await placeholderBlob.UploadAsync(stream);
}

Expand Down
Loading