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

RestTemplateBuilder.basicAuth causes the entire body to be read into memory #17010

Closed
wants to merge 1 commit into from

Conversation

nosan
Copy link
Contributor

@nosan nosan commented May 29, 2019

Update RestTemplateBuilder and TestRestTemplate to use a custom request factory to add
authentication headers.

Prior to this commit, the RestTemplateBuilder and TestRestTemplate used the
BasicAuthenticationInterceptor to add headers.

see gh-15078

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 29, 2019
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels May 29, 2019
@philwebb philwebb added this to the 2.2.x milestone May 29, 2019
@philwebb
Copy link
Member

Nice one! Thanks.

@nosan nosan force-pushed the gh-15078 branch 5 times, most recently from e29a463 to 67ea430 Compare May 30, 2019 11:03
…equest factory to add

authentication headers.

Prior to this commit, the `RestTemplateBuilder` and `TestRestTemplate` used the
`BasicAuthenticationInterceptor` interceptor to add headers.

Unfortunately, adding any interceptor causes the entire message body
to be read into a byte array. This causes an `OutOfMemoryError` whenever
a large file is uploaded.

Closes spring-projectsgh-15078
philwebb pushed a commit that referenced this pull request Jun 1, 2019
Update `RestTemplateBuilder` to use a custom request factory to add
authentication headers rather than an interceptor.

Prior to this commit, the use of the `BasicAuthenticationInterceptor`
interceptor could cause `OutOfMemoryError` whenever a large file is
uploaded.

See gh-17010
philwebb added a commit that referenced this pull request Jun 1, 2019
Reduce the surface area of the public API by making the
`BasicAuthentication` and `BasicAuthenticationClientHttpRequestFactory`
class package private.

This commit also attempts to simplify `TestRestTemplate` by keeping
the `RestTemplateBuilder` and reusing it, rather than needing to deal
only with a `RestTemplate` instance.

See gh-17010
@philwebb philwebb closed this in f56386e Jun 1, 2019
@philwebb
Copy link
Member

philwebb commented Jun 1, 2019

Thanks for another contribution @nosan !! This has now been merged to master. I've add a polish commit that makes some of the new classes package private just to reduce the surface area of the public API.

@philwebb philwebb modified the milestones: 2.2.x, 2.2.0.M4 Jun 1, 2019
@nosan nosan deleted the gh-15078 branch June 3, 2019 12:53
wilkinsona pushed a commit to wilkinsona/spring-boot that referenced this pull request Jun 4, 2019
Update `RestTemplateBuilder` to use a custom request factory to add
authentication headers rather than an interceptor.

Prior to this commit, the use of the `BasicAuthenticationInterceptor`
interceptor could cause `OutOfMemoryError` whenever a large file is
uploaded.

See spring-projectsgh-17010
wilkinsona pushed a commit to wilkinsona/spring-boot that referenced this pull request Jun 4, 2019
Reduce the surface area of the public API by making the
`BasicAuthentication` and `BasicAuthenticationClientHttpRequestFactory`
class package private.

This commit also attempts to simplify `TestRestTemplate` by keeping
the `RestTemplateBuilder` and reusing it, rather than needing to deal
only with a `RestTemplate` instance.

See spring-projectsgh-17010
@wilkinsona wilkinsona changed the title Update RestTemplateBuilder and TestRestTemplate to use BasicAuthenticationClientHttpRequestFactory instead of Interceptor RestTemplateBuilder.basicAuth causes the entire body to be read into memory Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants