-
Notifications
You must be signed in to change notification settings - Fork 284
Introduce a dedicated direct::LocalHttp target type #1417
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
Introduce a dedicated direct::LocalHttp target type #1417
Conversation
mateiidavid
left a comment
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.
Makes for a much nicer read now :) It also seems to put me at ease with the policy questions I had. The only thing that I'm really unsure of (and that I put in a comment) is whether through the old approach we'd end up checking the policy twice: once here, and once through the http stack.
| negotiated_protocol: client.alpn, | ||
| }, | ||
| ); | ||
| let permit = policy |
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.
Ah nice, so when we go direct w/o a protocol we can just check the policy here. This is something I was wondering about in my approach; if we check the policy in the direct stack, it'll be checked once again in the http stack?
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.
yeah, in this case we only check the authorization for the tcp stack -- this is why the LocalTcp type takes a permit: to prove that it has been authorized.
the http stack will do its own authorization checks on requests.
| protocol: SessionProtocol, | ||
| } | ||
|
|
||
| type Local = svc::Either<LocalTcp, LocalHttp>; |
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.
This is neat! So this basically allows us to just return the inner predicate on L129 and have the switch layer in one line? That's really cool.
No description provided.