-
Notifications
You must be signed in to change notification settings - Fork 465
Fix: remove extraneous content encoding #650
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
Closed
Closed
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
ed237e3
Add failing test showing that encoding happens more than once
chingor13 331e250
Only encode the streaming content once.
chingor13 0604972
Remove assertion that the content is a HttpEncodingStreamingContent
chingor13 c4f29bc
Fix a bunch of typos (#640)
chingor13 1d4270d
Linting cleanup (#645)
chingor13 e64f72f
Remove deprecated google-http-client-jackson artifact. (#647)
chingor13 91cbfdb
Add Base64Test case for some base64 decoding edge cases (#644)
chingor13 1b18a6e
Add renovate.json (#651)
1a4a5cc
Update dependency com.fasterxml.jackson.core:jackson-core to v2.9.9 (…
renovate-bot b0d7245
Update dependency com.coveo:fmt-maven-plugin to v2.9 (#652)
renovate-bot 13731cd
Update dependency com.google.code.gson:gson to v2.8.5 (#657)
renovate-bot 78d4991
Add failing test showing that encoding happens more than once
chingor13 4fa6b09
Only encode the streaming content once.
chingor13 3d20d9a
Remove assertion that the content is a HttpEncodingStreamingContent
chingor13 a03bd43
Merge branch 'fix-double-encoding' of github.com:chingor13/google-htt…
chingor13 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Only encode the streaming content once.
In order to calculate the contentLength, we must write the encode the data first. The encoded data is written to a buffer using a ByteArrayOutputStream implementation and we use that to figure out how many bytes of data is to be sent. Previously, this data was thrown away and the content was re-encoded when it was actually time to send the data. Instead, we now replace the content with a ByteArrayContent which contains the buffer we wrote to when calculating the size. We implemented a new CachingByteArrayOutputStream so that we can access the byte buffer directly rather than copying into a new byte array (for memory performance)
- Loading branch information
commit 331e250f242f4ca8939adffd06f5045e64398c53
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
...le-http-client/src/main/java/com/google/api/client/util/CachingByteArrayOutputStream.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /* | ||
| * Copyright 2019 Google LLC | ||
| * | ||
| * Licensed 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. | ||
| */ | ||
|
|
||
| package com.google.api.client.util; | ||
|
|
||
| import java.io.ByteArrayOutputStream; | ||
|
|
||
| /** | ||
| * Output stream that extends the built-in {@link ByteArrayOutputStream} to return the internal | ||
| * byte buffer rather than creating a copy. | ||
| */ | ||
| public class CachingByteArrayOutputStream extends ByteArrayOutputStream { | ||
|
|
||
| /** | ||
| * Returns the content length of the buffer. | ||
| * | ||
| * @return tthe content length of the buffer. | ||
| */ | ||
| public int getContentLength() { | ||
| return count; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the buffer where the byte data is stored. | ||
| * | ||
| * @return the buffer where the byte data is stored. | ||
| */ | ||
| public byte[] getBuffer() { | ||
| return buf; | ||
| } | ||
|
|
||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOUtils checks the size via streaming, and doesn't actually store the data in memory. The same thing for sending the data. A large object will currently never fully have to be in memory.
This change may cause an unexpected memory spike for users with large objects