You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by pmouawad <gi...@git.apache.org> on 2017/04/30 21:23:14 UTC

[GitHub] httpclient pull request #75: Add Brotli support

GitHub user pmouawad opened a pull request:

    https://github.com/apache/httpclient/pull/75

    Add Brotli support

    This PR adds support of Brotli decoding to HC.
    It relies on https://github.com/google/brotli/tree/master/java/org/brotli which is under MIT license.
    
    Regards
    @philmdot

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/pmouawad/httpclient brotli

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/httpclient/pull/75.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #75
    
----
commit 0ebfc5a2a9fe47712101a948260f3703fefefdad
Author: pmouawad <p....@ubik-ingenierie.com>
Date:   2017-04-30T21:21:13Z

    Add Brotli support

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    What we can do is to add org.brotli:dec as an optional dependency (mandatory at compile and optional at runtime) and wire it up _only_ if it is actually on the classpath. Users would need to explicitly add the library to the POM in order to enable it. This technique is widely used, for instance, by Spring to reduce its runtime footprint. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Commons Codec is alive and well and released fairly regularly. Disclosure: I am the project chair over at Apache Commons. Feel free to create a Jira or PR in Common Compress ;-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Hello,
    As advised by Oleg and Gary, I opened a PR towards commons-compress.
    
    Regards


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    We're all good :-) I only intended to mention the project is alive and well :-) And I meant to say Apache Commons Compress (https://commons.apache.org/proper/commons-compress/), not Codec. Doh!
    
    WRT PR, the process is the same as here. 
    
    You can create a JIRA here https://issues.apache.org/jira/browse/COMPRESS/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel
    
    And push PRs here https://github.com/apache/commons-compress
    
    The difference with HC is that Commons Compress has made the move to Git within the Apache infrastructure.
    
    Gary


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Re: [GitHub] httpclient issue #75: Add Brotli support

Posted by Gary Gregory <ga...@gmail.com>.
In general, we do not want to create additional dependencies. I think it
would be nice to have an optional dependency on Apache Commons Compress to
deflate/compress whatever Apache Commons Compress supports.

On Mon, May 1, 2017 at 1:37 PM, pmouawad <gi...@git.apache.org> wrote:

> Github user pmouawad commented on the issue:
>
>     https://github.com/apache/httpclient/pull/75
>
>     Hi Gary,
>     Oleg mentioned commons-codec.
>
>     By the way, why does HC not rely on deflate decompressor in
> commons-compress?
>
>     Thanks
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<https://www.amazon.com/gp/product/1617290459/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1617290459&linkCode=as2&tag=garygregory-20&linkId=cadb800f39946ec62ea2b1af9fe6a2b8>

<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1617290459>
JUnit in Action, Second Edition
<https://www.amazon.com/gp/product/1935182021/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182021&linkCode=as2&tag=garygregory-20&linkId=31ecd1f6b6d1eaf8886ac902a24de418%22>

<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182021>
Spring Batch in Action
<https://www.amazon.com/gp/product/1935182951/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182951&linkCode=%7B%7BlinkCode%7D%7D&tag=garygregory-20&linkId=%7B%7Blink_id%7D%7D%22%3ESpring+Batch+in+Action>
<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182951>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

[GitHub] httpclient issue #75: Add Brotli support

Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Hi Gary,
    Oleg mentioned commons-codec.
    
    By the way, why does HC not rely on deflate decompressor in commons-compress?
    
    Thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Tracking through a new ticket here: https://issues.apache.org/jira/browse/HTTPCLIENT-1843


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Hello @ok2c ,
    It appears to me gzip, deflate are standards and as such included in HC, as brotli will soon become the 3rd standard (I notice main browsers send it in Accept-Encoding) , if we follow current policy brotli should be included.
    
    I doubt commons-compress will contain it alone soon, it will be a bridge as Gary suggested, so dec will be a dependency anyway.
    
    I think the most built-in behaviour you provide the better it is for users , but as you like.
    
    Regards
    Philippe M.
    @philmdot


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Oops, wrong project, can you please patch against Apache Commons Compress? https://github.com/apache/commons-compress
    
    Thank you,
    Gary


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by ok2c <gi...@git.apache.org>.
Github user ok2c commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    @pmouawad I am not sure it is a good idea to introduce a hard dependency on an external library for a single decompression codec, especially given it is quite easy to plug it in with HttpClientBuilder. If the brotli codec were supported by Commons Codec it would be easier to add it to the default configuration. 
    
    ```
    LinkedHashMap<String, InputStreamFactory> contentDecoderMap = new LinkedHashMap<>();
    contentDecoderMap.put("gzip", new InputStreamFactory() {
    
        @Override
        public InputStream create(final InputStream instream) throws IOException {
            return new GZIPInputStream(instream);
        }
    
    });
    contentDecoderMap.put("br", new InputStreamFactory() {
    
        @Override
        public InputStream create(final InputStream instream) throws IOException {
            return new BrotliInputStream(instream);
        }
    
    });
    CloseableHttpClient httpclient = HttpClients.custom()
            .setContentDecoderRegistry(contentDecoderMap)
            .build();
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Hi @garydgregory ,
    Don't get me wrong, I never intended to say commons-codec was not alive :-)
    
    If you give me some tips on how I should proceed to integrate feature in commons-codec, I'll be happy to submit a PR.
    
    Thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient issue #75: Add Brotli support

Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad commented on the issue:

    https://github.com/apache/httpclient/pull/75
  
    Hi @garydgregory ,
    See https://github.com/apache/commons-codec/pull/6
    
    Regards


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[GitHub] httpclient pull request #75: Add Brotli support

Posted by pmouawad <gi...@git.apache.org>.
Github user pmouawad closed the pull request at:

    https://github.com/apache/httpclient/pull/75


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org