Improve connection wrapping #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I am still not very happy about this method, but it should be more complaint with the Proxy Protocol than the previous version. The tests are far from perfect, yey, but I'm getting there. This was much easier in my head than in practice.
Allows me not to add proxies for the net.Conn interface methods that we are not interested in implementing.
This was utterly wrong, we don't want to change this value. What we need instead is a way to tell the user the connection was coming from a proxy and what the remote, proxy and local addresses are. This will be addressed in a following commit.
This won't be visible yet, as the conn struct is not exported.
The previous code didn't really work, and it lacked tests. This commit makes several changes to the previous version and adds some necessary tests (which are far from perfect). The first important change is that the internal `conn` struct now is exported as `Conn`, which will allow users to cast to `*viaproxy.Conn` and access the additional ProxyAddr() method that returns the proxy address. The proxied connection initialization code was also rewrote almost from scratch, and while I'm still not very happy about it (I'm thinking how we can get rid of the buffered connection) it's working better than before, IMO.
inkel
force-pushed
the
feature/improve-conn-wrap
branch
from
4f71974 to
9f5f4c1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request refactors the package extensively. The biggest changes are the addition of tests (finally) and exporting the
Connstructure. This has the additional benefit that nowviaproxy.Connexports aProxyAddrmethod in addition toLocalAddrandRemoteAddrthat provides more information about the addresses of the connection.It also improves reading the Proxy Protocol line. Error management is improving, though is still far from what I'd like to have.
This pull request aims to address most (if not) the issues listed in #1.