-
Notifications
You must be signed in to change notification settings - Fork 358
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
Add support for Proxy Protocol V2 #404
Add support for Proxy Protocol V2 #404
Conversation
2758fc8
to
61d4b17
Compare
datas = Arrays.copyOf(datas, 4); | ||
// Extract the value | ||
int value = (int) (BufferUtils.getUInt(datas, 0, Integer.BYTES) >> Short.SIZE); | ||
return value & 0xFFFF; |
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 a bit strange. Why not simply return Buffer.getShort() & 0xFFFF;
? (Compare commit c411efb)
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.
You're right. This way is much better. I'll fix this.
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.
Will the Buffer
class ever include a reader for an unsigned short ?
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.
We could add one. Feel free to do so :-) It wasn't needed so far; the SSH protocol doesn't use 16bit values. (I think there is one extension that uses a 16bit value somewhere, check the call hierarchy of getShort() -- which is also why nobody noticed until recently that the implementation was completely broken.)
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.
Thx for the info.
I 've added a Buffer.getUShort()
to improve the code readability of my implementation.
61d4b17
to
8459f28
Compare
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.
Looks good; thanks. Just some nits.
I'm a strong believer in meaningful commit messages. They help so much when one tries to follow git history, or find out in a few years from the git history why certain things were done in a particular way.
I suggest to squash these three commits into one and give it a commit message that gives just a little bit more context. Perhaps
Add support for Proxy Protocol V2
HAProxy protocol V2 has several improvements over the V1 protocol. Add an
acceptor and parsing support for the protocol as described at [1].
Also add a Buffer.getUShort() method to read an unsigned 16bit value from
a Buffer. The proxy protocol V2 uses such values.
[1] https://www.haproxy.org/download/2.7/doc/proxy-protocol.txt
or some such.
public int getUShort() { | ||
return getShort() & 0xFFFF; | ||
} | ||
|
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.
Something wrong with the indentation. TAB vs. 4 spaces? Apache MINA sshd uses spaces throughout.
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.
Indentation fixed.
Sorry for that.
sshd-contrib/pom.xml
Outdated
<groupId>org.assertj</groupId> | ||
<artifactId>assertj-core</artifactId> | ||
<version>3.24.2</version> | ||
</dependency> |
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.
Indentation. Please use 4 spaces.
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.
Fixed.
dbe7a22
to
5ac49bd
Compare
Indentation fixed. |
HAProxy protocol V2 has several improvements over the V1 protocol. Add an acceptor and parsing support for the protocol as described at [1]. Also add a Buffer.getUShort() method to read an unsigned 16bit value from a Buffer. The proxy protocol V2 uses such values. [1] https://www.haproxy.org/download/2.7/doc/proxy-protocol.txt Signed-off-by: f.herbreteau <f.herbreteau@oodrive.com>
5ac49bd
to
8925c4f
Compare
Proposed implementation for PROXY Protocol V2 for use in SSHD.