Skip to content

Conversation

@olix0r
Copy link
Member

@olix0r olix0r commented Dec 20, 2021

No description provided.

Copy link
Member

@mateiidavid mateiidavid left a 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
Copy link
Member

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?

Copy link
Member Author

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>;
Copy link
Member

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.

@olix0r olix0r merged commit 8a5ed77 into matei/opaque-n-http-traffic Dec 20, 2021
@olix0r olix0r deleted the ver/matei/opaque-n-http-traffic branch December 20, 2021 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants