Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Netty Connector doesn't support Followredirects #5048

Merged
merged 2 commits into from
May 17, 2022

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Apr 25, 2022

Relates to #5045

@jbescos jbescos requested review from jansupol and senivam April 26, 2022 05:52
@jbescos jbescos force-pushed the issue5045 branch 2 times, most recently from d09bbb1 to cde8346 Compare April 27, 2022 09:22
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Copy link
Contributor

@jansupol jansupol left a comment

Choose a reason for hiding this comment

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

Looks good. It would even be better if a redirect loop would have been detected, something like org.glassfish.jersey.jdk.connector.internal.RedirectHandler implements.

@@ -46,21 +49,27 @@
*/
class JerseyClientHandler extends SimpleChannelInboundHandler<HttpObject> {

private static final String LOCATION_HEADER = "Location";
Copy link
Contributor

Choose a reason for hiding this comment

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

javax.ws.rs.core.HttpHeaders.Location

@jbescos
Copy link
Member Author

jbescos commented May 13, 2022

Thanks for the comments, I will work on that. I was not aware about the redirect handler, and it looks I should use it.

@jbescos jbescos marked this pull request as draft May 13, 2022 07:08
@jbescos jbescos marked this pull request as ready for review May 13, 2022 09:52
@jbescos jbescos force-pushed the issue5045 branch 4 times, most recently from b37494b to 70a9896 Compare May 13, 2022 10:49
* @see org.glassfish.jersey.client.ClientProperties#FOLLOW_REDIRECTS
* @see org.glassfish.jersey.netty.connector.internal.RedirectException
*/
public static final String MAX_REDIRECTS = "jersey.config.client.NettyConnectorProvider.maxRedirects";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please update the properties documentation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, lets wait for a green build just in case I did something wrong.

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants