Check size in bytes as opposed to string size #589
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://guides.rubygems.org/make-your-own-gem/ The convention is to have one Ruby file with the same name as your gem, since that gets loaded when require is run.
nokogiri <1.10.4 and rubyzip < 1.3.0 had CVEs and/or regressions, and whilst we're here, signal that it's okay to use Ruby 2.3 syntax and methods and later.
Add caxlsx.rb to make auto require work
This PR aims to fix several issues with Relationship cache: 1) It's not threadsafe, so I propose to use a TLS variable for this. 2) Memory obtained by cache remains non-freed before the next run of `serialize`. I think it should be freed immediately. 3) Memory should be freed in `ensure` block to prevent memory bloating in case of exception. *There are only two hard things in Computer Science: cache invalidation and naming things.*
Also cacle only ids, not entire instances.
Bump dependencies for nokogiri, ruby, and rubyzip.
…p_cache Fix Relationship.instances cache
Fixnum is removed by 12d6433, but this typo was remained.
This code was added in 3def8f8 (back in 2011). It has never been worked on further since; `Axlsx::Parser` is not used anywhere in the whole codebase. Currently there are no plans to support parsing xlsx file anytime soon, so let‘s remove all of this dead code.
|
@aardvarkk Would you like to recreate this PR in https://github.com/caxlsx/caxlsx/ ? I think we should get this merged. |
- `size` returns length in characters, but doesn't factor in multibyte Unicode characters. By switching to `bytesize`, we check the relevant measure of how many bytes the worksheet name is. - Fixes randym#588 - Copy of PR against original axlsx (randym#589)
|
@noniq Done! Please see caxlsx/caxlsx#35 |
noniq
pushed a commit
to caxlsx/caxlsx
that referenced
this pull request
Dec 17, 2019
- `size` returns length in characters, but doesn't factor in multibyte Unicode characters. By switching to `bytesize`, we check the relevant measure of how many bytes the worksheet name is. - Fixes randym/axlsx#588 - Copy of PR against original axlsx (randym/axlsx#589)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
sizereturns length in characters, but doesn't factor in multibyte Unicode characters.By switching to
bytesize, we check the relevant measure of how many bytes the worksheet name is.Fixes #588.