-
Notifications
You must be signed in to change notification settings - Fork 2k
upgrade(touchsocket):nuget version to 4.0.2 #10367
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
base: master
Are you sure you want to change the base?
Conversation
重构MyTcpSessionClientBase类以提升性能与可读性: - 将Json和Plaintext字段从public改为private - 优化pipeReader.ReadAsync与pipeWriter.FlushAsync的异步调用逻辑 - 为关键方法添加AggressiveOptimization标记以启用编译优化 - 简化TryReadLine方法的行尾标记处理逻辑 - 优化ParseUrlFast方法的URL解析逻辑,减少冗余代码 - 重构WriteResponseSync方法,使用if-else替代switch语句 - 删除冗余注释与代码,提升代码整洁度
升级TouchSocket相关包版本以使用最新功能和修复 - 在 `TouchSocketHttp.csproj` 中,将 `TouchSocket.WebApi` 替换为 `TouchSocket.Http`,并将版本更新至 `4.0.2` - 在 `TouchSocketHttpPlatform.csproj` 中,将 `TouchSocket` 包版本更新至 `4.0.2` - 在 `TouchSocketWebApi.csproj` 中: - 将 `TouchSocket.Hosting` 包版本更新至 `4.0.2` - 将 `TouchSocket.WebApi` 包版本更新至 `4.0.2`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR upgrades the TouchSocket NuGet packages from version 4.0.0 to 4.0.2 across multiple project files and includes significant refactoring of the HTTP request parsing logic in the MyTcpSessionClientBase class. The changes aim to improve performance through optimized async patterns and more efficient parsing algorithms.
Key Changes:
- Updated TouchSocket package versions from 4.0.0 to 4.0.2 across 4 .csproj files
- Corrected package reference in TouchSocketHttp.csproj from TouchSocket.WebApi to TouchSocket.Http
- Refactored HTTP parsing logic with performance optimizations including improved async/await patterns and streamlined URL parsing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| TouchSocketWebApi.csproj | Upgraded TouchSocket.Hosting and TouchSocket.WebApi packages to version 4.0.2 |
| TouchSocketHttpPlatform.csproj | Upgraded TouchSocket package to version 4.0.2 |
| TouchSocketHttp.csproj | Corrected package reference from TouchSocket.WebApi to TouchSocket.Http and upgraded to version 4.0.2 |
| MyTcpSessionClientBase.cs | Refactored HTTP parsing with visibility changes, async optimizations, and rewritten TryReadLine/ParseUrlFast methods (contains critical bugs) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var start = reader.Consumed; | ||
|
|
||
| if (!reader.TryPeek(1, out var next)) | ||
| { | ||
| if (b == '\r') | ||
| { | ||
| // 查看是否有'\n',若暂不可用则回退并判定不完整 | ||
| if (!reader.TryPeek(out var next)) | ||
| { | ||
| // 回退已读取的'\r' | ||
| reader.Rewind(1); | ||
| length = 0; | ||
| return false; | ||
| } | ||
| if (next == '\n') | ||
| { | ||
| // 消费'\n'并返回本行长度(不含CRLF) | ||
| reader.Advance(1); | ||
| length = reader.Consumed - start - 2; | ||
| return true; | ||
| } | ||
| } | ||
| length = 0; | ||
| return false; | ||
| } | ||
|
|
||
| if (next == '\n') | ||
| { | ||
| var lineLength = reader.Consumed; | ||
| reader.Advance(2); | ||
| length = lineLength - start; |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TryReadLine method has a logic error. On line 192, start is set to reader.Consumed after already advancing to '\r'. Then on line 202, lineLength is set to the same value (reader.Consumed hasn't changed). On line 204, length = lineLength - start will always be 0, which is incorrect.
The correct logic should capture the position before advancing to '\r', or calculate the length differently. The line length should represent the number of bytes in the line excluding CRLF.
| reader.Advance(1); | ||
| return TryReadLine(ref reader, out length); |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recursive call to TryReadLine on line 209 can lead to stack overflow for malformed input. If the input contains many '\r' characters not followed by '\n', this will recursively call itself multiple times. Consider using a loop instead of recursion to handle cases where '\r' is not followed by '\n'.
| var urlSlice = sequence.Slice(urlStart, urlLength); | ||
|
|
||
| if (urlSlice.IsSingleSegment) | ||
| { | ||
| if (urlSlice.FirstSpan.SequenceEqual(Plaintext)) | ||
| { | ||
| return RouteType.Plaintext; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| Span<byte> tmp = stackalloc byte[(int)urlLength]; | ||
| urlSlice.CopyTo(tmp); | ||
| if (tmp.SequenceEqual(Plaintext)) | ||
| { | ||
| return RouteType.Plaintext; | ||
| } | ||
| return urlSlice.FirstSpan.SequenceEqual(Plaintext) ? RouteType.Plaintext : RouteType.Unknown; | ||
| } | ||
|
|
||
| Span<byte> tmp = stackalloc byte[(int)urlLength]; | ||
| urlSlice.CopyTo(tmp); | ||
| return tmp.SequenceEqual(Plaintext) ? RouteType.Plaintext : RouteType.Unknown; | ||
| } | ||
| // "/json" | ||
| else if (urlLength == Json.Length) | ||
|
|
||
| if (urlLength == Json.Length) | ||
| { | ||
| var urlSlice = sequence.Slice(urlStart, urlLength); |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ParseUrlFast, the urlSlice variable is created inside each length-check block (lines 237 and 251), but it's created using positions relative to the original sequence. The urlStart value comes from reader.Consumed (line 226), which is the position AFTER advancing past the first space. However, when calling sequence.Slice(urlStart, urlLength), this should work correctly if urlStart represents the absolute position in the sequence. Consider verifying that sequence.Slice() is being called with the correct position, as SequenceReader.Consumed returns the total bytes consumed, not a SequencePosition.
| if (readTask.IsCompleted) | ||
| { | ||
| readResult = readTask.Result; | ||
| } | ||
| else | ||
| { | ||
| readResult = await readTask.ConfigureAwait(false); | ||
| } |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (readTask.IsCompleted) | |
| { | |
| readResult = readTask.Result; | |
| } | |
| else | |
| { | |
| readResult = await readTask.ConfigureAwait(false); | |
| } | |
| readResult = await readTask.ConfigureAwait(false); |
No description provided.