Skip to content

Conversation

@ok2c
Copy link
Member

@ok2c ok2c commented Aug 10, 2025

Relatively minor code optimization that improves consistency of automatic content decompression by the classic and async transports. @arturobernalg Please double-check.

@ok2c ok2c requested a review from arturobernalg August 10, 2025 13:50
Copy link
Member

@arturobernalg arturobernalg left a comment

Choose a reason for hiding this comment

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

@ok2c per RFC 9110 (§8.4) Content-Encoding lists codings in apply-order, so we should decode right→left (hence the reverse loop) — can you confirm that’s still your expectation here?”

@ok2c ok2c force-pushed the content_coding_support branch from 7dbbd2d to 71fc36c Compare August 11, 2025 14:39
@ok2c
Copy link
Member Author

ok2c commented Aug 11, 2025

@ok2c per RFC 9110 (§8.4) Content-Encoding lists codings in apply-order, so we should decode right→left (hence the reverse loop) — can you confirm that’s still your expectation here?”

@arturobernalg I made a mistake. Good catch. Please take another look.

Copy link
Member

@arturobernalg arturobernalg left a comment

Choose a reason for hiding this comment

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

LGTM

@rpkrajewski
Copy link

Funny that this popped up because I am new to using the HTTP5 library, and the HttpEntity.getContent() Javadoc does not say if you get back the encoded stream, or (better) a decoded stream. Does this PR implement a best effort for making HttpEntity.getContext() do the latter? If so, the Javadoc for that function should be clarified.

@ok2c ok2c merged commit 3947600 into apache:master Aug 14, 2025
10 checks passed
@ok2c ok2c deleted the content_coding_support branch August 14, 2025 10:12
@ok2c
Copy link
Member Author

ok2c commented Aug 14, 2025

Funny that this popped up because I am new to using the HTTP5 library, and the HttpEntity.getContent() Javadoc does not say if you get back the encoded stream, or (better) a decoded stream. Does this PR implement a best effort for making HttpEntity.getContext() do the latter? If so, the Javadoc for that function should be clarified.

@rpkrajewski The classic HttpClient has had support for the transparent content decompression for quite some time. Recently this support has also been added to the async HttpClient.

As to whether an entity content is encoded this is determined by the #getContentEncoding method. I am not sure what else there is to say but we happily take javadoc improvements.

@rpkrajewski
Copy link

Thanks - I will experiment more with this when 5.6 is out.

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.

3 participants