Skip to content
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

Obtaining client address without HttpServletRequest #1258

Open
mkarg opened this issue Apr 10, 2024 · 7 comments
Open

Obtaining client address without HttpServletRequest #1258

mkarg opened this issue Apr 10, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@mkarg
Copy link
Contributor

mkarg commented Apr 10, 2024

There are several use cases where it makes sense to obtain the client's IP address (and possibly port). For that sake, HttpServletRequest provides the methods getRemotePort() and getRemoteAddr().

While this should be sufficient for most scenarios, it binds the application to the Servlet API, just for calling a single method.

As JAX-RS's binding to the Servlet API isn't mandatory (e. g. a Java SE standalone standalone server might implement JAX-RS directly without relying on the Servlet API), this use case should be possible without the use of the Server API. Therefore I suggest to enhance the Request class by getRemotePort() and getRemoteAddr().

@mkarg mkarg added the enhancement New feature or request label Apr 10, 2024
@mkarg
Copy link
Contributor Author

mkarg commented Apr 10, 2024

I thought we discussed this topic already months back, but I could not find the issue / PR anymore. If I am right and this is a duplicate, please post a reference to the original discussion here. Thanks. 🤔

@jamezp
Copy link
Contributor

jamezp commented Apr 10, 2024

One thing I want to mention is I've heard rumblings, I don't know if there is anything concrete as I've not been involved, that there may end up being a Jakarta HTTP API specification at some point.

I'm only mentioning this so we don't end up in another scenario like the @Context and CDI situation we find ourselves in now. I do realize this specific case would likely be less of an issue though as it would just delegate.

@mkarg
Copy link
Contributor Author

mkarg commented Apr 10, 2024

One thing I want to mention is I've heard rumblings, I don't know if there is anything concrete as I've not been involved, that there may end up being a Jakarta HTTP API specification at some point.

I'm only mentioning this so we don't end up in another scenario like the @Context and CDI situation we find ourselves in now. I do realize this specific case would likely be less of an issue though as it would just delegate.

I do not see any relation to this issue at all. If we add the proposed methods, it is up to the implementation to get the address and port somewhere. Whether it internally refers to Servlet API, HTTP API, a proprietary API, or simply solves the request on its own, does in no way bring us into such a situation like @Context and CDI. Maybe I missed something?

@jamezp
Copy link
Contributor

jamezp commented Apr 10, 2024

My only thought was if this new API had something like HttpRequest.remotePort() and HttpRequest.remoteAddress() and you could inject the HttpRequest we now have two ways of getting the remote address/port.

In this case likely not a big deal to have it duplicated in two places. We don't even know if that new API will come to fruition. I just wanted to put it out there that there is potential for duplication.

@jansupol
Copy link
Contributor

The Server the implementation is hooked in does not need to provide the information. For instance, I can see Grizzly and Jetty does so, whereas in Netty I cannot find such information.

@mkarg
Copy link
Contributor Author

mkarg commented Apr 11, 2024

My only thought was if this new API had something like HttpRequest.remotePort() and HttpRequest.remoteAddress() and you could inject the HttpRequest we now have two ways of getting the remote address/port.

In this case likely not a big deal to have it duplicated in two places. We don't even know if that new API will come to fruition. I just wanted to put it out there that there is potential for duplication.

I do not see a problem in duplication.

@mkarg
Copy link
Contributor Author

mkarg commented Apr 11, 2024

The Server the implementation is hooked in does not need to provide the information. For instance, I can see Grizzly and Jetty does so, whereas in Netty I cannot find such information.

ChatGPT told me Netty will do it like this:

import io.netty.bootstrap.ServerBootstrap;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelInitializer;
import io.netty.channel.ChannelOption;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.SocketChannel;
import io.netty.channel.socket.nio.NioServerSocketChannel;
import io.netty.handler.codec.http.HttpRequestDecoder;
import io.netty.handler.codec.http.HttpResponseEncoder;
import io.netty.handler.logging.LogLevel;
import io.netty.handler.logging.LoggingHandler;

public class NettyHttpServer {
    public static void main(String[] args) throws Exception {
        EventLoopGroup bossGroup = new NioEventLoopGroup();
        EventLoopGroup workerGroup = new NioEventLoopGroup();
        try {
            ServerBootstrap b = new ServerBootstrap();
            b.group(bossGroup, workerGroup)
                .channel(NioServerSocketChannel.class)
                .handler(new LoggingHandler(LogLevel.INFO))
                .childHandler(new ChannelInitializer<SocketChannel>() {
                    @Override
                    public void initChannel(SocketChannel ch) {
                        ch.pipeline().addLast(new HttpRequestDecoder());
                        ch.pipeline().addLast(new HttpResponseEncoder());
                        ch.pipeline().addLast(new SimpleChannelInboundHandler<FullHttpRequest>() {
                            @Override
                            protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest msg) {
                                System.out.println("Remote Address: " + ctx.channel().remoteAddress());
                                System.out.println("Remote Port: " + ((InetSocketAddress) ctx.channel().remoteAddress()).getPort());
                            }
                        });
                    }
                })
                .option(ChannelOption.SO_BACKLOG, 128)
                .childOption(ChannelOption.SO_KEEPALIVE, true);

            ChannelFuture f = b.bind(8080).sync();
            f.channel().closeFuture().sync();
        } finally {
            workerGroup.shutdownGracefully();
            bossGroup.shutdownGracefully();
        }
    }
}

@mkarg mkarg self-assigned this Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants