StringDecoder shouldn't chop off strings randomly [SPR-16337]

[spring-projects-issues]

Abhijit Sarkar opened SPR-16337 and commented

Given a stream "abc\n", "abc", "def\n", "abc", "def\nxyz\n", "abc", "def", "xyz\n" StringDecoder simply chops them off at newlines. This is hardly desired, since most applications expect to deal with complete records that end on line boundaries. In other words, the expected output for the above would be "abc", "abcdef", "abcdef", "xyz", "abcdefxyz".

I've an alternate implementation that produces the expected output but before making a PR, I would like to check that the existing behavior is not based on some use case I'm not thinking about.
https://github.com/asarkar/spring/blob/master/buffer-learning/src/main/java/org/abhijitsarkar/java/StringDecoder.java

---

Affects: 5.0.2

Referenced from: commits spring-projects/spring-integration@11240be

0 votes, 7 watchers

[spring-projects-issues]

Sébastien Deleuze commented

Hey Abhijit Sarkar, the link to the alternate implementation seems to be broken, could you please fix it in order to allow us to have a look?

[spring-projects-issues]

Abhijit Sarkar commented

Fixed broken link

[spring-projects-issues]

Sébastien Deleuze commented

Thanks for the updated link.

Indeed with current implementation (which for the record comes from this commit), the decoded Flux<String> with splitOnNewline = true can contain a stream that mix chunks coming from new line characters and chunks coming from the source Publisher<DataBuffer>.

While I understand that the proposed new behavior would be more consistent and meaningful, I am wondering if it could not end up in an increase of latency and memory consumption for String stream without or with few new lines, or even in the impossibility to decode Flux<String> without new lines since splitOnNewline = true by default on server-side.

I would be interested to have your take on this one Arjen Poutsma.

[spring-projects-issues]

Abhijit Sarkar commented

few new lines, or even in the impossibility to decode Flux<String> without new lines

That's exactly why I've also implemented a time-based release. User can set a max duration after which the record will be released even if no newline is found. If needed, we can also throw in a max length based release, but it might be overkill.

On a separate note, have you considered using GitHub issues? Many Spring projects already have migrated.

[spring-projects-issues]

Sébastien Deleuze commented

After discussing with Arjen Poutsma, I think I am more and more for keeping existing behavior. Current split behavior is mainly done to deal with multi-byte characters, and I don't think I would like to introduce complex split/join logic with time or max length based release, especially by default. We will discuss that during our Reactive meeting and I will give you our feedback.

For GitHub, it does not feed with our need especially for release management so I guess we will stay on JIRA for now ;-)

[spring-projects-issues]

Abhijit Sarkar commented

While I can't convince you to change the implementation, I will say though that what you perceive as useful for the users may not be the same thing as what the users actually find useful for them. I don't know who thought chopping off multi-byte strings at newlines is a common use case, or at least should be a default use case for StringDecoder, vis-a-vis emitting complete records terminated at newlines like all the readers in the JDK have been doing for ages.

[spring-projects-issues]

Arjen Poutsma commented

This is now fixed, see 609f173

StringDecoder now aligns a buffer frame before decoding, where each frame ends with a delimiter (\n and \r\n by default). Multi-byte characters that cross buffer boundaries should be handled properly now.