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

ComputeComponentTypeNameHash does not handle multi-byte type names correctly #50879

Closed
1 task done
vcsjones opened this issue Sep 22, 2023 · 2 comments · Fixed by #52232
Closed
1 task done

ComputeComponentTypeNameHash does not handle multi-byte type names correctly #50879

vcsjones opened this issue Sep 22, 2023 · 2 comments · Fixed by #52232
Assignees
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug.
Milestone

Comments

@vcsjones
Copy link
Member

vcsjones commented Sep 22, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

👋 I was looking though the use of cryptography in ASP.NET Core, and stumbled on this code block:

var typeNameLength = typeName.Length;
var typeNameBytes = typeNameLength < 1024
? stackalloc byte[typeNameLength]
: new byte[typeNameLength];
Encoding.UTF8.GetBytes(typeName, typeNameBytes);
Span<byte> typeNameHashBytes = stackalloc byte[SHA1.HashSizeInBytes];

Nothing about the use of cryptography there concerns me too much, but I did want to point out that it does not handle types where the type name requires the use of multibyte characters.

I lack detailed knowledge of how SSR works, so I had a bit of trouble trying to reproduce it in a real app.

Expected Behavior

No response

Steps To Reproduce

I had a bit of trouble trying to reproduce this using an app, as I am not super familiar with the innards of SSR. However, in isolation it can be reproduced with:

using System;
using System.Text;
using System.Security.Cryptography;

string hash = ComputeComponentTypeNameHash(typeof(こんにちは));
Console.WriteLine(hash);

static string ComputeComponentTypeNameHash(Type componentType)
{
    if (componentType.FullName is not { } typeName)
    {
        throw new InvalidOperationException($"An invalid component type was used.");
    }

    var typeNameLength = typeName.Length;
    var typeNameBytes = typeNameLength < 1024
        ? stackalloc byte[typeNameLength]
        : new byte[typeNameLength];

    Encoding.UTF8.GetBytes(typeName, typeNameBytes);

    Span<byte> typeNameHashBytes = stackalloc byte[SHA1.HashSizeInBytes];
    SHA1.HashData(typeNameBytes, typeNameHashBytes);

    return Convert.ToHexString(typeNameHashBytes);
}

class こんにちは
{

}

Exceptions (if any)

This will produce a System.ArgumentException since the destination buffer is too small.

.NET Version

No response

Anything else?

Please feel free to close this issue if the type in ComputeComponentTypeNameHash is not exposed to developers.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Sep 22, 2023
@mkArtakMSFT mkArtakMSFT added area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug. and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Sep 22, 2023
@mkArtakMSFT mkArtakMSFT added this to the 8.0 milestone Sep 26, 2023
@vcsjones
Copy link
Member Author

vcsjones commented Oct 2, 2023

If this is indeed a but then I would propose the change look something like this:

static string ComputeComponentTypeNameHash(Type componentType)
{
    if (componentType.FullName is not { } typeName)
    {
        throw new InvalidOperationException($"An invalid component type was used.");
    }

    const int MaxStackBuffer = 1024;
    var maxTypeNameLength = Encoding.UTF8.GetMaxByteCount(typeName.Length);
    var typeNameBytes = maxTypeNameLength <= MaxStackBuffer
        ? stackalloc byte[MaxStackBuffer]
        : new byte[maxTypeNameLength];

    int written = Encoding.UTF8.GetBytes(typeName, typeNameBytes);

    Span<byte> typeNameHashBytes = stackalloc byte[SHA1.HashSizeInBytes];
    SHA1.HashData(typeNameBytes[..written], typeNameHashBytes);

    return Convert.ToHexString(typeNameHashBytes);
}

If you are targeting .NET 8 you can write this, which will have better performance for the "full type name is not more than 1024 bytes" case which seems typical.

static string ComputeComponentTypeNameHash(Type componentType)
{
    if (componentType.FullName is not { } typeName)
    {
        throw new InvalidOperationException($"An invalid component type was used.");
    }

    Span<byte> typeNameBytes = stackalloc byte[1024];
    int written;

    if (!Encoding.UTF8.TryGetBytes(typeName, typeNameBytes, out written))
    {
        typeNameBytes = Encoding.UTF8.GetBytes(typeName);
        written = typeNameBytes.Length;
    }
    Span<byte> typeNameHashBytes = stackalloc byte[SHA1.HashSizeInBytes];
    SHA1.HashData(typeNameBytes[..written], typeNameHashBytes);

    return Convert.ToHexString(typeNameHashBytes);
}

@wtgodbe wtgodbe modified the milestones: 8.0, 8.0.0 Oct 3, 2023
@mkArtakMSFT mkArtakMSFT modified the milestones: 8.0.0, Backlog Oct 4, 2023
@ghost
Copy link

ghost commented Oct 4, 2023

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, BlazorPlanning Nov 5, 2023
@mkArtakMSFT mkArtakMSFT modified the milestones: Planning: WebUI, 8.0.x Nov 20, 2023
mkArtakMSFT pushed a commit that referenced this issue Nov 27, 2023
…yte characters (#52316)

Backport of #52232 to release/8.0

/cc @MackinnonBuck

# [Blazor] Fix type name hashing when the type has multibyte characters

Fixes an issue where an exception gets thrown if a render mode boundary component has a type whose full name contains multibyte characters.

## Description

The bug results in an exception getting thrown if the type of a component with a render mode has a full name containing multibyte characters. It especially affects cases where a component (or the namespace it's defined in) contains non-Latin characters.

This PR fixes the issue by allocating a constant-sized stack buffer and falling back to a heap-allocated buffer when the type name is too long.

Fixes #50879
Fixes #52109

## Customer Impact

Blazor Apps with non-Latin code might not be able to use interactivity. The workaround is to ensure that the full name of any component serving as a render mode boundary does not contain multibyte characters.

## Regression?

- [ ] Yes
- [X] No

Render modes are a new feature in .NET 8, so this bug is not a regression.

## Risk

- [ ] High
- [ ] Medium
- [X] Low

The fix is straightforward and we have new automated tests for this scenario.

## Verification

- [X] Manual (required)
- [X] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [X] N/A

---------

Co-authored-by: Mackinnon Buck <[email protected]>
@mkArtakMSFT mkArtakMSFT modified the milestones: 8.0.x, 8.0.1 Nov 27, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components bug This issue describes a behavior which is not expected - a bug.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants