brotli support in HttpClient #2848

sullis · 2023-07-06T11:57:56Z

enable Brotli compression support in the Reactor http client.

sullis · 2023-07-07T13:25:54Z

@violetagg Please take a look :)

violetagg · 2023-07-10T12:54:48Z

@sullis I can take a look at the end of the week. Can you verify that when we do not have Brotli in the class path the native-image functionality is still working OK? When we have native-image we need to be careful with reflection and Brotli.isAvailable does reflection.

violetagg · 2023-07-19T09:05:59Z

@sullis I can take a look at the end of the week. Can you verify that when we do not have Brotli in the class path the native-image functionality is still working OK? When we have native-image we need to be careful with reflection and Brotli.isAvailable does reflection.

I tested this one

When there is no Brotli in the class path the HttpClient works as expected when native-image

One can see

========================================================================================================================
GraalVM Native Image: Generating 'brotli-example' (executable)...
========================================================================================================================
...
Warning: Could not resolve com.aayushatharva.brotli4j.Brotli4jLoader for reflection configuration. Reason: java.lang.ClassNotFoundException: com.aayushatharva.brotli4j.Brotli4jLoader.
...

10:07:26.303 [main] DEBUG io.netty.handler.codec.compression.Brotli -- brotli4j not in the classpath; Brotli support will be unavailable.

I'm using GraalVM reachability metadata
https://github.com/oracle/graalvm-reachability-metadata/blob/f629b5468b723521023dd05c65f2af5d47de0a24/metadata/io.netty/netty-common/4.1.80.Final/reflect-config.json#L300-L305

When I have Brotli in the class path and I run on Linux the HttpClient works as expected when native-image. The arch is recognised and GraalVM reachability metadata has the proper configuration
https://github.com/oracle/graalvm-reachability-metadata/blob/f629b5468b723521023dd05c65f2af5d47de0a24/metadata/io.netty/netty-common/4.1.80.Final/resource-config.json#L15

When I have Brotli in the class path and I run on Mac OS, I'm seeing the log below and the brotli support is disabled when native-image.

11:19:56.915 [main] DEBUG io.netty.handler.codec.compression.Brotli -- Failed to load brotli4j; Brotli support will be unavailable.
java.lang.UnsupportedOperationException: Unsupported OS and Architecture: Mac OS X, amd64
        at com.aayushatharva.brotli4j.Brotli4jLoader.getPlatform(Brotli4jLoader.java:142)
        at com.aayushatharva.brotli4j.Brotli4jLoader.<clinit>(Brotli4jLoader.java:53)
        at io.netty.handler.codec.compression.Brotli.<clinit>(Brotli.java:46)
        at reactor.netty.http.client.HttpClient.compress(HttpClient.java:521)
        at org.example.Application.main(Application.java:10)

violetagg · 2023-07-19T09:14:15Z

