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

Use the request's Host header when setting RE's URI object. #3361

Merged
merged 7 commits into from
Dec 13, 2022

Conversation

jamezp
Copy link
Contributor

@jamezp jamezp commented Dec 13, 2022

https://issues.redhat.com/browse/RESTEASY-3261

Replaces #3296 with one extra commit.

Upstream #3360

Samuel Cox and others added 7 commits October 7, 2022 08:15
We want to make a greater effort at capturing the actual hostname used
by the client.
which means we need to read it from the Host header.  Not sure why
we didn't just copy what the other adapters did.  Honestly, some of
that should probably be abstracted _if_ reading from Host header is
truly the right thing to do.  I _think_ we shied away from this because
it felt wrong to return 'unknown'.  While that still feels wrong, I'm
not sure what is a better option, so copying.  I briefly thought about
returning the IP address, which is what was happening before this effort
to _not_ return the IP address!
from the request.  We discussed 'asserting' here.  Maybe that is the
best way.  I just haven't seen anyone do that in quite a while.
https://issues.redhat.com/browse/RESTEASY-3261
Signed-off-by: James R. Perkins <jperkins@redhat.com>
@jamezp jamezp merged commit f31351a into resteasy:5.0 Dec 13, 2022
@jamezp jamezp deleted the reactor-netty-server-uriinfo branch December 13, 2022 02:29
@@ -95,7 +95,7 @@ public void testCannotDetermineHost() {
try {
extractor.extract(req, "/contextPath");
fail("Inability to determine a host address should have thrown an IllegalArgumentException.");
} catch (final AssertionError ae) {
} catch (final IllegalArgumentException ignore) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, this was an accidental push:(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, I got the test working at least. 5.0.5.Final has been released too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants