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

Handle inflate on buffer larger than chunk size #73

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

jbghoul
Copy link
Contributor

@jbghoul jbghoul commented Aug 31, 2023

Issue

  • In Zlib function zlibBufferSync :
    ** it tries to handle a given buffer
    ** the default chunkSize is 16384
    ** it calls zlib method _processChunk

  • in Zlib method _processChunk
    ** there is a do-while loop calling binding.writeSync method to process the given buffer (parameter named chunk)
    ** the chunkSize options is used as available out memory
    ** the do-while loop calls a function callback that is supposed to handle the case where the available out memory is not enough
    ** the do-while loop breaks if there is an error in the writeSync process

  • in Zlib method writeSync
    ** in calls inflate method and check returned status
    ** if the status is not OK, it marks an error, making the do-while loop breaks

  • in inflate method:
    ** in case MATCH: if there is no memory left, it breaks the for-loop
    ** ret is Z_OK
    ** condition flush === Z_FINISH and ret === Z_OK is turning ret to Z_BUF_ERROR

As a consequence :

  • status to Z_BUF_ERROR marks an error
  • the do-while loop breaks
  • the do-while loop does not handle the case where given buffer size is larger than chunk size option

Proposal

From: https://github.com/browserify/browserify-zlib/blob/master/src/binding.js#L241

  • Check status after inflate
  • if it is an Z_BUF_ERROR and no memory available, do not consider it has an error
  • if so, the do-while loop in _processChunk can continue to handle large buffer

@FredKSchott FredKSchott merged commit e4a0cfa into FredKSchott:main Nov 20, 2023
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.

2 participants