-
Notifications
You must be signed in to change notification settings - Fork 143
fix(proxy): 代理字段解析遗漏一种格式 #3495
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
Open
tangge233
wants to merge
1
commit into
dev
Choose a base branch
from
fix/proxy-string-format
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
fix(proxy): 代理字段解析遗漏一种格式 #3495
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
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.
suggestion: 当 URI 没有主机名时(例如文件/系统 URI),这种行为对代理而言可能并不理想。
当
Uri.TryCreate成功但Host为空时,当前代码会退回到将address视为原始代理端点。对于代理配置来说,像file:///c:/path这样的值在实际效果上是无效的,但仍会被通过,并在后续用于构建 HTTP URI。为了避免这种错误配置传播到 HTTP 栈,作为代理而言,对主机名为空的 URI 进行拒绝(例如返回null)会更安全。建议实现如下:
要完整应用这一改动,需要对文件的其他部分进行更新:
_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.TryCreatesucceeds butHostis empty, the code currently falls back to treatingaddressas a raw proxy endpoint. For proxy settings, values likefile:///c:/pathare 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., returnnull) so such misconfigurations don’t propagate into the HTTP stack.Suggested implementation:
To fully apply this change, the rest of the file needs to be updated to:
Uri.TryCreate+Hosthandling logic with calls to_ParseProxyItem, and:_ParseProxyItemreturnsnull(i.e., do not include them in theProxyItem[]result).ProxyItemdirectly from theUri.TryCreatebranch to use_ParseProxyIteminstead, or update its signature to returnProxyItem?if it needs to propagate thenull._ParseProtocolandProxyProtocol.Httpare used consistently when the protocol is not explicitly specified, as shown in the new helper method.