I'm seeing that com.aayushatharva.brotli4j.Brotli4jLoader#getPlatform checks the arch like this

        } else if (osName.startsWith("Mac")) {
            if (archName.equalsIgnoreCase("x86_64")) {
                return "osx-x86_64";
            }

While for example in Netty we do the following:
io.netty.util.internal.PlatformDependent#normalizeArch

        value = normalize(value);
        if (value.matches("^(x8664|amd64|ia32e|em64t|x64)$")) {
            return "x86_64";
        }

sullis · 2023-07-21T03:31:53Z

Interesting. I'll take a closer look this weekend.

violetagg · 2023-09-13T11:41:26Z

@sullis it will be great if you can rebase so that you can pick up the GraalVM smoke tests change.

sullis · 2023-09-14T19:27:26Z

@sullis it will be great if you can rebase so that you can pick up the GraalVM smoke tests change.

Rebase done.

violetagg · 2023-09-15T05:09:06Z

@sullis If you want we can proceed with this. We will state that Brotli supports native image only for linux-x86_64. For the rest it will be disabled.

08:05:21.382 [main] DEBUG io.netty.handler.codec.compression.Brotli - Failed to load brotli4j; Brotli support will be unavailable.
java.lang.UnsatisfiedLinkError: Failed to find Brotli native library in classpath: /lib/osx-aarch64/libbrotli.dylib
        at com.aayushatharva.brotli4j.Brotli4jLoader.<clinit>(Brotli4jLoader.java:77)
        at io.netty.handler.codec.compression.Brotli.<clinit>(Brotli.java:46)
        at reactor.netty.http.client.HttpClient.compress(HttpClient.java:521)

04:57:54.390 [main] DEBUG io.netty.handler.codec.compression.Brotli - Failed to load brotli4j; Brotli support will be unavailable.
java.lang.UnsatisfiedLinkError: Failed to find Brotli native library in classpath: /lib/linux-aarch64/libbrotli.so
        at com.aayushatharva.brotli4j.Brotli4jLoader.<clinit>(Brotli4jLoader.java:77)
        at io.netty.handler.codec.compression.Brotli.<clinit>(Brotli.java:46)
        at reactor.netty.http.client.HttpClient.compress(HttpClient.java:521)

sullis · 2023-09-15T05:18:02Z

@sullis If you want we can proceed with this. We will state that Brotli supports native image only for linux-x86_64. For the rest it will be disabled.

Sounds good to me!

reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientConfig.java

violetagg · 2023-09-18T11:03:11Z

reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClient.java

@@ -515,7 +516,13 @@ public final HttpClient baseUrl(String baseUrl) {
 * @return a new {@link HttpClient}
 */
 public final HttpClient compress(boolean compressionEnabled) {
+ configuration().headers.remove(HttpHeaderNames.ACCEPT_ENCODING);


This will remove any custom Accept-Encoding header that was set prior this invocation.
Why do we need to do this? It will be better to preserve the custom headers.

violetagg · 2023-09-18T11:08:12Z

reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java

+ if (brotliEnabled) {
+ assertThat(Brotli.isAvailable()).isTrue();
+ client = client.compress(true);
+ client.configuration().headers = client.configuration().headers()


Why do you need this? The compress method should add the header.

violetagg · 2023-09-18T11:09:37Z

reactor-netty-http/src/test/java/reactor/netty/http/client/HttpClientTest.java

+ .response((r, buf) -> buf.asString()
+ .elementAt(0)
+ .zipWith(Mono.just(r))))


Suggested change

.response((r, buf) -> buf.asString()

.elementAt(0)

.zipWith(Mono.just(r))))

.responseSingle((r, buf) -> buf.asString().zipWith(Mono.just(r))))

sullis · 2024-03-30T14:09:58Z

rebased

reactor-netty-http/src/test/java/reactor/netty/http/HttpCompressionClientServerTests.java

This change is based on PR #2848

violetagg · 2024-07-02T14:55:58Z

This PR is superseded by #3331

This change is based on PR #2848

violetagg added the type/enhancement A general enhancement label Jul 10, 2023

violetagg modified the milestones: 1.1.9, 1.1.x Backlog Jul 10, 2023

sullis force-pushed the httpclient-brotli branch from 3717140 to 532fd53 Compare September 14, 2023 19:26

violetagg marked this pull request as ready for review September 15, 2023 06:20

violetagg modified the milestones: 1.1.x Backlog, 1.1.12 Sep 15, 2023

violetagg requested changes Sep 18, 2023

View reviewed changes

violetagg modified the milestones: 1.1.12, 1.1.x Backlog Sep 29, 2023

violetagg modified the milestones: 1.1.x Backlog, 1.2.x Backlog Dec 15, 2023

sullis force-pushed the httpclient-brotli branch from 532fd53 to b78270a Compare February 7, 2024 17:15

sullis force-pushed the httpclient-brotli branch 2 times, most recently from 56434f8 to b0b2878 Compare March 20, 2024 14:43

sullis force-pushed the httpclient-brotli branch from b0b2878 to 794e69c Compare March 30, 2024 14:09

brotli support in HttpClient

0b893c4

sullis force-pushed the httpclient-brotli branch from 794e69c to 0b893c4 Compare April 16, 2024 17:10

Merge branch 'main' into httpclient-brotli

8ce0433

violetagg reviewed Jul 1, 2024

View reviewed changes

reactor-netty-http/src/test/java/reactor/netty/http/HttpCompressionClientServerTests.java Show resolved Hide resolved

Fix the build

e562df3

violetagg added a commit that referenced this pull request Jul 2, 2024

Add Brotli support for HttpClient

2112fc9

This change is based on PR #2848

violetagg mentioned this pull request Jul 2, 2024

Add Brotli support for HttpClient #3331

Merged

violetagg added a commit that referenced this pull request Jul 2, 2024

Add Brotli support for HttpClient

73d8887

This change is based on PR #2848

violetagg closed this Jul 2, 2024

violetagg added status/superseded An issue that has been superseded by another ignore-changelog labels Jul 2, 2024

violetagg removed this from the 1.2.x Backlog milestone Jul 2, 2024

violetagg removed ignore-changelog type/enhancement A general enhancement labels Jul 2, 2024

violetagg added a commit that referenced this pull request Jul 3, 2024

Add Brotli support for HttpClient (#3331)

ee46931

This change is based on PR #2848

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

brotli support in HttpClient #2848

brotli support in HttpClient #2848

sullis commented Jul 6, 2023 •

edited

Loading

sullis commented Jul 7, 2023

violetagg commented Jul 10, 2023 •

edited

Loading

violetagg commented Jul 19, 2023

violetagg commented Jul 19, 2023

sullis commented Jul 21, 2023

violetagg commented Sep 13, 2023

sullis commented Sep 14, 2023

violetagg commented Sep 15, 2023 •

edited

Loading

sullis commented Sep 15, 2023

violetagg Sep 18, 2023

violetagg Sep 18, 2023

violetagg Sep 18, 2023

sullis commented Mar 30, 2024

violetagg commented Jul 2, 2024

brotli support in HttpClient #2848

brotli support in HttpClient #2848

Conversation

sullis commented Jul 6, 2023 • edited Loading

sullis commented Jul 7, 2023

violetagg commented Jul 10, 2023 • edited Loading

violetagg commented Jul 19, 2023

violetagg commented Jul 19, 2023

sullis commented Jul 21, 2023

violetagg commented Sep 13, 2023

sullis commented Sep 14, 2023

violetagg commented Sep 15, 2023 • edited Loading

sullis commented Sep 15, 2023

violetagg Sep 18, 2023

Choose a reason for hiding this comment

violetagg Sep 18, 2023

Choose a reason for hiding this comment

violetagg Sep 18, 2023

Choose a reason for hiding this comment

sullis commented Mar 30, 2024

violetagg commented Jul 2, 2024

sullis commented Jul 6, 2023 •

edited

Loading

violetagg commented Jul 10, 2023 •

edited

Loading

violetagg commented Sep 15, 2023 •

edited

Loading