Skip to content

Improve HTML support - #153

Open
nikilase wants to merge 2 commits into
cdgriffith:masterfrom
nikilase:master
Open

Improve HTML support#153
nikilase wants to merge 2 commits into
cdgriffith:masterfrom
nikilase:master

Conversation

@nikilase

Copy link
Copy Markdown
  • Add <HTML to the magic_data headers for correct HTML file recognition

  • Limit CSV sniffer sample size to prevent slow detection. Truncate text to 8192 characters and use the first 50 lines when sniffing CSV dialects.

-Add HTML XLS detection and performance test

Fixes #152

- Add <HTML to the magic_data headers for correct HTML file recognition

- Limit CSV sniffer sample size to prevent slow detection. Truncate text to 8192 characters and use the first 50 lines when sniffing CSV dialects.

-Add HTML XLS detection and performance test
Copilot AI lite review requested due to automatic review settings August 18, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses #152 by improving HTML recognition (including Excel-generated HTML saved as .xls) and preventing pathological runtimes in CSV detection by limiting the amount of text passed to csv.Sniffer.

Changes:

  • Add an uppercase <HTML magic header entry so HTML files starting with <HTML are detected as text/html.
  • Limit the CSV sniffer input to the first 50 lines / 8192 characters to avoid extremely slow csv.Sniffer().sniff(...) calls.
  • Add a regression test for HTML .xls detection (and a performance-oriented assertion, currently time-based).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/test_common_extensions.py Adds a test for HTML-in-.xls detection and a wall-clock performance assertion.
puremagic/scanners/text_scanner.py Caps the CSV sniffer sample size to prevent long runtimes.
puremagic/magic_data.json Adds <HTML header signature to improve HTML detection for uppercase tags.
Suppressed comments (1)

test/test_common_extensions.py:300

  • This test currently (1) has a mismatched docstring/name (test_html_xls_msg / Outlook .msg), and (2) asserts a wall-clock threshold (elapsed < 1) which can be flaky across CI environments. Prefer a functional MIME assertion, and add a deterministic regression test that csv.Sniffer is only given a bounded sample (<=8192 chars / <=50 lines) to prevent pathological runtimes.
def test_html_xls_msg():
    """CFBF scanner correctly identifies Outlook .msg"""
    start = time.perf_counter()
    mime = puremagic.from_file(os.path.join(OFFICE_DIR, "test_html.xls"), mime=True)
    elapsed = time.perf_counter() - start

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/test_common_extensions.py
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.

Kind of long detection for some HTML files | CSV Sniffer sniffs too much data

2 participants