Skip to content
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

ActorId accepts invalid IDs, causing issues when making Actor HTTP calls #537

Closed
heunghingwan opened this issue Oct 18, 2023 · 3 comments · Fixed by #539
Closed

ActorId accepts invalid IDs, causing issues when making Actor HTTP calls #537

heunghingwan opened this issue Oct 18, 2023 · 3 comments · Fixed by #539
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@heunghingwan
Copy link
Contributor

When constructing an actor using strings such as url, it will throw with app-id not found error. I don’t know whether it is an SDK or core error.

Steps to Reproduce the Problem

const actor = await builder.build(new ActorId(url))
actor.test() <- throw with app-id not found

@shubham1172
Copy link
Member

So URLs should not be allowed as actor IDs because of the way actor APIs are written, reference: https://docs.dapr.io/reference/api/actors_api/#get-actor-reminder

For example, if actor ID is https://foo.bar, the below request will look like /actors/${actorType}/https://foo.bar/state/${key}

const result = await this.client.execute(`/actors/${actorType}/${actorId.getId()}/state/${key}`);

What we should do is throw an error if the actor ID is not proper or potentially sanitize it, instead of simply accepting any string.

this.id = id;

@shubham1172 shubham1172 added bug Something isn't working good first issue Good for newcomers labels Oct 18, 2023
@shubham1172 shubham1172 changed the title Actor api error when Actor Id contain :// ActorId accepts invalid IDs, causing issues when making Actor HTTP calls Oct 18, 2023
@heunghingwan
Copy link
Contributor Author

Does this mean it will work when using grpc protocol?

@XavierGeerinck
Copy link
Contributor

Does this mean it will work when using grpc protocol?

It would, but gRPC is not supported on the runtime :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants