Skip to content

Reject out-of-range column index in Dataset.add_formatter - #679

Open
uttam12331 wants to merge 1 commit into
jazzband:masterfrom
uttam12331:fix-add-formatter-offbyone
Open

Reject out-of-range column index in Dataset.add_formatter#679
uttam12331 wants to merge 1 commit into
jazzband:masterfrom
uttam12331:fix-add-formatter-offbyone

Conversation

@uttam12331

Copy link
Copy Markdown

Summary

Dataset.add_formatter accepts a column index one past the last column, then crashes later on export instead of rejecting it up front.

Columns are 0-indexed everywhere (get_col(index)row[index], formatter application → row[col] = handler(row[col])), so the maximum valid index is width - 1. But the bounds check uses <=:

if col is None or col <= self.width:      # accepts col == width (out of range)
    self._formatters.append((col, handler))
else:
    raise InvalidDatasetIndex

The else: raise InvalidDatasetIndex branch shows the intent is to reject out-of-range indices — the boundary is just off by one.

Reproduction

import tablib
d = tablib.Dataset()
d.headers = ['a', 'b']     # width 2 -> valid indices 0, 1
d.append([1, 2])
d.add_formatter(2, str)    # index 2 is out of range -> silently returns True
d.export('csv')            # IndexError: list index out of range (deep in format_row)

Expected: add_formatter(2, ...) should raise InvalidDatasetIndex at the call site.

Fix

-        if col is None or col <= self.width:
+        if col is None or col < self.width:

Tests

Added test_add_formatter_out_of_range_column: on a width-3 dataset it confirms the last valid index (2) is accepted and index 3 raises InvalidDatasetIndex. It fails on the current code (silently accepted) and passes with the fix; the existing formatter tests still pass.

Columns are 0-indexed, so the last valid index is `width - 1`. The bounds
check used `col <= self.width`, which accepts `col == width` (one past the
last column). That out-of-range index was silently stored and then raised
`IndexError: list index out of range` later, deep in export/format_row,
instead of the intended `InvalidDatasetIndex` at the `add_formatter` call
site.

Use `col < self.width`, and add a regression test for the boundary.
@MohammedAlkindi

Copy link
Copy Markdown

#655 is the same fix, opened six weeks earlier (2026-07-10). Identical hunk in src/tablib/core.py: col <= self.width becomes col < self.width, +1/-1 on both. Whichever merges first will conflict the other.

The difference worth keeping is in this PR's test. It asserts width == 3, accepts add_formatter(2, ...) and rejects add_formatter(3, ...), so it pins both sides of the off-by-one. #655's test asserts only the rejection, and it also adds an AUTHORS entry.

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.

2 participants