-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add validation for DTD service names #56177
Comments
Summary: The |
I've a change at https://dart-review.googlesource.com/c/sdk/+/374980 that disallows dots, but I haven't implemented the other suggestions here yet because they seem like questions.
What does "protected" mean here?
@bkonyi does the VM Service have any kind of restriction on these names? (Maybe some thought already went into whether it's a good and what should be allowed/disallowed?). |
There really aren't many restrictions on service extension names. For the |
I would need to look a little deeper at how all this code is implemented, but I was referring to first part services provided by DTD (like the FileSystem and UnifiedAnalytics services). We'd want to avoid naming conflicts if these go through the same service + method name call path, but we'd need to look at the code to verify whether or not that is a risk. |
Ah, got it! You're right - the way these services are currently registered, the existing check for whether a service name is used by another client does not prevent this (since they're registered by a client). I've included a fix for this in https://dart-review.googlesource.com/c/sdk/+/374980. |
…rvice names See #56177 Change-Id: I9765f232e71026a0ae6ee2ce95248472fb69ba3e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/374980 Reviewed-by: Kenzie Davisson <[email protected]> Commit-Queue: Ben Konyi <[email protected]> Reviewed-by: Ben Konyi <[email protected]>
DTD clients can register a service by calling the
registerService
method, which currently takes two String parameters for the service name and the method name, and concatenates them with a.
character to create the service method name (e.g.'foo.bar'
).https://github.com/dart-lang/sdk/blob/master/pkg/dtd_impl/lib/src/dtd_client.dart/#L192-L193
In the current state, the service and method name have no validation. Some things we may want to validate / restrict:
.
characters and only allow method names to contain.
. This will prevent confusion about what the service name is for something like 'foo.bar.baz'. In this example, we wouldn't know if this is the 'foo.bar' service with method 'baz' or the 'foo' service with method 'bar.baz'.The text was updated successfully, but these errors were encountered: