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

fix CharReaderFromSyncStream read characters less than chunkSize (#2108) #2109

Merged
merged 6 commits into from
Jan 1, 2024

Conversation

itboy87
Copy link
Contributor

@itboy87 itboy87 commented Jan 1, 2024

There was an issue with CharReaderFromSyncStream. When attempting to read characters fewer than the specified chunkSize, it returns the maximum chunkSize data instead. I have set a minimum chunkSize limit to 8 because the decoder requires a minimum size of the maximum character size to decode. I believe the maximum character size in bytes is 4 bytes, but for safety, I used 8 bytes. Therefore, if anyone sets the chunkSize to less than 8, it will throw an exception.

  • Created two new constants:
companion object {
       private const val DEFAULT_CHUNK_SIZE = 1024
       private const val MIN_CHUNK_SIZE = 8
}
  • Check chunkSize and throw IllegalArgumentException exception if its less than MIN_CHUNK_SIZE
init {
  if (chunkSize < MIN_CHUNK_SIZE) throw IllegalArgumentException("chunkSize must be greater than $MIN_CHUNK_SIZE")
}
  • Refactored buffer fill code to bufferUp() function. This function returns the number of bytes added to the buffer. If tempStringBuilder doesn't have enough characters to read, the function will attempt to read more into the buffer until the condition is met or the stream is empty.

This PR closes #2108

Copy link
Member

@soywiz soywiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URLText extra test should be done in a separate PR since it looks unrelated to this

@itboy87
Copy link
Contributor Author

itboy87 commented Jan 1, 2024

@soywiz ok i will update it.

@itboy87 itboy87 requested a review from soywiz January 1, 2024 21:04
Copy link
Member

@soywiz soywiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@soywiz soywiz merged commit 0effa24 into korlibs:main Jan 1, 2024
9 checks passed
@soywiz
Copy link
Member

soywiz commented Jan 1, 2024

Thanks!

@itboy87 itboy87 deleted the fix/2108/CharReaderFromSyncStream branch January 2, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected results with CharReaderFromSyncStream when chunk size is less than reading count
2 participants