Skip to content

[high] fix(xlsx_enrich): use None for unlimited display.max_colwidth - #809

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/015-xlsx_enrich-max-colwidth-1
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/015-xlsx_enrich-max-colwidth-1

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — xlsx_enrich failed every query because modern pandas rejects a display.max_colwidth of -1.

  • Problem — xlsx_enrich.py called pandas.set_option("display.max_colwidth", -1), which modern pandas rejects with ValueError, and the call sat outside the handler's try block so nothing caught it. Every query against the module failed before read_excel() was reached, returning neither a result nor a MISP error.
  • Fix — Passes None, the current spelling of unlimited width, and moves the call inside the try block, with regression tests.
  • Effect — xlsx attachments are parsed again, and any future pandas breakage surfaces as a normal misperrors dict.

Problem

misp_modules/modules/expansion/xlsx_enrich.py, line 46 (before this change) called:

pandas.set_option("display.max_colwidth", -1)

Modern pandas rejects a negative value for that option and raises
ValueError: Value must be a nonnegative integer or None (reproduced with pandas 2.3.3).
Because the call sat outside the surrounding try block, the exception was not caught by
the module's error handling and propagated out of handler().

Concrete trigger: any query at all against this module — e.g. a attachment query with a
valid base64-encoded .xlsx in data. The first statement after decoding the attachment
raises, so the module never reaches pandas.read_excel() and returns no result and no
MISP error dict.

Fix

  • Pass None instead of -1. None is the current pandas spelling of "unlimited column
    width" and preserves the original intent of the code.
  • Move the set_option() call inside the existing try block, so that any future pandas
    change here degrades into the module's normal misperrors error dict rather than an
    unhandled exception. This matches how the rest of the pandas work in this handler
    (read_excel, to_string) is already guarded.

The change is two lines and does not alter the module's output format.

Testing

Gate command: poetry run pytest (the same invocation used by the repo's
.github/workflows/test-package.yml).

Result: 106 passed, 5 subtests passed in 89.81s (0:01:29).

Regression coverage added in tests/test_xlsx_enrich.py:

  • TestXlsxEnrich::test_handler_extracts_text_from_xlsx — builds an in-memory .xlsx
    attachment, runs handler(), and asserts the cell contents come back in the freetext
    result. This fails with the old -1 value.
  • TestXlsxEnrich::test_handler_sets_unlimited_column_width — asserts
    display.max_colwidth ends up as None, pinning the intended "unlimited" behaviour.

🤖 Generated with Claude Code

`pandas.set_option("display.max_colwidth", -1)` was executed before the
try block. Since pandas 1.0 a negative value is rejected with
`ValueError: Value must be a nonnegative integer or None`, so the handler
raised on every single invocation and the module was effectively dead
(reproduced with pandas 2.3.3).

Pass `None`, the modern spelling of "unlimited", and move the call inside
the guarded block so any future pandas change degrades to the module's
error dict instead of an unhandled exception.

Adds tests/test_xlsx_enrich.py covering the handler round-trip on a
generated .xlsx attachment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012VxctWivKrfQWn8tVD8qkT
@elhoim elhoim changed the title fix(xlsx_enrich): use None for unlimited display.max_colwidth [high] fix(xlsx_enrich): use None for unlimited display.max_colwidth Sep 3, 2026

This branch has not been deployed

No deployments
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.

1 participant