Add SSM support to Direct Sockets API (services/network and Blink layers) [chromium/src : main]
Omar Ramadan (Gerrit)
Omar Ramadan abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 2 comments![]( "Open in Gerrit")
Vector<String> joined_groups_;Why not hold MembershipKey in this vector? Then string operations to join strings + "@" won't be necessary.
Related details
- Andrew Rayskiy
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 1 comment![]( "Open in Gerrit")
std::optional<net::IPAddress> MulticastController::ParseAndValidateIPAddress(IsValidMulticastAddress can be included in this function. It is fine if old joinGroup/leaveGroup will also be checked for multicast address.
Omar Ramadan (Gerrit)
Omar Ramadan added 3 comments![]( "Open in Gerrit")
Why not hold MembershipKey in this vector? Then string operations to join strings + "@" won't be necessary.
Done. Changed joined_groups_ from Vector<String> to Vector<MembershipKey> to eliminate string concatenation operations.
std::optional<net::IPAddress> MulticastController::ParseAndValidateIPAddress(IsValidMulticastAddress can be included in this function. It is fine if old joinGroup/leaveGroup will also be checked for multicast address.
Done. Created ParseAndValidateMulticastAddress() and ParseAndValidateUnicastAddress() that include the multicast check. Both joinGroup and leaveGroup now validate addresses appropriately.
'net::IPAddress',Omar RamadanWhy this is needed?
MulticastController uses it in the public API for ParseAndValidateIPAddress and IsValidMulticastAddress. The audit tool requires explicit approval for non-Blink types used in Blink code.
Related details
- Andrew Rayskiy
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 2 comments![]( "Open in Gerrit")
HeapHashMap<String, Member<ScriptPromiseResolver<IDLUndefined>>>Why not use MembershipKey here as well as a key. Then membership_key_str will not be necessary. The hashing function will be required.
const String& ip_string,Since these functions are not used anywhere outside of this file, I recommend to move them to .cc and to anynimous namespace. This will also eliminate the need for touching third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
Related details
- Andrew Rayskiy
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omar Ramadan (Gerrit)
Omar Ramadan added 2 comments![]( "Open in Gerrit")
HeapHashMap<String, Member<ScriptPromiseResolver<IDLUndefined>>>Why not use MembershipKey here as well as a key. Then membership_key_str will not be necessary. The hashing function will be required.
Changed HeapHashMap keys from String to MembershipKey with custom hash function, eliminating membership_key_str conversions.
const String& ip_string,Since these functions are not used anywhere outside of this file, I recommend to move them to .cc and to anynimous namespace. This will also eliminate the need for touching third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
Moved to anonymous namespace in .cc but audit still complained about net::IPAddress although was already used in multicast_controller.cc. Is it safe to ignore?
Related details
- Andrew Rayskiy
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reilly Grant (Gerrit)
Reilly Grant added 2 comments![]( "Open in Gerrit")
bool IsValidMulticastAddress(const net::IPAddress& address) {Consider adding this to `IPAddress` like the existing `IsPubliclyRoutable` method.
return {};We now need to catch the case where an empty string was provided for this non-optional parameter.
Related details
- Andrew Rayskiy
- Omar Ramadan
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reilly Grant (Gerrit)
Reilly Grant added 1 comment![]( "Open in Gerrit")
Promise<undefined> joinGroup(DOMString ipAddress, optional DOMString? sourceAddress);Create a new feature in `runtime_enabled_features.json5` for this and add the [RuntimeEnabled] attribute to `sourceAddress` so that it's only accepted if the feature is enabled.
Andrew Rayskiy (Gerrit)
Andrew Rayskiy added 8 comments![]( "Open in Gerrit")
return StringHash::GetHash(key.group) ^Tbh I'd rather do a `StringHash::GetHash(key.ToString())` here -- it's unique enough
bool operator==(const MembershipKey& other) const {just default the operator?
String source; // Empty for ASM`optional<>`?
String group;I'd just keep `IP`, `IP` here instead of strings
ScriptPromise<IDLUndefined> joinGroup(ScriptState*,Wouldn't it be simpler to merge these functions? With `(ScriptState*, const String&, MulticastGroupOptions*, ExceptionState&`)?
const String& ip_string,Omar RamadanSince these functions are not used anywhere outside of this file, I recommend to move them to .cc and to anynimous namespace. This will also eliminate the need for touching third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py
Moved to anonymous namespace in .cc but audit still complained about net::IPAddress although was already used in multicast_controller.cc. Is it safe to ignore?
Yes, it's fine.
MembershipKey key;I'd create a proper constructor for this which takes `(const IP&, const optional<IP>&)`
Promise<undefined> joinGroup(DOMString ipAddress, optional DOMString? sourceAddress);I believe you wanted to go with MulticastGroupOptions as per the spec PR?
Related details
- Omar Ramadan
- Vlad Krot
Omar Ramadan (Gerrit)
Omar Ramadan added 11 comments![]( "Open in Gerrit")
thank you so much for the quick review all, and guiding me as a first time contributor. sorry for tagging folks unnecessarily
Tbh I'd rather do a `StringHash::GetHash(key.ToString())` here -- it's unique enough
Done
bool operator==(const MembershipKey& other) const {Omar Ramadanjust default the operator?
Done
String source; // Empty for ASMOmar Ramadan`optional<>`?
Done
I'd just keep `IP`, `IP` here instead of strings
Done
Wouldn't it be simpler to merge these functions? With `(ScriptState*, const String&, MulticastGroupOptions*, ExceptionState&`)?
Done
bool IsValidMulticastAddress(const net::IPAddress& address) {Consider adding this to `IPAddress` like the existing `IsPubliclyRoutable` method.
Done
We now need to catch the case where an empty string was provided for this non-optional parameter.
Done
I'd create a proper constructor for this which takes `(const IP&, const optional<IP>&)`
Done
Promise<undefined> joinGroup(DOMString ipAddress, optional DOMString? sourceAddress);I believe you wanted to go with MulticastGroupOptions as per the spec PR?
Done
Promise<undefined> joinGroup(DOMString ipAddress, optional DOMString? sourceAddress);Create a new feature in `runtime_enabled_features.json5` for this and add the [RuntimeEnabled] attribute to `sourceAddress` so that it's only accepted if the feature is enabled.
- Andrew Rayskiy
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reilly Grant (Gerrit)
Reilly Grant added 5 comments![]( "Open in Gerrit")
bool IPAddress::IsMulticast() const {Use `IPAddressPrefixCheck` in this method.
if (!group_ip.has_value()) {
return {};
}I would move the check above into here to make sure we always catch cases where `group_ip` is nullopt without an exception having been thrown.
```suggestion
if (!group_ip.has_value()) {
if (!exception_state.HadException()) {
exception_state.ThrowTypeError("ipAddress must not be empty");
}
return {};
}
```
required DOMString groupAddress;```suggestion
DOMString groupAddress;
```
MulticastGroupOptions MakeSourceOptions(const char* source_address) {```suggestion
MulticastGroupOptions MakeSourceOptions(std::string_view source_address) {
```
status: "stable",This lets us land this change before the Intent to Ship is approved.
```suggestion
status: "experimental",
```
Related details
- Andrew Rayskiy
- Omar Ramadan
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omar Ramadan (Gerrit)
Omar Ramadan added 6 comments![]( "Open in Gerrit")
Use `IPAddressPrefixCheck` in this method.
Done
I would move the check above into here to make sure we always catch cases where `group_ip` is nullopt without an exception having been thrown.
```suggestion
if (!group_ip.has_value()) {
if (!exception_state.HadException()) {
exception_state.ThrowTypeError("ipAddress must not be empty");
}
return {};
}
```
Done
required DOMString groupAddress;Omar Ramadan```suggestion
DOMString groupAddress;
```
Done
MulticastGroupOptions MakeSourceOptions(const char* source_address) {```suggestion
MulticastGroupOptions MakeSourceOptions(std::string_view source_address) {
```
Done
This lets us land this change before the Intent to Ship is approved.
```suggestion
status: "experimental",
```
- Andrew Rayskiy
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 1 comment![]( "Open in Gerrit")
status: "stable",Omar RamadanThis lets us land this change before the Intent to Ship is approved.
```suggestion
status: "experimental",
```
Done
We need a separate flag. This change will disable Multicast altogether which is already used by developers, and they expect this functionality to not disappear.
Also, I would suggest adding a flag in about://flags to simplify testing for those who are interested.
And do this all in a separate CL.
Reference - https://chromium-review.googlesource.com/c/chromium/src/+/6890773
Related details
- Andrew Rayskiy
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omar Ramadan (Gerrit)
Omar Ramadan added 1 comment![]( "Open in Gerrit")
status: "stable",Omar RamadanThis lets us land this change before the Intent to Ship is approved.
```suggestion
status: "experimental",
```
Vlad KrotDone
We need a separate flag. This change will disable Multicast altogether which is already used by developers, and they expect this functionality to not disappear.
Also, I would suggest adding a flag in about://flags to simplify testing for those who are interested.
And do this all in a separate CL.
Reference - https://chromium-review.googlesource.com/c/chromium/src/+/6890773
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 2 comments![]( "Open in Gerrit")
udp_socket_->JoinSourceGroup(group_address, *source_address,Source-Specific Multicast (SSM) feature flag must be checked here in case renderer was compromised. If the flag is disabled, but the function was called with source_address, then do mojo::ReportBadMessage("no permission to use source-specific multicast");
ScriptPromise<IDLUndefined> MulticastController::joinGroup(There should be a check for new feature flag here. Now the addition to multicast API is possible to use with the flag disabled.
Related details
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 1 comment![]( "Open in Gerrit")
name: "SourceSpecificMulticastInDirectSockets",I don't think that a new permission policy is necessary. This is usually added to guard some dangerous functionality, I don't see how source specific multicast is more dangerous then the usual one.
Related details
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 1 comment![]( "Open in Gerrit")
https://chromestatus.com/feature/6208452397498368Create intent to prototype.
Omar Ramadan (Gerrit)
Omar Ramadan added 4 comments![]( "Open in Gerrit")
https://chromestatus.com/feature/6208452397498368Omar RamadanCreate intent to prototype.
Done
I don't think that a new permission policy is necessary. This is usually added to guard some dangerous functionality, I don't see how source specific multicast is more dangerous then the usual one.
Done
udp_socket_->JoinSourceGroup(group_address, *source_address,Source-Specific Multicast (SSM) feature flag must be checked here in case renderer was compromised. If the flag is disabled, but the function was called with source_address, then do mojo::ReportBadMessage("no permission to use source-specific multicast");
Done
ScriptPromise<IDLUndefined> MulticastController::joinGroup(There should be a check for new feature flag here. Now the addition to multicast API is possible to use with the flag disabled.
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 2 comments![]( "Open in Gerrit")
if (!base::FeatureList::IsEnabled(Nit: I think it was easier just check this flag in restricted_udp_socket, the reason multicast was passed as a variable, because it had check for permission policy.
// We don't require a separate permissions policy for SSM (see crbug.com/1520671).Related details
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omar Ramadan (Gerrit)
Omar Ramadan added 2 comments![]( "Open in Gerrit")
Nit: I think it was easier just check this flag in restricted_udp_socket, the reason multicast was passed as a variable, because it had check for permission policy.
Done
// We don't require a separate permissions policy for SSM (see crbug.com/1520671).(see crbug.com/1520671).
How is this bug relevant here?
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot voted Code-Review+1![]( "Open in Gerrit")
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Rayskiy (Gerrit)
Andrew Rayskiy added 1 comment![]( "Open in Gerrit")
IsMulticastAllowed(context_) &&I don't think it's necessary to propagate this all the way down the stack; since all processes generally have the same launch flags, you can simply do
```
if (source_address.has_value()) {
// Source-Specific Multicast (SSM)
if (!base::FeatureList::IsEnabled(blink::features::kSourceSpecificMulticastInDirectSockets)) {
// Calling this method with source_address without the SSM permission
// means that the renderer is sending faulty IPCs.
mojo::ReportBadMessage("no permission to use source-specific multicast");
return;
}
udp_socket_->JoinSourceGroup(group_address, *source_address,
std::move(callback));
} else {
// Any-Source Multicast (ASM)
udp_socket_->JoinGroup(group_address, std::move(callback));
}
```
In your `restricted_udp_socket.cc`
Related details
- Omar Ramadan
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omar Ramadan (Gerrit)
Omar Ramadan added 1 comment![]( "Open in Gerrit")
IsMulticastAllowed(context_) &&I don't think it's necessary to propagate this all the way down the stack; since all processes generally have the same launch flags, you can simply do
```
if (source_address.has_value()) {
// Source-Specific Multicast (SSM)
if (!base::FeatureList::IsEnabled(blink::features::kSourceSpecificMulticastInDirectSockets)) {
// Calling this method with source_address without the SSM permission
// means that the renderer is sending faulty IPCs.
mojo::ReportBadMessage("no permission to use source-specific multicast");
return;
}
udp_socket_->JoinSourceGroup(group_address, *source_address,
std::move(callback));
} else {
// Any-Source Multicast (ASM)
udp_socket_->JoinGroup(group_address, std::move(callback));
}
```In your `restricted_udp_socket.cc`
@green...@google.com if we can use `FeatureList::IsEnabled` directly in `restricted_udp_socket.cc` to check both multicast and source specific multicast, should I remove `allow_multicast` all together from `content/browser`? cc @vk...@google.com
Related details
- Andrew Rayskiy
- Reilly Grant
- Vlad Krot
Vlad Krot (Gerrit)
Vlad Krot added 1 comment![]( "Open in Gerrit")
IsMulticastAllowed(context_) &&Omar RamadanI don't think it's necessary to propagate this all the way down the stack; since all processes generally have the same launch flags, you can simply do
```
if (source_address.has_value()) {
// Source-Specific Multicast (SSM)
if (!base::FeatureList::IsEnabled(blink::features::kSourceSpecificMulticastInDirectSockets)) {
// Calling this method with source_address without the SSM permission
// means that the renderer is sending faulty IPCs.
mojo::ReportBadMessage("no permission to use source-specific multicast");
return;
}
udp_socket_->JoinSourceGroup(group_address, *source_address,
std::move(callback));
} else {
// Any-Source Multicast (ASM)
udp_socket_->JoinGroup(group_address, std::move(callback));
}
```In your `restricted_udp_socket.cc`
@green...@google.com if we can use `FeatureList::IsEnabled` directly in `restricted_udp_socket.cc` to check both multicast and source specific multicast, should I remove `allow_multicast` all together from `content/browser`? cc @vk...@google.com
"should I remove allow_multicast all together from" - no, because AllowMulticast also contains result of permissions policy check for which you need ExecutionContext.
Related details
- Andrew Rayskiy
- Omar Ramadan
- Reilly Grant
Andrew Rayskiy (Gerrit)
Andrew Rayskiy added 1 comment![]( "Open in Gerrit")
IsMulticastAllowed(context_) &&Omar RamadanI don't think it's necessary to propagate this all the way down the stack; since all processes generally have the same launch flags, you can simply do
```
if (source_address.has_value()) {
// Source-Specific Multicast (SSM)
if (!base::FeatureList::IsEnabled(blink::features::kSourceSpecificMulticastInDirectSockets)) {
// Calling this method with source_address without the SSM permission
// means that the renderer is sending faulty IPCs.
mojo::ReportBadMessage("no permission to use source-specific multicast");
return;
}
udp_socket_->JoinSourceGroup(group_address, *source_address,
std::move(callback));
} else {
// Any-Source Multicast (ASM)
udp_socket_->JoinGroup(group_address, std::move(callback));
}
```In your `restricted_udp_socket.cc`
Vlad Krot@green...@google.com if we can use `FeatureList::IsEnabled` directly in `restricted_udp_socket.cc` to check both multicast and source specific multicast, should I remove `allow_multicast` all together from `content/browser`? cc @vk...@google.com
"should I remove allow_multicast all together from" - no, because AllowMulticast also contains result of permissions policy check for which you need ExecutionContext.
+1! allow_multicast is per-frame, FeatureList::IsEnabled is global
Related details
- Omar Ramadan
- Reilly Grant
Omar Ramadan (Gerrit)
Omar Ramadan added 1 comment![]( "Open in Gerrit")
Omar RamadanI don't think it's necessary to propagate this all the way down the stack; since all processes generally have the same launch flags, you can simply do
```
if (source_address.has_value()) {
// Source-Specific Multicast (SSM)
if (!base::FeatureList::IsEnabled(blink::features::kSourceSpecificMulticastInDirectSockets)) {
// Calling this method with source_address without the SSM permission
// means that the renderer is sending faulty IPCs.
mojo::ReportBadMessage("no permission to use source-specific multicast");
return;
}
udp_socket_->JoinSourceGroup(group_address, *source_address,
std::move(callback));
} else {
// Any-Source Multicast (ASM)
udp_socket_->JoinGroup(group_address, std::move(callback));
}
```In your `restricted_udp_socket.cc`
Vlad Krot@green...@google.com if we can use `FeatureList::IsEnabled` directly in `restricted_udp_socket.cc` to check both multicast and source specific multicast, should I remove `allow_multicast` all together from `content/browser`? cc @vk...@google.com
Andrew Rayskiy"should I remove allow_multicast all together from" - no, because AllowMulticast also contains result of permissions policy check for which you need ExecutionContext.
+1! allow_multicast is per-frame, FeatureList::IsEnabled is global
- Andrew Rayskiy
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Rayskiy (Gerrit)
Andrew Rayskiy voted Code-Review+1![]( "Open in Gerrit")
- Omar Ramadan
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Andrew Rayskiy
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vlad Krot (Gerrit)
Vlad Krot added 1 comment![]( "Open in Gerrit")
bool is_deleted = false;What is the reason for adding this?
Related details
- Andrew Rayskiy
- Omar Ramadan
- Reilly Grant
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omar Ramadan (Gerrit)
Omar Ramadan added 1 comment![]( "Open in Gerrit")
bool is_deleted = false;What is the reason for adding this?
Blink's hash tables use open addressing so when a collision occurs, the table probes for the next available slot. To work correctly, the table needs to distinguish between three states for each slot (empty/deleted/valid). If you simply marked deleted slots as "empty," you'd break the probing chain. The `is_deleted` acts as a sentinel to differentiate deleted entries as `true` while an invalid/empty IP address `false`
Related details
- Andrew Rayskiy
- Reilly Grant
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: tse...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): tse...@chromium.org
Reviewer source(s):
tse...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Andrew Rayskiy
- David Schinazi
- Reilly Grant
- Tom Sepez
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reilly Grant (Gerrit)
Reilly Grant added 2 comments![]( "Open in Gerrit")
// Calling this method with source_address without the SSM permission```suggestion
// Calling this method with source_address without the SSM feature enabled
```
mojo::ReportBadMessage("no permission to use source-specific multicast");This isn't a permissions thing.
```suggestion
mojo::ReportBadMessage("Source-specific multicast disabled");
```
Related details
- Andrew Rayskiy
- David Schinazi
- Omar Ramadan
- Tom Sepez
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Omar Ramadan (Gerrit)
Omar Ramadan added 3 comments![]( "Open in Gerrit")
thank you
// Calling this method with source_address without the SSM permission```suggestion
// Calling this method with source_address without the SSM feature enabled
```
Fix applied.
mojo::ReportBadMessage("no permission to use source-specific multicast");This isn't a permissions thing.
```suggestion
mojo::ReportBadMessage("Source-specific multicast disabled");
```
- Andrew Rayskiy
- David Schinazi
- Reilly Grant
- Tom Sepez
- Vlad Krot
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |