Provide marker wrapper types for Ingress / Egress

[GlenDC]

A proxy is one category of projects where we have both a client side and a server side. More abstractly, this corresponds to inbound and outbound, or ingress and egress, or north and south. Right now we are in a situation where we:

• either only have one type, which can cause confusion. For example, a TLS tunnel for the incoming HTTP over TLS proxy might add a “secure” marker or similar, but that marker could then confuse the proxy when making a connection on the egress side, where that detail should not matter.
• or we already have explicit wrapper types for specific extension use cases. For example, ClientSocketInfo, which is a wrapper around SocketInfo.

Now that we have cleaned up the context and extension machinery and made it read only, it might be a good moment before 0.3 to introduce more hygiene here. I propose that we introduce wrapper types whose only purpose is to indicate which side they belong to:

#[derive(Debug, Clone, PartialEq, Eq, ...)]
pub struct Egress<T>(pub T);

#[derive(Debug, Clone, PartialEq, Eq, ...)]
pub struct Ingress<T>(pub T);

Instead of writing:

ext.get::<SocketInfo>();

you would write:

ext.get::<Ingress<SocketInfo>>();

let Ingress(socket_info) = ext.get().unwrap();

The concept is simple enough. My main question is what naming is clearest for everyone:

• ingress and egress
• inbound and outbound
• something else?

I think this, combined with adding side specific information to types that might otherwise still be ambiguous, will help us prevent unexpected or unhygienic behaviour. This way, extension extractors can make well informed decisions without any risk of ambiguity.

Please share your feedback. Once we are aligned, I can implement this quickly. I would prefer doing this before 0.3, since it might otherwise break users subtly if they rely on these types without the new side specific wrappers.

[GlenDC]

Please provide your feedback this week @soundofspace and call/chat with me if you need a more interactive discussion.
Would love to have this settled by end of this week, will tackle it right after I'm done with the Uri work which is starting today.

[soundofspace]

I like egress and ingress naming for this, it's probably the least ambiguous naming we can use. One more place I have noticed that is also having this problem, is upgraded connections. I'm honestly not sure how we should handle those:

• Don't transfer extensions there. Don't see this as an option, just mentioning so we have the complete spectrum of options
• Transfer all extensions. This is what we do now
• Transfer extensions but wrap them? Extension::new().insert(UpgradedExtensions(before_upgrade_extensions))
• Have some generic concept of tunnel/connection in extensions. Some/all extensions could be tied to specific connection. Ingress/Egress is specific use case for this, but so could TunneledConnection be. I'm not sure how naming would work here, or we would generalize this...

Way more invasive options

• Introduce a boundary mechanism in extensions. By default things like get only go back until a boundary is hit, but we also provide getters that can work across boundaries. This could be nice since by default you only look at the current setup, but specific use cases can look at all the info if needed. Boundaries could be enum such as Connection {Ingress, Egress} and activated like extensions.traverse_boundy(Connection::Egress)
• Rethink the idea of what an extensions is. Right now everything can be inserted in extensions, but we could limit this to only types implementing ExtensionTrait. With this trait we could make extensions more aware of what is happening and make them more powerful. For all basic uses cases this would probably be a derive impl, but advanced cases could implement logic like wrapping automatically. Advantage of this approach: we move the decision to the extensions itself, which might be a better place todo this, but that also means extensions logic is more all over the place, which is also a huge downside

To me it seems that upgraded connections are just another example of the same problem, be it a more difficult one (since they can used in so many different ways, and nested). But in its most basic form an upgrade is a new "connection", so is an ingress "connection" and egress "connection". There are probably more examples of where we want to "not leak" info by default, so it seems like we need a way to tie extensions to a specific place/connection.

[GlenDC]

I honestly think we should keep the design linear, flat and easy like today as otherwise you start to introduce complexities which ones again leads to surprises.

• Ingress/Egress is what this issue is about, which is to ensure we dis-ambiguate between which direction
• For other things we might need additional wrappers or introduce different extension types for different use cases.

You mentioned on discord "connectors do not always know that they are part of a tunnel". That is fundamentally wrong. The beauty of Rama is that nothing is a surprise or magic. As such the developer who creates the connector knows exact for what it is. This makes it trivial to pass info at creation to ensure the appropriate context is clear.

