Skip to content

convert_tables: don't crash on tables with no rows (#63 case 1) - #69

Open
eeshsaxena wants to merge 1 commit into
fhightower:mainfrom
eeshsaxena:fix/empty-table-indexerror
Open

convert_tables: don't crash on tables with no rows (#63 case 1)#69
eeshsaxena wants to merge 1 commit into
fhightower:mainfrom
eeshsaxena:fix/empty-table-indexerror

Conversation

@eeshsaxena

Copy link
Copy Markdown

Picks off the unambiguous part of #63: an empty table crashes.

import html_to_json
html_to_json.convert_tables('<table></table>')
html_to_json.convert_tables('<table><tbody></tbody></table>')
# IndexError: list index out of range

_process_table reads find_all('tr')[0] before checking that the table has any rows. An empty <tbody> is ordinary CMS output, so this isn't only an adversarial-input concern.

Added an early return of the empty result when there are no rows. convert_tables already drops empty results, so a rowless table is simply omitted (including when it sits next to a real table). Tables with rows are untouched.

I deliberately left the ragged/class-B cases from #63 out of this PR since, as you noted there, they need a semantics decision and case 2 overlaps with #64.

Added two tests in test_html_tables_misc.py (empty table alone, and empty table next to a real one). The empty-table case raises IndexError on main and passes with this change.

_process_table indexed find_all('tr')[0] before checking whether the table
had any rows, so an empty <table> or an empty <tbody> (ordinary output from
many CMSes) raised IndexError instead of being skipped.

Return early with no data when the table has no rows; convert_tables already
drops empty results, so such tables are simply omitted from the output.

This is case 1 from fhightower#63; the ragged/class-B cases there involve a semantics
decision and are left out of this change.
@eeshsaxena

Copy link
Copy Markdown
Author

Hi! Gentle nudge on this one whenever you have some bandwidth. It's a small, self-contained fix (convert_tables: don't crash on tables with no rows (#63 case 1)), and it's currently mergeable with no conflicts. No urgency at all, and I'm happy to make any changes you'd like. Thanks for maintaining html-to-json!

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