fix(proxy): 代理字段解析遗漏一种格式 - #3495
Open
tangge233 wants to merge 1 commit into
Open
Conversation
pcl-ce-automation
Bot
added
🛠️ 等待审查
Contributor
Reviewer's Guide重构代理字符串解析，以支持更多格式并改进校验，将逻辑集中到可复用的辅助方法中，并更加严格地控制系统 HTTP 代理的应用。 更新后的系统代理应用顺序图sequenceDiagram
participant HttpProxyManager
participant _GetProxyFromString
participant _ParseKeyValueSegment
participant _ParseAddress
participant _systemWebProxy
HttpProxyManager->>_GetProxyFromString: _GetProxyFromString(systemProxyString)
alt proxyString contains =
_GetProxyFromString->>_ParseKeyValueSegment: _ParseKeyValueSegment(segment)
_ParseKeyValueSegment->>_ParseAddress: _ParseAddress(address, protocol)
_ParseAddress-->>_GetProxyFromString: ProxyItem
else proxyString without =
_GetProxyFromString->>_ParseAddress: _ParseAddress(proxyString.Trim())
_ParseAddress-->>_GetProxyFromString: ProxyItem or null
end
HttpProxyManager->>HttpProxyManager: selectedProxy = FirstOrDefault(Protocol == Http)
HttpProxyManager->>_systemWebProxy: set Address based on selectedProxy and isSystemProxyEnabled
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideRefactors proxy string parsing to support additional formats and improve validation, centralizing the logic into reusable helpers and tightening application of the system HTTP proxy. Sequence diagram for updated system proxy applicationsequenceDiagram
participant HttpProxyManager
participant _GetProxyFromString
participant _ParseKeyValueSegment
participant _ParseAddress
participant _systemWebProxy
HttpProxyManager->>_GetProxyFromString: _GetProxyFromString(systemProxyString)
alt proxyString contains =
_GetProxyFromString->>_ParseKeyValueSegment: _ParseKeyValueSegment(segment)
_ParseKeyValueSegment->>_ParseAddress: _ParseAddress(address, protocol)
_ParseAddress-->>_GetProxyFromString: ProxyItem
else proxyString without =
_GetProxyFromString->>_ParseAddress: _ParseAddress(proxyString.Trim())
_ParseAddress-->>_GetProxyFromString: ProxyItem or null
end
HttpProxyManager->>HttpProxyManager: selectedProxy = FirstOrDefault(Protocol == Http)
HttpProxyManager->>_systemWebProxy: set Address based on selectedProxy and isSystemProxyEnabled
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 在
RefreshSystemProxy中，当没有找到 HTTP 代理时，你不再将isSystemProxyEnabled重置为 0；如果其他逻辑依赖这个标志来准确反映有效的 HTTP 代理可用性，建议考虑保留之前的行为，或者进一步澄清它的语义。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 在 `RefreshSystemProxy` 中，当没有找到 HTTP 代理时，你不再将 `isSystemProxyEnabled` 重置为 0；如果其他逻辑依赖这个标志来准确反映有效的 HTTP 代理可用性，建议考虑保留之前的行为，或者进一步澄清它的语义。
## Individual Comments
### Comment 1
<location path="PCL.Core/IO/Net/Http/HttpProxyManager.cs" line_range="90-93" />
<code_context>
- }
- else
+ // 能解析出主机名的地址，规范化为 host:port
+ if (Uri.TryCreate(address, new UriCreationOptions(), out var uri) && !uri.Host.IsNullOrEmpty())
{
- ret.Add(new ProxyItem { Protocol = ProxyProtocol.Http, Address = proxyString.Trim() });
+ var hostPort = uri.Port > 0 ? $"{uri.Host}:{uri.Port}" : uri.Host;
+ return new ProxyItem(protocol ?? _ParseProtocol(uri.Scheme), hostPort);
}
</code_context>
<issue_to_address>
**suggestion:** 当 URI 没有主机名时（例如文件/系统 URI），这种行为对代理而言可能并不理想。
当 `Uri.TryCreate` 成功但 `Host` 为空时，当前代码会退回到将 `address` 视为原始代理端点。对于代理配置来说，像 `file:///c:/path` 这样的值在实际效果上是无效的，但仍会被通过，并在后续用于构建 HTTP URI。为了避免这种错误配置传播到 HTTP 栈，作为代理而言，对主机名为空的 URI 进行拒绝（例如返回 `null`）会更安全。
建议实现如下：
```csharp
// 能解析出主机名的地址，规范化为 host:port
private sealed record ProxyItem(ProxyProtocol Protocol, string Address);
// 返回 ProxyItem?，以便在 URI 有效但没有主机名时能够返回 null
private static ProxyItem? _ParseProxyItem(string address, ProxyProtocol? protocol = null)
{
// 能解析出主机名的地址，规范化为 host:port
if (Uri.TryCreate(address, new UriCreationOptions(), out var uri))
{
if (uri.Host.IsNullOrEmpty())
{
// 对于没有主机名的 URI（例如 file://），认为是无效的代理配置
return null;
}
var hostPort = uri.Port > 0 ? $"{uri.Host}:{uri.Port}" : uri.Host;
return new ProxyItem(protocol ?? _ParseProtocol(uri.Scheme), hostPort);
}
// 非 URI 格式的地址，按原始代理端点处理
return new ProxyItem(protocol ?? ProxyProtocol.Http, address.Trim());
}
private static ProxyItem[] _GetProxyFromString(string? proxyString)
{
if (proxyString.IsNullOrWhiteSpace()) return [];
```
要完整应用这一改动，需要对文件的其他部分进行更新：
1. 用对 `_ParseProxyItem` 的调用替换当前内联的 `Uri.TryCreate` + `Host` 处理逻辑，并且：
- 跳过 `_ParseProxyItem` 返回 `null` 的条目（即不要将它们包含在 `ProxyItem[]` 结果中）。
2. 更新目前在 `Uri.TryCreate` 分支中直接返回 `ProxyItem` 的方法，使其改用 `_ParseProxyItem`，或者如果需要向上传递 `null`，则将其签名更新为返回 `ProxyItem?`。
3. 确保在未显式指定协议时，能像新的辅助方法所示那样一致地使用 `_ParseProtocol` 和 `ProxyProtocol.Http`。
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
RefreshSystemProxy, you no longer resetisSystemProxyEnabledto 0 when no HTTP proxy is found; if other logic relies on this flag accurately reflecting effective HTTP proxy availability, consider preserving the previous behavior or clarifying the semantics.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `RefreshSystemProxy`, you no longer reset `isSystemProxyEnabled` to 0 when no HTTP proxy is found; if other logic relies on this flag accurately reflecting effective HTTP proxy availability, consider preserving the previous behavior or clarifying the semantics.
## Individual Comments
### Comment 1
<location path="PCL.Core/IO/Net/Http/HttpProxyManager.cs" line_range="90-93" />
<code_context>
- }
- else
+ // 能解析出主机名的地址，规范化为 host:port
+ if (Uri.TryCreate(address, new UriCreationOptions(), out var uri) && !uri.Host.IsNullOrEmpty())
{
- ret.Add(new ProxyItem { Protocol = ProxyProtocol.Http, Address = proxyString.Trim() });
+ var hostPort = uri.Port > 0 ? $"{uri.Host}:{uri.Port}" : uri.Host;
+ return new ProxyItem(protocol ?? _ParseProtocol(uri.Scheme), hostPort);
}
</code_context>
<issue_to_address>
**suggestion:** The behavior when the URI has no host (e.g., file/system URIs) may not be ideal for proxies.
When `Uri.TryCreate` succeeds but `Host` is empty, the code currently falls back to treating `address` as a raw proxy endpoint. For proxy settings, values like `file:///c:/path` are effectively invalid but would still pass through and later be used to build an HTTP URI. It would be safer to reject URIs with an empty host for proxy purposes (e.g., return `null`) so such misconfigurations don’t propagate into the HTTP stack.
Suggested implementation:
```csharp
// 能解析出主机名的地址，规范化为 host:port
private sealed record ProxyItem(ProxyProtocol Protocol, string Address);
// 返回 ProxyItem?，以便在 URI 有效但没有主机名时能够返回 null
private static ProxyItem? _ParseProxyItem(string address, ProxyProtocol? protocol = null)
{
// 能解析出主机名的地址，规范化为 host:port
if (Uri.TryCreate(address, new UriCreationOptions(), out var uri))
{
if (uri.Host.IsNullOrEmpty())
{
// 对于没有主机名的 URI（例如 file://），认为是无效的代理配置
return null;
}
var hostPort = uri.Port > 0 ? $"{uri.Host}:{uri.Port}" : uri.Host;
return new ProxyItem(protocol ?? _ParseProtocol(uri.Scheme), hostPort);
}
// 非 URI 格式的地址，按原始代理端点处理
return new ProxyItem(protocol ?? ProxyProtocol.Http, address.Trim());
}
private static ProxyItem[] _GetProxyFromString(string? proxyString)
{
if (proxyString.IsNullOrWhiteSpace()) return [];
```
To fully apply this change, the rest of the file needs to be updated to:
1. Replace the existing inline `Uri.TryCreate` + `Host` handling logic with calls to `_ParseProxyItem`, and:
- Skip entries where `_ParseProxyItem` returns `null` (i.e., do not include them in the `ProxyItem[]` result).
2. Update any current method that was returning `ProxyItem` directly from the `Uri.TryCreate` branch to use `_ParseProxyItem` instead, or update its signature to return `ProxyItem?` if it needs to propagate the `null`.
3. Ensure that `_ParseProtocol` and `ProxyProtocol.Http` are used consistently when the protocol is not explicitly specified, as shown in the new helper method.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+90
to
+93
| if (Uri.TryCreate(address, new UriCreationOptions(), out var uri) && !uri.Host.IsNullOrEmpty()) | ||
| { | ||
| ret.Add(new ProxyItem { Protocol = ProxyProtocol.Http, Address = proxyString.Trim() }); | ||
| var hostPort = uri.Port > 0 ? $"{uri.Host}:{uri.Port}" : uri.Host; | ||
| return new ProxyItem(protocol ?? _ParseProtocol(uri.Scheme), hostPort); |
Contributor
There was a problem hiding this comment.
suggestion: 当 URI 没有主机名时（例如文件/系统 URI），这种行为对代理而言可能并不理想。
当 Uri.TryCreate 成功但 Host 为空时，当前代码会退回到将 address 视为原始代理端点。对于代理配置来说，像 file:///c:/path 这样的值在实际效果上是无效的，但仍会被通过，并在后续用于构建 HTTP URI。为了避免这种错误配置传播到 HTTP 栈，作为代理而言，对主机名为空的 URI 进行拒绝（例如返回 null）会更安全。
建议实现如下：
// 能解析出主机名的地址，规范化为 host:port
private sealed record ProxyItem(ProxyProtocol Protocol, string Address);
// 返回 ProxyItem?，以便在 URI 有效但没有主机名时能够返回 null
private static ProxyItem? _ParseProxyItem(string address, ProxyProtocol? protocol = null)
{
// 能解析出主机名的地址，规范化为 host:port
if (Uri.TryCreate(address, new UriCreationOptions(), out var uri))
{
if (uri.Host.IsNullOrEmpty())
{
// 对于没有主机名的 URI（例如 file://），认为是无效的代理配置
return null;
}
var hostPort = uri.Port > 0 ? $"{uri.Host}:{uri.Port}" : uri.Host;
return new ProxyItem(protocol ?? _ParseProtocol(uri.Scheme), hostPort);
}
// 非 URI 格式的地址，按原始代理端点处理
return new ProxyItem(protocol ?? ProxyProtocol.Http, address.Trim());
}
private static ProxyItem[] _GetProxyFromString(string? proxyString)
{
if (proxyString.IsNullOrWhiteSpace()) return [];要完整应用这一改动，需要对文件的其他部分进行更新：
- 用对
_ParseProxyItem的调用替换当前内联的Uri.TryCreate+Host处理逻辑，并且：- 跳过
_ParseProxyItem返回null的条目（即不要将它们包含在ProxyItem[]结果中）。
- 跳过
- 更新目前在
Uri.TryCreate分支中直接返回ProxyItem的方法，使其改用_ParseProxyItem，或者如果需要向上传递null，则将其签名更新为返回ProxyItem?。 - 确保在未显式指定协议时，能像新的辅助方法所示那样一致地使用
_ParseProtocol和ProxyProtocol.Http。
Original comment in English
suggestion: The behavior when the URI has no host (e.g., file/system URIs) may not be ideal for proxies.
When Uri.TryCreate succeeds but Host is empty, the code currently falls back to treating address as a raw proxy endpoint. For proxy settings, values like file:///c:/path are effectively invalid but would still pass through and later be used to build an HTTP URI. It would be safer to reject URIs with an empty host for proxy purposes (e.g., return null) so such misconfigurations don’t propagate into the HTTP stack.
Suggested implementation:
// 能解析出主机名的地址，规范化为 host:port
private sealed record ProxyItem(ProxyProtocol Protocol, string Address);
// 返回 ProxyItem?，以便在 URI 有效但没有主机名时能够返回 null
private static ProxyItem? _ParseProxyItem(string address, ProxyProtocol? protocol = null)
{
// 能解析出主机名的地址，规范化为 host:port
if (Uri.TryCreate(address, new UriCreationOptions(), out var uri))
{
if (uri.Host.IsNullOrEmpty())
{
// 对于没有主机名的 URI（例如 file://），认为是无效的代理配置
return null;
}
var hostPort = uri.Port > 0 ? $"{uri.Host}:{uri.Port}" : uri.Host;
return new ProxyItem(protocol ?? _ParseProtocol(uri.Scheme), hostPort);
}
// 非 URI 格式的地址，按原始代理端点处理
return new ProxyItem(protocol ?? ProxyProtocol.Http, address.Trim());
}
private static ProxyItem[] _GetProxyFromString(string? proxyString)
{
if (proxyString.IsNullOrWhiteSpace()) return [];To fully apply this change, the rest of the file needs to be updated to:
- Replace the existing inline
Uri.TryCreate+Hosthandling logic with calls to_ParseProxyItem, and:- Skip entries where
_ParseProxyItemreturnsnull(i.e., do not include them in theProxyItem[]result).
- Skip entries where
- Update any current method that was returning
ProxyItemdirectly from theUri.TryCreatebranch to use_ParseProxyIteminstead, or update its signature to returnProxyItem?if it needs to propagate thenull. - Ensure that
_ParseProtocolandProxyProtocol.Httpare used consistently when the protocol is not explicitly specified, as shown in the new helper method.
XueL1ng
added a commit
to PCL-Nex-Developer/PCL2-Nex
that referenced
this pull request
Aug 26, 2026
来源：PCL-Community/PCL-CE #3495 PCL-Community/PCL-CE#3495
XueL1ng
added a commit
to PCL-Nex-Developer/PCL2-Nex
that referenced
this pull request
Sep 4, 2026
来源：PCL-Community/PCL-CE #3495 PCL-Community/PCL-CE#3495
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
通过拓展代理字符串解析方式并收紧应用 HTTP 代理的条件，改进系统代理处理。
Bug Fixes:
Enhancements:
ProxyItem的表示形式。Original summary in English
Summary by Sourcery
Improve system proxy handling by broadening proxy string parsing and tightening HTTP proxy application conditions.
Bug Fixes:
Enhancements: