Skip to content

Conversation

@ernolf
Copy link
Contributor

@ernolf ernolf commented Sep 30, 2025

Summary

This patch updates the internal Nextcloud HTTP client to prefer HTTP/2 where available and to advertise Brotli ("br") in the Accept-Encoding header when the native php-brotli extension is present. The change is designed to be fully backwards compatible: if Brotli support isn't available in the runtime, the client behaves as before (gzip) and cURL will fall back to HTTP/1.1 if HTTP/2 is not supported by the server or the libcurl build.

Motivation

  • HTTP/2 offers multiplexing, header compression and latency improvements for outgoing requests which can improve performance and resource utilization.
  • Brotli generally provides better compression ratios than gzip for many text-based resources, reducing bandwidth and improving perceived latency when both client and server support it.
  • The changes provide potential performance wins without requiring mandatory runtime changes: environments without php-brotli continue to operate as before.

What changed:

  • Set RequestOptions::VERSION => '2.0' to indicate a preference for HTTP/2 at the PSR-7 request level.
  • Add a cURL hint: CURLOPT_HTTP_VERSION is set to prefer HTTP/2 via CURL_HTTP_VERSION_2TLS (or CURL_HTTP_VERSION_2_0 as fallback) when those constants are available. This leverages ALPN for protocol negotiation and gracefully falls back to HTTP/1.1 when necessary.
  • Accept-Encoding header handling:
    • If the caller has already provided an Accept-Encoding header, we preserve it.
    • If not, we create/set Accept-Encoding to include br when the php-brotli extension is available (detected via function_exists('brotli_uncompress')), otherwise we keep gzip.
  • Defensive checks are used around curl constants to avoid breakage on older libcurl builds.

Backwards compatibility:

  • Fully backwards compatible: without the php-brotli extension the client will continue to request gzip only. If libcurl or the remote server does not support HTTP/2, the connection falls back to HTTP/1.1 automatically.

TODO

  • ...

Checklist

@ernolf ernolf requested a review from a team as a code owner September 30, 2025 13:03
@ernolf ernolf requested review from ArtificialOwl, SystemKeeper, blizzz, leftybournes, nfebe, nickvergessen and szaimen and removed request for a team September 30, 2025 13:03
- Prefer HTTP/2 by setting RequestOptions::VERSION => "2.0" so clients
  that respect PSR-7 request version will prefer HTTP/2.
- Add a curl hint (CURLOPT_HTTP_VERSION) to prefer HTTP/2 via ALPN
  (CURL_HTTP_VERSION_2TLS or CURL_HTTP_VERSION_2_0 fallback) while allowing
  automatic fallback to HTTP/1.1.
- Advertise Brotli ("br") in Accept-Encoding when the php-brotli extension
  is available (detected via function_exists('brotli_uncompress')), otherwise
  fall back to gzip.

Notes:
- The PSR-7 request version is used as a hint for HTTP client libraries;
  setting the version to "2.0" signals a preference for HTTP/2 at the request
  abstraction level.
- The curl option is defensive: it prefers HTTP/2 where libcurl supports it
  (via ALPN), but will not break on older libcurl/builds (uses defined()).

Compatibility:
- Fully backwards compatible: if the php-brotli extension is not present,
  no Brotli usage will occur and behaviour remains equivalent to previous
  (gzip).

Signed-off-by: ernolf <raphael.gradenwitz@googlemail.com>
@ernolf ernolf force-pushed the ernolf/enh/http2-brotli-client branch from 4b4ab4d to 11785c9 Compare September 30, 2025 15:02
Signed-off-by: ernolf <raphael.gradenwitz@googlemail.com>
@ernolf ernolf force-pushed the ernolf/enh/http2-brotli-client branch from 11785c9 to 65aa731 Compare September 30, 2025 16:06
@szaimen szaimen added enhancement 3. to review Waiting for reviews labels Oct 1, 2025
@szaimen szaimen added this to the Nextcloud 33 milestone Oct 1, 2025
@szaimen szaimen requested a review from tcitworld October 1, 2025 08:24
@cwilson397

This comment was marked as spam.

@nickvergessen nickvergessen removed their request for review October 7, 2025 14:28
@github-actions
Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@miaulalala
Copy link
Contributor

Hey @ernolf, are you ok with adding the changes that Carl and I suggested? If you don't have the time or energy, let us know and one of us can take over and finish the PR :) Cheers!

Signed-off-by: ernolf <raphael.gradenwitz@googlemail.com>
@ernolf
Copy link
Contributor Author

ernolf commented Oct 22, 2025

are you ok with adding the changes that Carl and I suggested?

✅ - Done!

@Altahrim
Copy link
Collaborator

Altahrim commented Jan 8, 2026

Can you fix the conflict please?

ernolf and others added 2 commits January 8, 2026 21:02
Signed-off-by: Raphael Gradenwitz <39901936+ernolf@users.noreply.github.com>
Signed-off-by: ernolf <raphael.gradenwitz@googlemail.com>
@ernolf
Copy link
Contributor Author

ernolf commented Jan 9, 2026

Can you fix the conflict please?

✅ Conflicts resolved and pushed.

I was not able to fix the failing Cypress / runner setup job, which causes the required Cypress / cypress-summary check to fail.

This appears to be unrelated to the changes in this PR (the remaining Cypress failure is about app loading, not HTTP client behavior).
Please let me know if you want me to investigate further or if this needs CI-side adjustment.

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

🐘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants