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

6962 include proxy setting for relative uris 4.x #7425

Conversation

klustria
Copy link
Member

Description:
This is a feature parity change with prior versions related to relativeUris config logic. When proxy is set or host is in no-proxy list, the request uses absolute URI because of Section 5.1.2 Request-URI spec in https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html which states: "The absoluteURI form is REQUIRED when the request is being made to a proxy.". If absoluteURI does not work as the request URI, then relativeUris config can be set to true to override this behavior.

Documentation impact:
When relativeUris is documented as a webclient config, we can add the notes mentioned in the Description above.

Additional notes:
This change can be compared to this code section in 3.x that performs the same logic: https://github.com/helidon-io/helidon/blob/helidon-3.x/webclient/webclient/src/main/java/io/helidon/webclient/WebClientRequestBuilderImpl.java#L756-L763

Description:
This is a feature parity change with prior versions related to relativeUris config logic. When proxy is set or host is in no-proxy list, the request uses absolute URI because of Section 5.1.2 Request-URI spec in https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html which states: "The absoluteURI form is REQUIRED when the request is being made to a proxy.". If absoluteURI does not work as the request URI, then relativeUris config can be set to true to override this behavior.

Documentation impact:
When relativeUris is documented as a webclient config, we can add the notes mentioned in the Description above.

Additional notes:
This change can be compared to this code section in 3.x that performs the same logic: https://github.com/helidon-io/helidon/blob/helidon-3.x/webclient/webclient/src/main/java/io/helidon/webclient/WebClientRequestBuilderImpl.java#L756-L763
@klustria klustria self-assigned this Aug 23, 2023
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 23, 2023
@klustria
Copy link
Member Author

Goal is to resolve issue #6962

@klustria klustria requested a review from jbescos August 24, 2023 04:31
Copy link
Member

@jbescos jbescos left a comment

Choose a reason for hiding this comment

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

LGTM

@klustria klustria merged commit dc39f0a into helidon-io:main Aug 25, 2023
12 checks passed
dalexandrov pushed a commit to dalexandrov/helidon that referenced this pull request Aug 26, 2023
* Include Proxy setting as part of deciding relativeURIs logic

Description:
This is a feature parity change with prior versions related to relativeUris config logic. When proxy is set or host is in no-proxy list, the request uses absolute URI because of Section 5.1.2 Request-URI spec in https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html which states: "The absoluteURI form is REQUIRED when the request is being made to a proxy.". If absoluteURI does not work as the request URI, then relativeUris config can be set to true to override this behavior.

Documentation impact:
When relativeUris is documented as a webclient config, we can add the notes mentioned in the Description above.

Additional notes:
This change can be compared to this code section in 3.x that performs the same logic: https://github.com/helidon-io/helidon/blob/helidon-3.x/webclient/webclient/src/main/java/io/helidon/webclient/WebClientRequestBuilderImpl.java#L756-L763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants