Skip to content
This repository was archived by the owner on Feb 24, 2026. It is now read-only.

Remote address in client connection channel#17

Merged
mjallday merged 4 commits into
verygoodsecurity:vgs-editionfrom
viacheslav-fomin-main:remote-address-in-client-connection-channel
Jun 21, 2017
Merged

Remote address in client connection channel#17
mjallday merged 4 commits into
verygoodsecurity:vgs-editionfrom
viacheslav-fomin-main:remote-address-in-client-connection-channel

Conversation

@viacheslav-fomin-main
Copy link
Copy Markdown
Collaborator

Comment thread pom.xml
<artifactId>littleproxy</artifactId>
<packaging>jar</packaging>
<version>1.1.3.2-VGS-SNAPSHOT</version>
<version>1.1.3.3-VGS-SNAPSHOT</version>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will discuss the versioning mechanism when we all get together in one room

LOG.debug("Starting new connection to: {}", remoteAddress);

this.clientConnection.channel.attr(remoteAddressAttrKey).set(remoteAddress);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 channels: client to proxy channel and proxy to destination server channel.

The proxy to server channel is created from client to proxy channel and contains reference to it. This reference (to client to proxy channel) is passed to mitm manager factory from ProxyToServerConnection (when a connection to remote server is established). To select a proper mitm manager (with white listed hosts) we need to pass active server connection host to client to proxy channel so it is accessible from ConditionallyInsecureMitmManagerFactory

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to remove this once the connection has completed? otherwise we're going to maintain a reference to these channels forever won't we?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjallday each time a connection is established a new ClientToProxyConnection class is created with its corresponding new channel here https://github.com/adamfisk/LittleProxy/blob/master/src/main/java/org/littleshoot/proxy/impl/DefaultHttpProxyServer.java#L514
https://github.com/adamfisk/LittleProxy/blob/master/src/main/java/org/littleshoot/proxy/impl/ProxyConnection.java#L591
Also I read a book about netty and there was nothing about cleaning up attributes in the channel, it was said that the attributes are a good way of sharing state between components that may use the same channel (as it is thread safe, it can determine if the channel was called from particular thread and if not put it in queue)

But let me clarify it further so I am 100 percent sure

Copy link
Copy Markdown

@mjallday mjallday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable but I want to know about the cleanup process here. I fear this will accumulate references to the channels and never release them.

*/
private static final int MINIMUM_RECV_BUFFER_SIZE_BYTES = 64;

public static final AttributeKey<InetSocketAddress> remoteAddressAttrKey = AttributeKey.valueOf("remoteAddressAttrKey");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be named REMOTE_ADDRESS_ATTR_KEY?

@viacheslav-fomin-main
Copy link
Copy Markdown
Collaborator Author

@mjallday each time a connection is established a new ClientToProxyConnection class is created with its corresponding new channel here https://github.com/adamfisk/LittleProxy/blob/master/src/main/java/org/littleshoot/proxy/impl/DefaultHttpProxyServer.java#L514
https://github.com/adamfisk/LittleProxy/blob/master/src/main/java/org/littleshoot/proxy/impl/ProxyConnection.java#L591
Also I read a book about netty and there was nothing about cleaning up attributes in the channel, it was said that the attributes are a good way of sharing state between components that may use the same channel (as it is thread safe, it can determine if the channel was called from particular thread and if not put it in queue)

But let me clarify it further so I am 100 percent sure

@viacheslav-fomin-main
Copy link
Copy Markdown
Collaborator Author

@mjallday

  1. a new ClientToProxyConnection is created for every connection: https://github.com/adamfisk/LittleProxy/blob/master/src/main/java/org/littleshoot/proxy/impl/DefaultHttpProxyServer.java#L514
  2. a new channel is created for every ClientToProxyConnection Remote address in client connection channel #17
  3. ClientToProxyConnection owns ProxyToServerConnection
  4. ProxyToServerConnection owns remoteAddress

So this.clientConnection.channel.attr(REMOTE_ADDRESS_ATTR_KEY).set(remoteAddress); should not result in memory leak.

@mjallday mjallday merged commit 26018b2 into verygoodsecurity:vgs-edition Jun 21, 2017
@osklyarenko osklyarenko added this to the HTTPS proxy enhancements milestone Jun 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants