Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HttpContent is encoded two times when posting data #648

Closed
chingor13 opened this issue May 24, 2019 · 4 comments · Fixed by #910
Closed

HttpContent is encoded two times when posting data #648

chingor13 opened this issue May 24, 2019 · 4 comments · Fixed by #910
Assignees
@codyoss
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@chingor13
Copy link
Collaborator

The resulting request is correctly encoded once, but we are encoding an extra time when we calculate the content length.
This was referenced May 24, 2019
Closed
Closed
@chingor13 chingor13 added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels May 24, 2019
@chingor13 chingor13 self-assigned this May 24, 2019
@sduskis
Copy link
Contributor

sduskis commented May 29, 2019

This is a duplicate of #283.

@chingor13
Copy link
Collaborator Author

It turns out this is non-trivial to fix. The issue is that for some transport adapters, we have to set the Content-Length header before we start streaming the content. This means that we must count the bytes before we send them. We don't want to cache the encoded content in memory as it defeats the purpose of streaming the content and could cause memory spikes in the application. We don't want to write to a temporary file as there may be file system issues/restrictions (like on GAE) that we cannot necessarily plan for.

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Jun 8, 2019
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Jun 18, 2019
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Jun 28, 2019
@codyoss
Copy link
Member

codyoss commented Oct 15, 2019

@chingor13 do we have more details on this one? Such as what is being affected? Or is there a finite list of Transport Adapters that need the Content-Length set?

@codyoss codyoss assigned codyoss and unassigned chingor13 Dec 9, 2019
@codyoss
Copy link
Member

codyoss commented Dec 9, 2019

I will take a stab at this.

codyoss added a commit to codyoss/google-http-java-client that referenced this issue Dec 10, 2019
@codyoss
feat: support chunked transfer encoding
74556be 
Currently any time HttpRequest works with encoded data it encodes
the data twice. Once for the actaul stream and once for checking
the length of the stream. Instead, when there is encoding just don't
set the content length. This will cause the underlying transports,
with a few tweaks, to use Transfer-Encoding: chunked.

Fixes googleapis#648
@codyoss codyoss mentioned this issue Dec 10, 2019
Merged
codyoss added a commit that referenced this issue Dec 17, 2019
@codyoss
feat: support chunked transfer encoding (#910)
b8d6abe 
Currently any time HttpRequest works with encoded data it encodes
the data twice. Once for the actual stream and once for checking
the length of the stream. Instead, when there is encoding just don't
set the content length. This will cause the underlying transports,
with a few tweaks, to use Transfer-Encoding: chunked.

Fixes #648
clundin25 pushed a commit to clundin25/google-http-java-client that referenced this issue Aug 11, 2022
@renovate-bot
chore(deps): update dependency org.jacoco:jacoco-maven-plugin to v0.8…
5ef4514 
….7 (googleapis#648)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@codyoss codyoss

Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
4 participants
@chingor13 @sduskis @codyoss @yoshi-automation