Delete WebHeaderEncoding.cs and replace usages with Encoding.Latin1#125592
Delete WebHeaderEncoding.cs and replace usages with Encoding.Latin1#125592
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes unsafe pointer-based implementations from WebHeaderEncoding by rewriting the byte↔char conversion paths using Span/ReadOnlySpan, keeping the existing loop unrolling pattern for GetString.
Changes:
- Replaced unsafe pointer loop in
WebHeaderEncoding.GetStringwith a span-based unrolledforloop. - Replaced unsafe pointer loop in
WebHeaderEncoding.GetByteswith span slicing + indexed copy. - Made the
string.Createlambdastaticto avoid captures.
|
I would consider deleting this file while you're at it. Its stated justification for existence seems to be outdated / wrong runtime/src/libraries/Common/src/System/Net/WebHeaderEncoding.cs Lines 9 to 14 in c38f37a Switching it to Latin1 would only change behavior if the inputs weren't already in the [0, 255] range, in which case we'd already be corrupting data. |
@copilot please address this feedback |
Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Deleted Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@MihaZupan is that what you meant? Seems to be passing CI |
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
MihaZupan
left a comment
MihaZupan
left a comment
There was a problem hiding this comment.
is that what you meant?
Yes, thank you.
Looks like we even had a test for "I want this particular flavor of data corruption" 😄
|
/ba-g "Build osx-x64 Debug Mono_MiniJIT_LibrariesTests is stuck (10h)" |
Deletes
WebHeaderEncoding.csentirely — its stated justification (Latin-1 being slow for byte→string, best-fit-mapping issues for string→byte) is outdated. All call sites now useEncoding.Latin1directly.Description
The
WebHeaderEncodingclass provided custom unsafe Latin-1 byte↔char conversion helpers. As noted in review,Encoding.Latin1handles both directions correctly today, making the custom implementation unnecessary.Deleted
src/libraries/Common/src/System/Net/WebHeaderEncoding.csand its<Compile>reference fromSystem.Net.HttpListener.csproj.Replaced all call sites across 3 files:
HttpListener.Windows.cs:WebHeaderEncoding.GetString(bytes, 0, bytes.Length)→Encoding.Latin1.GetString(bytes)HttpListenerResponse.Windows.cs: CollapsedGetByteCount+new byte[]+GetBytesinto singleEncoding.Latin1.GetBytes(str)calls (5 occurrences)HttpListenerResponse.Managed.cs:WebHeaderEncoding.GetBytes(str)→Encoding.Latin1.GetBytes(str)Updated test: For chars > U+00FF (e.g. U+1234),
Encoding.Latin1produces?(replacement character) instead of truncating to the low byte. This only affects already-invalid inputs that were silently corrupting data before.AllowUnsafeBlocksremains in the HttpListener.csproj— other files in the project still require it.Original prompt
remove unsafe code from WebHeaderEncoding using local changes.
[Chronological Review] 1. User requested removal of unsafe code from WebHeaderEncoding.cs, initially asking for Vector128.Widen-based replacement 2. Explored the file, found two unsafe methods: GetString (byte→char with unrolled-by-8 pointer loop) and GetBytes (char→byte with pointer loop) 3. Found the file is only consumed by System.Net.HttpListener project 4. Implemented Vector128.Widen/Narrow replacement with MemoryMarshal.Cast and Vector128.Create/CopyTo on Spans 5. Build failed - missing System.Runtime.Intrinsics reference in csproj 6. Added ProjectReference for System.Runtime.Intrinsics to HttpListener csproj 7. Build succeeded, all 693 tests passed 8. Created a benchmark file for performance validation 9. User reverted changes and asked for simpler approach: plain Span access without MemoryMarshal or Unsafe.X, keeping same 8-per-iteration unrolling 10. Implemented simple span-based version with while loop unrolled by 8 11. Build succeeded, all 693 tests passed 12. User asked to convert the while loop into a for loop 13. Converted while to for loop with lastBlockStart = source.Length - 7[Intent Mapping]
[Technical Inventory]
[Code Archaeology]
[Progress Assessment]
[Context Validation]
[Recent Commands Analysis]
1. Conversation Overview: - Primary Objectives: User requested: "please remove unsafe code from C:\prj\runtime-main2\src\libraries\Common\src\System\Net\WebHeaderEncoding.cs". Initially wanted Vector128.Widen replacement, then revised to: "just convert to just plain Span access (without MemoryMarshal or Unsafe.X) same 8 per iteration". Final request: "please convert that while into for loop". - Session Context: Started with ambitious Vector128 SIMD approach, hit build issues (missing Intrinsics reference), resolved them, user reverted and asked for simpler plain-Span approach, then refined loop style. - User Intent Evolution: Vector128+MemoryMarshal → plain Span only → while→for loop refinement-
- Repository: dotnet/runtime at C:\prj\runtime-main2, targeting .NET 11 (SDK 11.0.100-preview.3.26161.119)
- Build: Must use
- Build command:
- Test command:
- The file is a Common shared source file linked via
-
- Purpose: Latin-1 byte↔char conversion helper for HTTP headers (1:1 mapping U+0000-U+00FF ↔ 0x00-0xFF)
- Current State: All unsafe code removed. Uses plain Span indexing. Last edit converted while→for loop but has not been build-tested after this final edit.
- Key methods:
- Current code uses:
- `src/librarie...
Technical Foundation:
.\build.cmdnotdotnet build(global.json requires preview SDK not installed globally).\build.cmd -subset libs.ref+libs.src -project <csproj>.\build.cmd -subset libs.tests -project <test-csproj> -test<Compile Include="$(CommonPath)System\Net\WebHeaderEncoding.cs" />— only consumed by System.Net.HttpListenerCodebase Status:
src/libraries/Common/src/System/Net/WebHeaderEncoding.cs:GetString(byte[], int, int)→ string (unrolled-by-8 for loop over Spans),GetBytes(string, int, int, byte[], int)→ void (simple span for loop),GetBytes(string)→ byte[],GetByteCount(string)→ intReadOnlySpan<byte> source = state.bytes.AsSpan(state.byteIndex, buffer.Length)withint lastBlockStart = source.Length - 7andfor (; i < lastBlockStart; i += 8)unrolled patternCreated from Copilot CLI via the copilot delegate command.
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.