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

fix: Service utilize httputil.SafeHttpClient #1926

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jentfoo
Copy link
Contributor

@jentfoo jentfoo commented Feb 18, 2025

Proposed Changes

This PR follows #1910 by updating service to utilize the new httputil helper for constructing a client which wont follow redirects, and has sensible timeouts. To quote that description:

This change switches us from maintaining a tls config which we then on-demand initialize an http.Client with to instead maintain and reuse an http.Client instance. This enables us to utilize the connection pooling which occurs within the http.Transport to reduce ssl handshakes and thus reduce latency.

In addition this change provides us a central place to configure out http.Client (httputil). Allowing us to easily set configuration options to reduce the security risks of using an unconfigured http.Client. Notably timeouts to reduce DoS risks, and control around following redirects to prevent blind SSRF's.

The prior auth API was marked as deprecated. There is no remaining use within this repo, so it may be able to be removed.

Merging this should fully resolve #1891.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

This PR follows #1910 by updating `service` to utilize the new `httputil` helper for constructing a client which wont follow redirects, and has sensible timeouts.

The prior auth API was marked as deprecated. There is no remaining use within this repo, so it may be able to be removed.

Merging this should fully resolve #1891.
@jentfoo jentfoo self-assigned this Feb 18, 2025
@jentfoo jentfoo requested review from a team as code owners February 18, 2025 22:52
@jentfoo jentfoo marked this pull request as draft February 18, 2025 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve http.Client usage
1 participant