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

Provide an memory efficient alternative to ClientHttpRequestInterceptor #22002

Closed
spring-issuemaster opened this issue Nov 5, 2018 · 4 comments
Closed
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Nov 5, 2018

Phil Webb opened SPR-17470 and commented

See this issue for some background.

Currently the BasicAuthenticationInterceptor class is a ClientHttpRequestInterceptor which causes the entire body to be read into a byte array and can cause OutOfMemoryError errors. I've managed to work around the issue by using a AbstractClientHttpRequestFactoryWrapper, but it would be really nice if there was a better way to do this. Perhaps we can create an alternative ClientHttpRequestInterceptor that can be used when only headers need to be changed.


Reference URL: spring-io/artifactory-resource#27

@poutsma

This comment has been minimized.

Copy link
Contributor

@poutsma poutsma commented Jan 23, 2019

This would require a major overhaul of the interception mechanism, while we are winding down our development effort of RestTemplate in favour of WebClient.

I suggest closing as declined. What do you think, @rstoyanchev ?

@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Jan 23, 2019

Yes I agree, and it helps that there is a workaround.

@philwebb philwebb changed the title BasicAuthenticationInterceptor can cause OutOfMemoryError [SPR-17470] Provide an memory efficient alternative to ClientHttpRequestInterceptor Sep 10, 2019
@philwebb philwebb reopened this Sep 10, 2019
philwebb added a commit to philwebb/spring-framework that referenced this issue Sep 10, 2019
Add `ClientHttpRequestInitializer` interface that can be used with any
`HttpAccessor` to initialize each `ClientHttpRequest` before it's used.

This provides a useful alternative to `ClientHttpRequestInterceptor`
when users need to configure things like `HttpHeaders`.

Closes spring-projectsgh-22002
@philwebb

This comment has been minimized.

Copy link
Member

@philwebb philwebb commented Sep 10, 2019

Unfortunately the workaround that I suggested cannot work with MockRestServiceServer which is causing us issues in Spring Boot (see spring-projects/spring-boot#17885).

I have a relatively small enhancement to HttpAccessor that add an initialization callback. I've pushed it to https://github.com/philwebb/spring-framework/tree/gh-22002 for review.

I know it's quite late in the day to be asking for this, but could it be considered for 5.2? I can't really find any good solution using the existing interceptor mechanism that works in all situations.

@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Sep 10, 2019

Looks good to me. Straight forward enough for 5.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.