From 97b86d453b612e5f119abecadd239409f88a9793 Mon Sep 17 00:00:00 2001 From: argha-c <007.argha@gmail.com> Date: Mon, 16 Nov 2020 13:41:37 -0800 Subject: [PATCH] [zuul-core] Remove deprecated port attrs. Use `_ADDRESS` instead. --- .../common/SourceAddressChannelHandler.java | 78 +++++++------------ .../HAProxyMessageChannelHandler.java | 2 - .../server/http2/Http2StreamInitializer.java | 2 - .../ElbProxyProtocolChannelHandlerTest.java | 42 +++++----- 4 files changed, 47 insertions(+), 77 deletions(-) diff --git a/zuul-core/src/main/java/com/netflix/netty/common/SourceAddressChannelHandler.java b/zuul-core/src/main/java/com/netflix/netty/common/SourceAddressChannelHandler.java index 8071962ecc..4548543234 100644 --- a/zuul-core/src/main/java/com/netflix/netty/common/SourceAddressChannelHandler.java +++ b/zuul-core/src/main/java/com/netflix/netty/common/SourceAddressChannelHandler.java @@ -32,19 +32,17 @@ import javax.annotation.Nullable; /** - * Stores the source IP address as an attribute of the channel. This has the advantage of allowing - * us to overwrite it if we have more info (eg. ELB sends a HAProxyMessage with info of REAL source - * host + port). - * - * User: michaels@netflix.com - * Date: 4/14/16 - * Time: 4:29 PM + * Stores the source IP address as an attribute of the channel. This has the advantage of allowing us to overwrite it if + * we have more info (eg. ELB sends a HAProxyMessage with info of REAL source host + port). + *

+ * User: michaels@netflix.com Date: 4/14/16 Time: 4:29 PM */ @ChannelHandler.Sharable public final class SourceAddressChannelHandler extends ChannelInboundHandlerAdapter { + /** - * Indicates the actual source (remote) address of the channel. This can be different than the - * one {@link Channel} returns if the connection is being proxied. (e.g. over HAProxy) + * Indicates the actual source (remote) address of the channel. This can be different than the one {@link Channel} + * returns if the connection is being proxied. (e.g. over HAProxy) */ public static final AttributeKey ATTR_REMOTE_ADDR = AttributeKey.newInstance("_remote_addr"); @@ -54,30 +52,22 @@ public final class SourceAddressChannelHandler extends ChannelInboundHandlerAdap public static final AttributeKey ATTR_PROXY_PROTOCOL_DESTINATION_ADDRESS = AttributeKey.newInstance("_proxy_protocol_destination_address"); - /** Use {@link #ATTR_REMOTE_ADDR} instead. */ + /** + * Use {@link #ATTR_REMOTE_ADDR} instead. + */ @Deprecated public static final AttributeKey ATTR_SOURCE_INET_ADDR = AttributeKey.newInstance("_source_inet_addr"); /** - * The host address of the source. This is derived from {@link #ATTR_REMOTE_ADDR}. If the - * address is an IPv6 address, the scope identifier is absent. + * The host address of the source. This is derived from {@link #ATTR_REMOTE_ADDR}. If the address is an IPv6 + * address, the scope identifier is absent. */ public static final AttributeKey ATTR_SOURCE_ADDRESS = AttributeKey.newInstance("_source_address"); /** - * Indicates the actual source (remote) port of the channel, if present. This can be different - * than the one {@link Channel} returns if the connection is being proxies. (e.g. over - * HAProxy). - * @deprecated use {@link #ATTR_REMOTE_ADDR} instead, and check if it is an {@code - * InetSocketAddress}. - */ - @Deprecated - public static final AttributeKey ATTR_SOURCE_PORT = AttributeKey.newInstance("_source_port"); - - /** - * Indicates the local address of the channel. This can be different than the - * one {@link Channel} returns if the connection is being proxied. (e.g. over HAProxy) + * Indicates the local address of the channel. This can be different than the one {@link Channel} returns if the + * connection is being proxied. (e.g. over HAProxy) */ public static final AttributeKey ATTR_LOCAL_ADDR = AttributeKey.newInstance("_local_addr"); @@ -89,33 +79,27 @@ public final class SourceAddressChannelHandler extends ChannelInboundHandlerAdap AttributeKey.newInstance("_local_inet_addr"); /** - * The local address of this channel. This is derived from {@code channel.localAddress()}, or from the - * Proxy Protocol preface if provided. If the address is an IPv6 address, the scope identifier is absent. - * Unlike {@link #ATTR_SERVER_LOCAL_ADDRESS}, this value is overwritten with the Proxy Protocol local address - * (e.g. the LB's local address), if enabled. + * The local address of this channel. This is derived from {@code channel.localAddress()}, or from the Proxy + * Protocol preface if provided. If the address is an IPv6 address, the scope identifier is absent. Unlike {@link + * #ATTR_SERVER_LOCAL_ADDRESS}, this value is overwritten with the Proxy Protocol local address (e.g. the LB's local + * address), if enabled. */ public static final AttributeKey ATTR_LOCAL_ADDRESS = AttributeKey.newInstance("_local_address"); /** - * The port number of the local socket, or {@code -1} if not appropriate. Use {@link #ATTR_LOCAL_ADDR} instead. - */ - @Deprecated - public static final AttributeKey ATTR_LOCAL_PORT = AttributeKey.newInstance("_local_port"); - - - /** - * The actual local address of the channel, in string form. If the address is an IPv6 address, the scope - * identifier is absent. Unlike {@link #ATTR_LOCAL_ADDRESS}, this is not overwritten by the Proxy Protocol message - * if present. + * The actual local address of the channel, in string form. If the address is an IPv6 address, the scope identifier + * is absent. Unlike {@link #ATTR_LOCAL_ADDRESS}, this is not overwritten by the Proxy Protocol message if + * present. * * @deprecated Use {@code channel.localAddress()} instead. */ @Deprecated - public static final AttributeKey ATTR_SERVER_LOCAL_ADDRESS = AttributeKey.newInstance("_server_local_address"); + public static final AttributeKey ATTR_SERVER_LOCAL_ADDRESS = + AttributeKey.newInstance("_server_local_address"); /** - * The port number of the local socket, or {@code -1} if not appropriate. Unlike {@link #ATTR_LOCAL_PORT}, this - * is not overwritten by the Proxy Protocol message if present. + * The port number of the local socket, or {@code -1} if not appropriate. This is not overwritten by the Proxy + * Protocol message if present. * * @deprecated Use {@code channel.localAddress()} instead. */ @@ -123,19 +107,15 @@ public final class SourceAddressChannelHandler extends ChannelInboundHandlerAdap public static final AttributeKey ATTR_SERVER_LOCAL_PORT = AttributeKey.newInstance("_server_local_port"); @Override - public void channelActive(ChannelHandlerContext ctx) throws Exception - { + public void channelActive(ChannelHandlerContext ctx) throws Exception { ctx.channel().attr(ATTR_REMOTE_ADDR).set(ctx.channel().remoteAddress()); InetSocketAddress sourceAddress = sourceAddress(ctx.channel()); ctx.channel().attr(ATTR_SOURCE_INET_ADDR).setIfAbsent(sourceAddress); ctx.channel().attr(ATTR_SOURCE_ADDRESS).setIfAbsent(getHostAddress(sourceAddress)); - ctx.channel().attr(ATTR_SOURCE_PORT).setIfAbsent(sourceAddress.getPort()); - ctx.channel().attr(ATTR_LOCAL_ADDR).set(ctx.channel().localAddress()); InetSocketAddress localAddress = localAddress(ctx.channel()); ctx.channel().attr(ATTR_LOCAL_INET_ADDR).setIfAbsent(localAddress); ctx.channel().attr(ATTR_LOCAL_ADDRESS).setIfAbsent(getHostAddress(localAddress)); - ctx.channel().attr(ATTR_LOCAL_PORT).setIfAbsent(localAddress.getPort()); // ATTR_LOCAL_ADDRESS and ATTR_LOCAL_PORT get overwritten with what is received in // Proxy Protocol (via the LB), so set local server's address, port explicitly ctx.channel().attr(ATTR_SERVER_LOCAL_ADDRESS).setIfAbsent(localAddress.getAddress().getHostAddress()); @@ -168,8 +148,7 @@ static String getHostAddress(InetSocketAddress socketAddress) { } } - private InetSocketAddress sourceAddress(Channel channel) - { + private InetSocketAddress sourceAddress(Channel channel) { SocketAddress remoteSocketAddr = channel.remoteAddress(); if (null != remoteSocketAddr && InetSocketAddress.class.isAssignableFrom(remoteSocketAddr.getClass())) { InetSocketAddress inetSocketAddress = (InetSocketAddress) remoteSocketAddr; @@ -180,8 +159,7 @@ private InetSocketAddress sourceAddress(Channel channel) return null; } - private InetSocketAddress localAddress(Channel channel) - { + private InetSocketAddress localAddress(Channel channel) { SocketAddress localSocketAddress = channel.localAddress(); if (null != localSocketAddress && InetSocketAddress.class.isAssignableFrom(localSocketAddress.getClass())) { InetSocketAddress inetSocketAddress = (InetSocketAddress) localSocketAddress; diff --git a/zuul-core/src/main/java/com/netflix/netty/common/proxyprotocol/HAProxyMessageChannelHandler.java b/zuul-core/src/main/java/com/netflix/netty/common/proxyprotocol/HAProxyMessageChannelHandler.java index 6f6e53ed4b..a0a1190a46 100644 --- a/zuul-core/src/main/java/com/netflix/netty/common/proxyprotocol/HAProxyMessageChannelHandler.java +++ b/zuul-core/src/main/java/com/netflix/netty/common/proxyprotocol/HAProxyMessageChannelHandler.java @@ -53,7 +53,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception String destinationAddress = hapm.destinationAddress(); if (destinationAddress != null) { channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDRESS).set(destinationAddress); - channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_PORT).set(hapm.destinationPort()); SocketAddress addr; out: { @@ -84,7 +83,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception String sourceAddress = hapm.sourceAddress(); if (sourceAddress != null) { channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS).set(sourceAddress); - channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_PORT).set(hapm.sourcePort()); SocketAddress addr; out: diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/server/http2/Http2StreamInitializer.java b/zuul-core/src/main/java/com/netflix/zuul/netty/server/http2/Http2StreamInitializer.java index db2ef7e8c4..815d77980a 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/netty/server/http2/Http2StreamInitializer.java +++ b/zuul-core/src/main/java/com/netflix/zuul/netty/server/http2/Http2StreamInitializer.java @@ -92,10 +92,8 @@ protected void copyAttrsFromParentChannel(Channel parent, Channel child) AttributeKey[] attributesToCopy = { SourceAddressChannelHandler.ATTR_LOCAL_ADDRESS, SourceAddressChannelHandler.ATTR_LOCAL_INET_ADDR, - SourceAddressChannelHandler.ATTR_LOCAL_PORT, SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS, SourceAddressChannelHandler.ATTR_SOURCE_INET_ADDR, - SourceAddressChannelHandler.ATTR_SOURCE_PORT, SourceAddressChannelHandler.ATTR_SERVER_LOCAL_ADDRESS, SourceAddressChannelHandler.ATTR_SERVER_LOCAL_PORT, SourceAddressChannelHandler.ATTR_PROXY_PROTOCOL_DESTINATION_ADDRESS, diff --git a/zuul-core/src/test/java/com/netflix/netty/common/proxyprotocol/ElbProxyProtocolChannelHandlerTest.java b/zuul-core/src/test/java/com/netflix/netty/common/proxyprotocol/ElbProxyProtocolChannelHandlerTest.java index 1f121e53b7..45a5aa8324 100644 --- a/zuul-core/src/test/java/com/netflix/netty/common/proxyprotocol/ElbProxyProtocolChannelHandlerTest.java +++ b/zuul-core/src/test/java/com/netflix/netty/common/proxyprotocol/ElbProxyProtocolChannelHandlerTest.java @@ -44,7 +44,7 @@ @RunWith(MockitoJUnitRunner.class) public class ElbProxyProtocolChannelHandlerTest { - + @Mock private Registry registry; @Mock @@ -58,7 +58,8 @@ public void setup() { @Test public void noProxy() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, false)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, false)); ByteBuf buf = Unpooled.wrappedBuffer( "PROXY TCP4 192.168.0.1 124.123.111.111 10008 443\r\n".getBytes(StandardCharsets.US_ASCII)); channel.writeInbound(buf); @@ -73,16 +74,15 @@ public void noProxy() { assertNull(channel.attr(HAProxyMessageChannelHandler.ATTR_HAPROXY_MESSAGE).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDRESS).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDR).get()); - assertNull(channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_PORT).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS).get()); - assertNull(channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_PORT).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_REMOTE_ADDR).get()); } @Test public void extraDataForwarded() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); ByteBuf buf = Unpooled.wrappedBuffer( "PROXY TCP4 192.168.0.1 124.123.111.111 10008 443\r\nPOTATO".getBytes(StandardCharsets.US_ASCII)); channel.writeInbound(buf); @@ -98,7 +98,8 @@ public void extraDataForwarded() { @Test public void passThrough_ProxyProtocolEnabled_nonProxyBytes() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); //Note that the bytes aren't prefixed by PROXY, as required by the spec ByteBuf buf = Unpooled.wrappedBuffer( "TCP4 192.168.0.1 124.123.111.111 10008 443\r\n".getBytes(StandardCharsets.US_ASCII)); @@ -114,16 +115,15 @@ public void passThrough_ProxyProtocolEnabled_nonProxyBytes() { assertNull(channel.attr(HAProxyMessageChannelHandler.ATTR_HAPROXY_MESSAGE).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDRESS).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDR).get()); - assertNull(channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_PORT).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS).get()); - assertNull(channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_PORT).get()); assertNull(channel.attr(SourceAddressChannelHandler.ATTR_REMOTE_ADDR).get()); } @Test public void incrementCounterWhenPPEnabledButNonHAPMMessage() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); //Note that the bytes aren't prefixed by PROXY, as required by the spec ByteBuf buf = Unpooled.wrappedBuffer( "TCP4 192.168.0.1 124.123.111.111 10008 443\r\n".getBytes(StandardCharsets.US_ASCII)); @@ -140,7 +140,8 @@ public void incrementCounterWhenPPEnabledButNonHAPMMessage() { @Test public void detectsSplitPpv1Message() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); ByteBuf buf1 = Unpooled.wrappedBuffer( "PROXY TCP4".getBytes(StandardCharsets.US_ASCII)); channel.writeInbound(buf1); @@ -161,7 +162,8 @@ public void detectsSplitPpv1Message() { @Test public void negotiateProxy_ppv1_ipv4() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); ByteBuf buf = Unpooled.wrappedBuffer( "PROXY TCP4 192.168.0.1 124.123.111.111 10008 443\r\n".getBytes(StandardCharsets.US_ASCII)); channel.writeInbound(buf); @@ -177,21 +179,18 @@ public void negotiateProxy_ppv1_ipv4() { // in later versions of netty. assertNotNull(channel.attr(HAProxyMessageChannelHandler.ATTR_HAPROXY_MESSAGE).get()); assertEquals("124.123.111.111", channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDRESS).get()); - assertEquals( - new InetSocketAddress(InetAddresses.forString("124.123.111.111"), 443), + assertEquals(new InetSocketAddress(InetAddresses.forString("124.123.111.111"), 443), channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDR).get()); - assertEquals(Integer.valueOf(443), channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_PORT).get()); assertEquals("192.168.0.1", channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS).get()); - assertEquals(Integer.valueOf(10008), channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_PORT).get()); - assertEquals( - new InetSocketAddress(InetAddresses.forString("192.168.0.1"), 10008), + assertEquals(new InetSocketAddress(InetAddresses.forString("192.168.0.1"), 10008), channel.attr(SourceAddressChannelHandler.ATTR_REMOTE_ADDR).get()); } @Test public void negotiateProxy_ppv1_ipv6() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); ByteBuf buf = Unpooled.wrappedBuffer( "PROXY TCP6 ::1 ::2 10008 443\r\n".getBytes(StandardCharsets.US_ASCII)); channel.writeInbound(buf); @@ -210,9 +209,7 @@ public void negotiateProxy_ppv1_ipv6() { assertEquals( new InetSocketAddress(InetAddresses.forString("::2"), 443), channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDR).get()); - assertEquals(Integer.valueOf(443), channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_PORT).get()); assertEquals("::1", channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS).get()); - assertEquals(Integer.valueOf(10008), channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_PORT).get()); assertEquals( new InetSocketAddress(InetAddresses.forString("::1"), 10008), channel.attr(SourceAddressChannelHandler.ATTR_REMOTE_ADDR).get()); @@ -221,7 +218,8 @@ public void negotiateProxy_ppv1_ipv6() { @Test public void negotiateProxy_ppv2_ipv4() { EmbeddedChannel channel = new EmbeddedChannel(); - channel.pipeline().addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); + channel.pipeline() + .addLast(ElbProxyProtocolChannelHandler.NAME, new ElbProxyProtocolChannelHandler(registry, true)); ByteBuf buf = Unpooled.wrappedBuffer( new byte[]{0x0D, 0x0A, 0x0D, 0x0A, 0x00, 0x0D, 0x0A, 0x51, 0x55, 0x49, 0x54, 0x0A, 0x21, 0x11, 0x00, 0x0C, (byte) 0xC0, (byte) 0xA8, 0x00, 0x01, 0x7C, 0x7B, 0x6F, 0x6F, 0x27, 0x18, 0x01, @@ -242,9 +240,7 @@ public void negotiateProxy_ppv2_ipv4() { assertEquals( new InetSocketAddress(InetAddresses.forString("124.123.111.111"), 443), channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_ADDR).get()); - assertEquals(Integer.valueOf(443), channel.attr(SourceAddressChannelHandler.ATTR_LOCAL_PORT).get()); assertEquals("192.168.0.1", channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_ADDRESS).get()); - assertEquals(Integer.valueOf(10008), channel.attr(SourceAddressChannelHandler.ATTR_SOURCE_PORT).get()); assertEquals( new InetSocketAddress(InetAddresses.forString("192.168.0.1"), 10008), channel.attr(SourceAddressChannelHandler.ATTR_REMOTE_ADDR).get());