For example https://ramaproxy.org/docs/rama/tls/boring/client/struct.TlsConnectorLayer.html#method.tunnel is such an example where we make it explicitly clear that this is a tunnel and not a regular server. Where we need such disambiguation we should ensure that is made possible to be made clear fro the types.

In the same spirit as Ingress/Egress it should be up to us to ensure we make all types (be it wrapped, or just by itself non-ambigious) as clear as water. This way the linear approach continues to exist and it's all very easy. Yes it means the developer will at times need to make sure to call the correct constructor/setter/builder, but that's acceptable as it is expected that developers developing with Rama know what they are doing. It's not a one-json-and-all-magic-ready-framework, or something like pingora where you just have some hooks to implement. It's really a bunch of building blocks and layers of abstractions.

Our goal is to ensure we do not introduce accidental complexity or introduce edge cases where none are needed. We only just now finished a giant series of refactors to hugely simplify rama from lessons learned over last years. We do not want to go back in the opposite direction and make everything again more complex.

Until you proof otherwise I'm pretty certain by using the type system to its full extent we can solve this in the same simple linear fashion as we have today. Just need to make sure our types are either non-ambigious or where not otherwise possible that the types carry properties with them to ensure the consumers know what to make of it.

WDYT @soundofspace ? Or you have some concrete examples in mind that challenge my theory? Would like to start working on this somewhere next week ASAP, so let's make sure we take the time today or tomorrow to settle on this. Of course no need to rush either so in case there are valid concerns already and we know of concrete examples for which we clearly need to have a different solution before we can continue than I am still ok to delay ofc.

[GlenDC]

FWIW I consider the current extension system we have as "not leaking". A system which leaks info is in my book a system which leaks info about things not related to the current context. But in our system all info is about the connection/request/stream the user is in as we only ever go deeper and deeper, not side-ways.

Of course a rama user could make something leaky on purpose by using an Arc to make a pseudo-global in sheep clothing, but that's on the developer and hopefully they have a good reason for that. Not something I'll worry about.

[soundofspace]

Example setup

• Client connects to proxy that is exposed with TLS

• TLS negotiates http2 -> TargetHttpVersion is configured to http2

• Http2 connection is opened to Proxy

• Do proxied https request to a website that doesn't negotiate http version over TLS. (So probably means http1.1 should be used). This means we don't explicitly set TargetHttpVersion for this connection

• Http2 connect request to proxy server. Server opens tcp tunnel to website

• Upgraded connection inherits all Extensions including TargetHttpVersion

• HttpConnector and HttpService will use the TargetHttpVersion from the underlying connection and assume it was set for this connection

This will result in a broken connection since TargetHttpVersion "leaks" in this case. When I mention that something leaks, I don't necessarily mean that the entire system leaks, but like you mentioned it leaks info that is not valid for the current context. The question here is what is the current context? TargetHttpVersion is valid for the global / underlying context, but not specifically to this request / the inner upgraded connection.

This is not the best example, but it's the first one that I thought of.

[GlenDC]

yeah but that's what I mean with ensuring a proper use of the type system. This is not about leaking info this is about making sure extension types are not ambigious. The Ingres/Egress wrapper is one step in the right direction.

Here I actually think that TargetHttpVersion should never even have been injected. We already have correctly the differentiator there of impl<S, Input> Service<Input> for TlsConnector<S, ConnectorKindTunnel>, problem is that it's calling

#[cfg(feature = "http")]
        set_target_http_version(
            input.extensions(),
            conn.extensions_mut(),
            &negotiated_params,
        )?;

which it shouldn't do as a tunnel connector has nothing to do with the application layer. It's as easy as:

1. add a unit test to ensure that is not injected when using a tunnel connector (already correctly created)
2. fix the test by removing that call

As easy as that.

For other things we might:

• need to introduce another wrapper / or have separate types for separate use cases, etc...
• for some we might even still need to provide ability for it to know about its context at creation time (here we already did correctly differentiate, just made a logic bug)

So that solves that case. As such I am still convinced that we can always solve this within the spirit I bring here forward.