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

WebFlux JSON request body garbled under heavy load [SPR-17193] #21728

Closed
spring-issuemaster opened this issue Aug 17, 2018 · 6 comments
Closed

WebFlux JSON request body garbled under heavy load [SPR-17193] #21728

spring-issuemaster opened this issue Aug 17, 2018 · 6 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Aug 17, 2018

Martin Kutter opened SPR-17193 and commented

Running a load test against a webflux application, I experienced garbled requests under (moderately) heavy load: once in a while (i.e. 1 in 10 to 50.000 requests), the deserialization of the request body into the @RequestBody field yields wrong results.

In these bad results, field values are set to another field's value - but truncated to the lenght of the original field.

 

Example:

Original Request:

{"subject"`:

{"username":"USERNAME"}

,"action":"HTTP:ACTION","resource":"https://resource.url"}
 

Garbeled Request (example):

{"subject":

{"username":"USERNAME"}

,"action":"https://res","resource":"https://resource.url"}
 

 

The error only appears with authentication (basic auth) enabled.

 

I've created a test project to reproduce and better describe the error:

https://github.com/mkutter/netty-webflux-load


Affects: 5.0.8

Reference URL: https://github.com/mkutter/netty-webflux-load

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 4, 2018

Rossen Stoyanchev commented

I can confirm the sample reproduces the issue on my side.

@spring-issuemaster
Copy link
Collaborator Author

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

Rossen Stoyanchev commented

I ran the JMeter script many times, and from what I've seen so far the data always arrives as a single chunk, and is fed as a one byte array into Jackson's NonBlockingJsonParser, and is then accumulated into a single TokenBuffer whose content appears garbled when the issue occurs. So a pretty straight-forward scenario.

When Spring Security is used, it performs authentication on a dedicated thread to avoid blocking the server thread, so in terms of threads the NonBlockingJsonParser is created initially on one thread, while the data arrives on a separate (Netty) thread. Given that we create a new parser for each request, I suspect something is shared inside the parser. That's how I came across the USE_THREAD_LOCAL_FOR_BUFFER_RECYCLING feature which is used when creating the parser context. I tried and if disabled it makes the tests pass consistently. I ran 5-6 times to completion vs failing every time.

I will need to find out if this feature should be disabled for non-blocking parsing but for now if you could confirm that disabling it on the ObjectMapper's JsonFactory works for you too. There is a way to customize the ObjectMapper via WebFluxConfigurer under defaultCodecs.

 

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 6, 2018

Rossen Stoyanchev commented

Okay jackson-core#476 was just opened, so a fix is likely to come from Jackson. In the mean time I'm going to temporarily disable the feature by default in WebFlux.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 6, 2018

Rossen Stoyanchev commented

Link to related issue spring-boot#14315 also with an example project.

@NavidMitchell
Copy link

@NavidMitchell NavidMitchell commented Apr 8, 2019

I just noticed FasterXML/jackson-core#476 is fixed. This may be able to be enabled again in the current WebFlux.

@rstoyanchev
Copy link
Contributor

@rstoyanchev rstoyanchev commented Apr 12, 2019

It was fixed in 2.9.7. We still support 2.9+ which is why it hasn't changed. I guess it's been a little while now and we can consider removing the workaround for 5.2. Can you create a new ticket please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.