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

Invalid default buffer size #155

Closed
florianreinhart opened this issue Jul 16, 2021 · 1 comment · Fixed by #156
Closed

Invalid default buffer size #155

florianreinhart opened this issue Jul 16, 2021 · 1 comment · Fixed by #156
Assignees

Comments

@florianreinhart
Copy link
Contributor

Version

main branch

Description of the bug

The comments for both init methods on XLSXFile state that the default buffer size is 10KB, but it is actually 10MB.

  /// - Parameters:
  ///   - filepath: path to the `.xlsx` file to be processed.
  ///   - bufferSize: ZIP archive buffer size in bytes. The default is 10KB.
  /// You may need to set a bigger buffer size for bigger files.
  ///   - errorContextLength: The error context length. The default is `0`.
  /// Non-zero length makes an error thrown from
  /// the XML parser with line/column location repackaged with a context
  /// around that location of specified length. For example, if an error was
  /// thrown indicating that there's an unexpected character at line 3, column
  /// 15 with `errorContextLength` set to 10, a new error type is rethrown
  /// containing 5 characters before column 15 and 5 characters after, all on
  /// line 3. Line wrapping should be handled correctly too as the context can
  /// span more than a few lines.
  public init?(
    filepath: String,
    bufferSize: UInt32 = 10 * 1024 * 1024,
    errorContextLength: UInt = 0
  ) {

Let me know if the comment or code is wrong and I'll create a PR.

@MaxDesiatov
Copy link
Collaborator

Thanks, the comment is clearly wrong. I think it initially was 10KB, which led to corrupted data. Then I had to bump it to 10MB, but apparently forgot to update the comment.

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 a pull request may close this issue.

2 participants