garydgregory commented on code in PR #651: URL: https://github.com/apache/httpcomponents-client/pull/651#discussion_r2154470414
########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/ContentDecoderRegistry.java: ########## @@ -0,0 +1,147 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; + +import org.apache.hc.client5.http.entity.BrotliDecompressingEntity; +import org.apache.hc.client5.http.entity.BrotliInputStreamFactory; +import org.apache.hc.client5.http.entity.DeflateInputStreamFactory; +import org.apache.hc.client5.http.entity.GZIPInputStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * Run-time catalogue of all {@link InputStreamFactory} instances that + * implement HTTP <em>content-decoding</em>. + * + * <p>The catalogue is populated once at class-initialisation: + * <ul> + * <li><b>Built-ins</b> – {@code gzip} and {@code deflate} – are registered + * unconditionally via the long-standing singletons + * {@link GZIPInputStreamFactory#getInstance() GZIPInputStreamFactory} + * and {@link DeflateInputStreamFactory#getInstance() + * DeflateInputStreamFactory}.</li> + * <li>When <em>commons-compress</em> is present, the registry attempts to + * add additional codecs (br, zstd, xz, …) using + * {@link CommonsCompressDecoderFactory}. Each codec is included only if + * its own helper JAR (e.g. google-brotli, zstd-jni) is available.</li> + * <li>If Commons did <em>not</em> contribute a <code>br</code> decoder the + * fallback singleton {@link BrotliInputStreamFactory#getInstance()} + * is used when the native Brotli library is on the class-path.</li> + * </ul> + * + * <p>The resulting map is immutable and exposed via + * {@link #snapshot()} for safe concurrent use.</p> + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +public final class ContentDecoderRegistry { + + /** + * Canonical, immutable map built at class-initialisation. + */ + private static final Map<String, InputStreamFactory> REGISTRY = build(); + + + private static Map<String, InputStreamFactory> build() { + final LinkedHashMap<String, InputStreamFactory> m = new LinkedHashMap<>(); + + /* 1. Built-ins (always succeed) */ + register(m, new GZIPInputStreamFactory()); + register(m, new DeflateInputStreamFactory()); + + /* 2. Optional Commons-Compress decoders (reflection, safe) */ + if (commonsCompressPresent()) { + addCommons(m, "br"); + addCommons(m, "zstd"); + addCommons(m, "xz"); + addCommons(m, "lzma"); + addCommons(m, "lz4-framed"); + addCommons(m, "lz4-block"); + addCommons(m, "bzip2"); + addCommons(m, "pack200"); + addCommons(m, "deflate64"); + } + + /* 3. Native Brotli fallback when Commons is not present */ + if (!m.containsKey("br")) { + if (BrotliDecompressingEntity.isAvailable()) { + register(m, new BrotliInputStreamFactory()); + } + } + return m; + } + + + private static void register(final Map<String, InputStreamFactory> map, + final InputStreamFactory f) { + map.put(f.getContentEncoding().toLowerCase(Locale.ROOT), f); + } + + private static void addCommons(final Map<String, InputStreamFactory> map, + final String encoding) { + final InputStreamFactory f = CommonsCompressDecoderFactory.tryCreate(encoding); + if (f != null) { + map.put(encoding.toLowerCase(Locale.ROOT), f); + } + } + + /** + * Quick presence check: is the Commons factory class on the class-path? + */ + private static boolean commonsCompressPresent() { + try { + Class.forName( + "org.apache.commons.compress.compressors.CompressorStreamFactory", + false, + ContentDecoderRegistry.class.getClassLoader()); + return true; + } catch (final ClassNotFoundException e) { Review Comment: What about `LinkageError`? ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/ContentDecoderRegistry.java: ########## @@ -0,0 +1,147 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; + +import org.apache.hc.client5.http.entity.BrotliDecompressingEntity; +import org.apache.hc.client5.http.entity.BrotliInputStreamFactory; +import org.apache.hc.client5.http.entity.DeflateInputStreamFactory; +import org.apache.hc.client5.http.entity.GZIPInputStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * Run-time catalogue of all {@link InputStreamFactory} instances that + * implement HTTP <em>content-decoding</em>. + * + * <p>The catalogue is populated once at class-initialisation: + * <ul> + * <li><b>Built-ins</b> – {@code gzip} and {@code deflate} – are registered + * unconditionally via the long-standing singletons + * {@link GZIPInputStreamFactory#getInstance() GZIPInputStreamFactory} + * and {@link DeflateInputStreamFactory#getInstance() + * DeflateInputStreamFactory}.</li> + * <li>When <em>commons-compress</em> is present, the registry attempts to + * add additional codecs (br, zstd, xz, …) using + * {@link CommonsCompressDecoderFactory}. Each codec is included only if + * its own helper JAR (e.g. google-brotli, zstd-jni) is available.</li> + * <li>If Commons did <em>not</em> contribute a <code>br</code> decoder the + * fallback singleton {@link BrotliInputStreamFactory#getInstance()} + * is used when the native Brotli library is on the class-path.</li> + * </ul> + * + * <p>The resulting map is immutable and exposed via + * {@link #snapshot()} for safe concurrent use.</p> + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +public final class ContentDecoderRegistry { + + /** + * Canonical, immutable map built at class-initialisation. + */ + private static final Map<String, InputStreamFactory> REGISTRY = build(); + + + private static Map<String, InputStreamFactory> build() { + final LinkedHashMap<String, InputStreamFactory> m = new LinkedHashMap<>(); + + /* 1. Built-ins (always succeed) */ + register(m, new GZIPInputStreamFactory()); + register(m, new DeflateInputStreamFactory()); + + /* 2. Optional Commons-Compress decoders (reflection, safe) */ + if (commonsCompressPresent()) { + addCommons(m, "br"); + addCommons(m, "zstd"); + addCommons(m, "xz"); + addCommons(m, "lzma"); + addCommons(m, "lz4-framed"); + addCommons(m, "lz4-block"); + addCommons(m, "bzip2"); + addCommons(m, "pack200"); + addCommons(m, "deflate64"); + } + + /* 3. Native Brotli fallback when Commons is not present */ + if (!m.containsKey("br")) { + if (BrotliDecompressingEntity.isAvailable()) { + register(m, new BrotliInputStreamFactory()); + } + } + return m; + } + + + private static void register(final Map<String, InputStreamFactory> map, + final InputStreamFactory f) { + map.put(f.getContentEncoding().toLowerCase(Locale.ROOT), f); + } + + private static void addCommons(final Map<String, InputStreamFactory> map, + final String encoding) { + final InputStreamFactory f = CommonsCompressDecoderFactory.tryCreate(encoding); + if (f != null) { + map.put(encoding.toLowerCase(Locale.ROOT), f); + } + } + + /** + * Quick presence check: is the Commons factory class on the class-path? + */ + private static boolean commonsCompressPresent() { + try { + Class.forName( + "org.apache.commons.compress.compressors.CompressorStreamFactory", + false, + ContentDecoderRegistry.class.getClassLoader()); + return true; + } catch (final ClassNotFoundException e) { + return false; + } + } + + + /** + * Returns a <em>defensive copy</em> of the internal registry + * (key = canonical encoding token, value = {@link InputStreamFactory}). + */ + public static Map<String, InputStreamFactory> snapshot() { Review Comment: `snapshot()` is a bad method name here IMO, rename to `getRegistry()`. Saying this is a `defensive copy` is misleading because `InputStreamFactory` makes no thread-safety guarantees. I would say this is a "shallow copy" which is technically correct and doesn't imply intent of attack or defense for call sites. ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/ContentDecoderRegistry.java: ########## @@ -0,0 +1,147 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; + +import org.apache.hc.client5.http.entity.BrotliDecompressingEntity; +import org.apache.hc.client5.http.entity.BrotliInputStreamFactory; +import org.apache.hc.client5.http.entity.DeflateInputStreamFactory; +import org.apache.hc.client5.http.entity.GZIPInputStreamFactory; +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * Run-time catalogue of all {@link InputStreamFactory} instances that + * implement HTTP <em>content-decoding</em>. + * + * <p>The catalogue is populated once at class-initialisation: + * <ul> + * <li><b>Built-ins</b> – {@code gzip} and {@code deflate} – are registered + * unconditionally via the long-standing singletons + * {@link GZIPInputStreamFactory#getInstance() GZIPInputStreamFactory} + * and {@link DeflateInputStreamFactory#getInstance() + * DeflateInputStreamFactory}.</li> + * <li>When <em>commons-compress</em> is present, the registry attempts to + * add additional codecs (br, zstd, xz, …) using + * {@link CommonsCompressDecoderFactory}. Each codec is included only if + * its own helper JAR (e.g. google-brotli, zstd-jni) is available.</li> + * <li>If Commons did <em>not</em> contribute a <code>br</code> decoder the + * fallback singleton {@link BrotliInputStreamFactory#getInstance()} + * is used when the native Brotli library is on the class-path.</li> + * </ul> + * + * <p>The resulting map is immutable and exposed via + * {@link #snapshot()} for safe concurrent use.</p> + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +public final class ContentDecoderRegistry { + + /** + * Canonical, immutable map built at class-initialisation. + */ + private static final Map<String, InputStreamFactory> REGISTRY = build(); + + + private static Map<String, InputStreamFactory> build() { + final LinkedHashMap<String, InputStreamFactory> m = new LinkedHashMap<>(); + + /* 1. Built-ins (always succeed) */ + register(m, new GZIPInputStreamFactory()); + register(m, new DeflateInputStreamFactory()); + + /* 2. Optional Commons-Compress decoders (reflection, safe) */ + if (commonsCompressPresent()) { + addCommons(m, "br"); Review Comment: See my other comment about magic strings, for a later PR. ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/BrotliInputStreamFactory.java: ########## @@ -41,6 +41,17 @@ @Contract(threading = ThreadingBehavior.STATELESS) public class BrotliInputStreamFactory implements InputStreamFactory { + /** + * Canonical token for the deflate content-coding. + * @since 5.6 + */ + public static final String ENCODING = "br"; Review Comment: This new constant can be reused in this PR in a few places. ########## httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java: ########## @@ -176,4 +173,16 @@ public ClassicHttpResponse execute( return response; } + private static List<String> buildDefaultAcceptEncoding() { + + + final List<String> list = new ArrayList<>(4); + list.add("gzip"); + list.add("x-gzip"); Review Comment: Maybe a later PR could gather all these magic strings ("x-gzip" and so on) into a class (existing or new). ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/compress/CommonsCompressDecoderFactory.java: ########## @@ -0,0 +1,187 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * <http://www.apache.org/>. + * + */ + +package org.apache.hc.client5.http.entity.compress; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Locale; + +import org.apache.hc.client5.http.entity.InputStreamFactory; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.annotation.ThreadingBehavior; + +/** + * <p>An {@link InputStreamFactory} implementation that delegates to the + * decoder implementations provided by <em>Apache Commons Compress</em> + * (<a href="https://commons.apache.org/proper/commons-compress/";>commons-compress</a>) + * <em>only if</em> the relevant classes are present on the run-time + * class-path.</p> + * + * <p>The class is loaded and initialised entirely by reflection, so + * {@code httpclient5} does <strong>not</strong> declare a build-time + * dependency on Commons Compress, Google Brotli, Zstd-JNI or XZ-Java. When + * any of those JARs are missing the factory simply returns {@code null} and + * the corresponding codec will not be registered in + * {@link ContentDecoderRegistry}.</p> + * + * <h3>Threading model</h3> + * <ul> + * <li>Instances are <em>immutable</em>; all state is {@code final} and + * set during construction.</li> + * <li>The class performs no caching beyond its two final fields.</li> + * <li>Therefore it is safe to share one instance between any number of + * threads. The class is annotated + * {@code @Contract(threading = ThreadingBehavior.STATELESS)} to make + * that guarantee explicit to static analysers.</li> + * </ul> + * + * @since 5.6 + */ +@Internal +@Contract(threading = ThreadingBehavior.STATELESS) +final class CommonsCompressDecoderFactory implements InputStreamFactory { + + /** + * Fully-qualified name of {@code CompressorStreamFactory}. + */ + private static final String CCSF = + "org.apache.commons.compress.compressors.CompressorStreamFactory"; + + + /** + * Attempts to build a Commons-Compress backed decoder for the given + * content-encoding token. + * + * @param encoding canonical content-encoding token + * @return a new {@link InputStreamFactory} when the codec is fully + * supported on the current class-path, or {@code null} otherwise. + */ + static InputStreamFactory tryCreate(final String encoding) { + try { + final Class<?> factoryClass = Class.forName( + CCSF, false, + CommonsCompressDecoderFactory.class.getClassLoader()); + + if (!runtimeAvailable(encoding)) { + return null; + } + return new CommonsCompressDecoderFactory(encoding, factoryClass); + + } catch (final ClassNotFoundException ex) { + return null; + } + } + + private final String encoding; + private final Method createMethod; + + private CommonsCompressDecoderFactory(final String encoding, + final Class<?> factoryClass) { + this.encoding = encoding.toLowerCase(Locale.ROOT); + try { + this.createMethod = factoryClass.getMethod( + "createCompressorInputStream", + String.class, InputStream.class); + } catch (final NoSuchMethodException e) { + throw new IllegalStateException("Commons-Compress API mismatch", e); + } + } + + + @Override + public String getContentEncoding() { + return encoding; + } + + @Override + public InputStream create(final InputStream source) throws IOException { + try { + final Object factory = + createMethod.getDeclaringClass().getConstructor().newInstance(); + return (InputStream) createMethod.invoke(factory, encoding, source); + + } catch (final InvocationTargetException ex) { + throw new IOException( + "Unable to decode Content-Encoding \"" + encoding + '"', + ex.getTargetException()); + + } catch (final ReflectiveOperationException ex) { + throw new IOException( + "Could not instantiate Commons-Compress factory", ex); + } + } + + /** + * Maps encodings that require an additional helper JAR + * to the fully-qualified name of a class that must be present. + */ + private enum CodecProbe { + BR("br", "org.brotli.dec.BrotliInputStream"), + ZSTD("zstd", "com.github.luben.zstd.ZstdInputStream"), + XZ("xz", "org.tukaani.xz.XZInputStream"), + LZMA("lzma", "org.tukaani.xz.XZInputStream"); + + final String encoding; + final String probeClass; + + CodecProbe(final String encoding, final String probeClass) { + this.encoding = encoding; + this.probeClass = probeClass; + } + + static String probeFor(final String enc) { + for (final CodecProbe p : values()) { + if (p.encoding.equals(enc)) { + return p.probeClass; + } + } + return null; // pure-Java codec + } + } + + /** + * Returns {@code true} when all run-time pieces needed for {@code enc} are present. + */ + private static boolean runtimeAvailable(final String enc) { + final String probe = CodecProbe.probeFor(enc.toLowerCase(Locale.ROOT)); + return probe == null || classExists(probe); + } + + private static boolean classExists(final String cn) { + try { + Class.forName(cn, false, + CommonsCompressDecoderFactory.class.getClassLoader()); + return true; + } catch (final ClassNotFoundException e) { Review Comment: What about `LinkageError`? ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/DeflateInputStreamFactory.java: ########## @@ -41,6 +41,17 @@ @Contract(threading = ThreadingBehavior.STATELESS) public class DeflateInputStreamFactory implements InputStreamFactory { + /** + * Canonical token for the deflate content-coding. + * @since 5.6 + */ + public static final String ENCODING = "deflate"; Review Comment: This new constant can be reused in this PR in a few places. ########## httpclient5/src/main/java/org/apache/hc/client5/http/entity/GZIPInputStreamFactory.java: ########## @@ -42,6 +42,17 @@ @Contract(threading = ThreadingBehavior.STATELESS) public class GZIPInputStreamFactory implements InputStreamFactory { + /** + * Canonical token for the gzip content-coding. + * @since 5.6 + */ + public static final String ENCODING = "gzip"; Review Comment: This new constant can be reused in this PR in a few places. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org
