Skip to content

Deferred minor findings from the archive and OpenDocument reviews #33

Description

@vaceslav

Minor findings of the final reviews of #19 (zip archives) and #13 (OpenDocument). They were deferred on purpose: none of them gives wrong data on files a real writer produces. They are listed here so they are not lost. Each item says what a user would see.

Archive (ArchiveCursor, ChunkedBuffer)

  • A small workbook inside an archive costs a 1 MB allocation, twice (once when the sheets are listed, once when they are read). When the first piece fills exactly to the declared size, the loop allocates a whole piece just to find the end of the file. The remark saying "a small workbook does not cost a megabyte" is therefore wrong. Fix: probe with a one-byte read before allocating the next piece.
  • ReadRow after a failed reopen throws NullReferenceException, not a TabularException. Only running out of memory reaches this. Separately, CurrentSheetIndex is left stale when the inner MoveToSheet throws.
  • The archive-wide sheet ceiling reuses XlsxCursorOptions.MaxSheets for csv and ods sources as well, and reports the limit as the bare string "MaxSheets". Fix: document which option applies, or use nameof.
  • Two entries with the same path produce two sheets nothing tells apart: same name, same Source. Fix: skip the later duplicate, or document it.
  • TabularFile.ClassifyZip reads a zip's central directory once more before any entry-count bound applies. This is negligible for real files, but doubles the cost for a hostile directory. Fix: read the entry count from the end-of-central-directory record first.
  • Polish: ArchiveCursor.Dispose has no <inheritdoc/>; ChunkedBuffer throws ArgumentOutOfRangeException rather than IOException on a negative seek, and returns 0 when read after dispose.

OpenDocument (OdsCursor, TableNameScan)

  • A paragraph nested inside a paragraph drops the outer one's tail. <text:p>ab<text:p>in</text:p>ef</text:p> reads ab\nin. The markup is malformed ODF.
  • UTF-16 XML is not refused as XML. A .fods or Excel 2003 XML file in UTF-16 with a BOM gets past IsXmlDocument and is read as csv. Fix: also check FF FE/FE FF followed by <?xml.
  • NaN, Infinity and 1e400 in office:value become Number cells. The xlsx reader does the same, so the two agree. A double.IsFinite guard in both would keep such values away from analysis.
  • Every time cell allocates a string (XmlConvert.ToTimeSpan takes a string), and a malformed time-value costs a thrown and caught exception, about 22 µs per cell. A small span-based ISO-duration parser would remove both.
  • In ODS mode, the error for repeated attributes still says "repeats its r, t or s attribute", which is the xlsx wording.
  • When the OdsCursor constructor fails after the content part is opened, its scanner is not disposed. The archive underneath is disposed, so only a pooled buffer leaks.
  • table:null-date in table:calculation-settings is ignored when time durations are read. Durations always count from the 1899 null date.
  • OdsCursor.ReadRow does not wrap InvalidDataException as format.corrupt, as XlsxCursor does. It is hardly reachable, because the name pass inflates the whole part first.
  • The table-tag ceiling in TableNameScan, and the scanner's 16 M-character node ceiling, ignore a configured MaxValueChars. Sheet names have no counterpart of xlsx's MaxMetadataChars.

Related, but an API change and so tracked in #15: ITabularCursor.MoveToSheet takes no cancellation token (see KNOWN-ISSUES).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions