-
Couldn't load subscription status.
- Fork 62
API Review: Adding CoreWebView2NetworkComponentScope to Port Configuration #5397
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: main
Are you sure you want to change the base?
Conversation
This change extends the existing WebView2 environment options API for port configuration (SetAllowedPortRange and GetAllowedPortRange) by introducing a new parameter: CoreWebView2NetworkComponentScope. This addition gives developers more granular control over port restrictions, allowing them to target specific network components rather than applying restrictions globally across all network traffic.
|
|
||
| 1. Network Scope param in SetAllowedPortRange | ||
| - A network component-specific scope (e.g. _WEB_RTC) always takes precedence over _ALL for that component in `SetAllowedPortRange`. | ||
| - `_ALL` defines the port range restrictions for all components without specific overrides. |
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.
I think "All" enum should be named "Default" or "Unspecified" if it's not going to apply to WebRTC when _web_rtc is also set.
It's unintuitive that calling SetAllowedPortRange(ALL, 350-400); doesn't restrict the port range of WebRTC ports if WebRTC is currently restricted to 300-500, even though I'm Setting a narrower restriction on "all" component scopes.
It's unintuitive that WebRTC escapes the restriction of an existing SetAllowedPortRange(ALL, 300-400); restriction if I temporarily restrict it more narrowly (SetAllowedPortRange(WebRTC, 350-375);) and then remove that WebRTC-specific range (SetAllowedPortRange(WebRTC, 0-0);), since the original range now no longer applies to "all" scopes.
Calling the 0 value something other than "All" mitigates both of these misreadings, without having to revisit the underlying range interaction behavior you're proposing.
| ``` | ||
| /// Specifies the network component scope for port configuration. | ||
| [v1_enum] | ||
| typedef enum COREWEBVIEW2_NETWORK_COMPONENT_SCOPE { |
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.
"Network component scope" seems more generic than what it's being used for in this API. Is this enum feasibly reusable in another future API or should this be CoreWebview2AllowedPortRangeKind ?
| | Network Scope State | Behaviour | | ||
| | ------------------------------------------ | ------------------------------------------------------------------------------ | | ||
| | Only `_ALL` is set | `_ALL` applies port range restrictions to all network components | | ||
| | `_ALL` and `_WEB_RTC` are both set | `_WEB_RTC` port range restrictions applies to WebRTC; `_ALL` applies to others | |
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 other candidate interaction model would be that when "All" and a specific Component both have ranges set, the effective range at runtime is the intersection of the two ranges. It's not immediately clear to me if this model would be more or less useful to developers. Just checking, have both models been considered?
| | `_ALL` set and `_WEB_RTC` reset to `(0,0)` | `_ALL` applies port range restrictions to all except WebRTC (unrestricted) | | ||
|
|
||
| 2. Network Scope param in GetAllowedPortRange | ||
| - `GetAllowedPortRange` returns the range explicitly set for the queried scope. |
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.
GetEffectiveAllowedPortRange since it is not the opposite of Set but rather does calculations to determine the effect of multiple Sets.
XAML has OverflowButtonVisibility + EffectiveOverflowButtonVisibility. Also Height/Width + ActualHeight/ActualWidth.
HdcpSession has SetDesiredMinProtection + GetEffectiveProtection.
| 1. Network Scope param in SetAllowedPortRange | ||
| - A network component-specific scope (e.g. _WEB_RTC) always takes precedence over _ALL for that component in `SetAllowedPortRange`. | ||
| - `_ALL` defines the port range restrictions for all components without specific overrides. | ||
| - Passing `(0, 0)` for a network component scope removes its restriction. |
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.
How do I block all ports?
| 1. Network Scope param in SetAllowedPortRange | ||
| - A network component-specific scope (e.g. _WEB_RTC) always takes precedence over _ALL for that component in `SetAllowedPortRange`. | ||
| - `_ALL` defines the port range restrictions for all components without specific overrides. | ||
| - Passing `(0, 0)` for a network component scope removes its restriction. |
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.
"Removes its restriction" implies "Pretend I never set a restriction".
But from the discussion below, it seems that passing (0,0) really means "set to unrestricted".
So how do I remove the restriction, i.e., set a scope to "no custom restriction; use the generic one"?
Do we need a ClearAllowedPortRange to go back to the "unset" state?
Or maybe we say that (0,0) means "remove restriction" and "0, 65535" means "explicitly unrestricted".
| /// | ||
| /// `minPort` and `maxPort` must be within the range 1025–65535 (inclusive), | ||
| /// and `minPort` must be less than or equal to `maxPort`. | ||
| /// `minPort` and `maxPort` must be within the range 1025-65535 (inclusive). |
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.
Why do we disallow restrictions on ports below 1025?
Saying that it must be in the range 1025-65535 contradicts the remark later that you can pass 0 here if you want to set to unrestricted.
This change extends the existing WebView2 environment options API for port configuration (SetAllowedPortRange and GetAllowedPortRange) by introducing a new parameter: CoreWebView2NetworkComponentScope. This addition gives developers more granular control over port restrictions, allowing them to target specific network components rather than applying restrictions globally across all network traffic.