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

[0.23] Error parsing address from Jetty with ipv6 #4952

Closed
perok opened this issue Jul 8, 2021 · 10 comments
Closed

[0.23] Error parsing address from Jetty with ipv6 #4952

perok opened this issue Jul 8, 2021 · 10 comments
Labels
bug Determined to be a bug in http4s good first issue A tractable issue for those looking to make an initial contribution module:jetty-server

Comments

@perok
Copy link
Contributor

perok commented Jul 8, 2021

Running http4s 0.23.0-RC1 with Jetty and IPv6 crashes with the following:

ERROR org.http4s.servlet.Http4sServlet - Error processing request
java.util.NoSuchElementException: None.get

The issue is that Jetty returns the following addresses:

req.getLocalAddr -> "[0:0:0:0:0:0:0:1]"
req.getRemoteAddr -> "[0:0:0:0:0:0:0:1]"

This crashes the following parsing IpAddress.fromString(req.getRemoteAddr).getin Http4sServlet. Is brackets on the address considered an error on the parsing side or on Jetty's side?

A temporary workaround for us is to force ipv4 with -Djava.net.preferIPv4Stack=true.

@rossabaker rossabaker added bug Determined to be a bug in http4s module:jetty-server labels Jul 9, 2021
@rossabaker
Copy link
Member

[hangs head in shame for using .get]

This looks related to jetty/jetty.project#1503. Rather than say Jetty or ip4s is wrong, I'd say http4s is wrong for assuming they agree. I think we should strip brackets before that call.

Arguably, ip4s should handle both, like the Java APIs. But that's up to @mpilquist.

@rossabaker rossabaker added the good first issue A tractable issue for those looking to make an initial contribution label Jul 9, 2021
@mpilquist
Copy link
Contributor

No strong opinions. At first glance, I'd say that bracketed v6 is part of the URI RFC, not IPv6, and hence the brackets should be removed prior to calling IpAddress.fromString. But I very well may be missing something and it may be more practical to handle it in ip4s regardless. Let me know what you think.

@rossabaker
Copy link
Member

I think this Jetty comment summarizes the grim situation from the specs. Since I used it as a drop-in for the Java libraries, the argument that it should be compatible with the Java libraries seems good to me.

Regardless, I think we should fix it here and not rely on the generosity of ip4s' parser.

@mpilquist
Copy link
Contributor

😱

@rossabaker
Copy link
Member

@san-coding I don't think anybody has taken this up, if you're still looking for one.

@san-coding
Copy link
Contributor

san-coding commented Sep 4, 2021

@san-coding I don't think anybody has taken this up, if you're still looking for one.

Thanks ross, yes I am looking forward to contribute , I am always refreshing the issues page and looking for issues I can contribute to

@san-coding
Copy link
Contributor

san-coding commented Sep 4, 2021

@rossabaker @mpilquist , I have figured out that I have to change this file servlet/src/main/scala/org/http4s/servlet/Http4sServlet.scala , could you help me out on what changes I should make

@san-coding
Copy link
Contributor

san-coding commented Sep 4, 2021

@rossabaker @mpilquist , Is this where I should make changes ? Should we call the stripBracketsFromAddr() function before fromString()

local = SocketAddress(
IpAddress.fromString(stripBracketsFromAddr(req.getLocalAddr)).get,
Port.fromInt(req.getLocalPort).get),
remote = SocketAddress(
IpAddress.fromString(stripBracketsFromAddr(req.getRemoteAddr)).get,
Port.fromInt(req.getRemotePort).get),
secure = req.isSecure
)

@rossabaker
Copy link
Member

I'm sorry. That stripBracketsFromAddr already fixes this, and it wasn't linked, so I neglected to close it. I'll look for another one.

@san-coding
Copy link
Contributor

I'm sorry. That stripBracketsFromAddr already fixes this, and it wasn't linked, so I neglected to close it. I'll look for another one.

Ohh okay , thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Determined to be a bug in http4s good first issue A tractable issue for those looking to make an initial contribution module:jetty-server
Projects
None yet
Development

No branches or pull requests

4 participants