Skip to content

Conversation

@jrturton
Copy link
Contributor

Bug/issue #, if applicable: #238

Summary

If a table included multiple pipe characters that took the total number of columns (including spanning columns) above the main column count, this would cause a crash instead of just dropping the excess content as per the spec.

This PR removes the attempted out-of-bounds read of the widths array by capping the count.

Dependencies

None

Testing

Two unit tests have been added to exercise the code. To manually test it, you can attempt to parse markdown like this:

|A|B|C|
|-|-|-|
|1|2|3||4|5|6|

And then format the resulting document, which will crash without these changes in place.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary (not applicable)

@jrturton jrturton requested a review from d-ronnqvist November 13, 2025 15:56
@d-ronnqvist d-ronnqvist linked an issue Nov 13, 2025 that may be closed by this pull request
Comment on lines 1034 to 1035
let lastColumn = min(finalColumnWidths.count, column + colspan)
return (column..<lastColumn).map({ finalColumnWidths[$0] }).reduce(0, { $0 + $1 })
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: I think this can also be written as

Suggested change
let lastColumn = min(finalColumnWidths.count, column + colspan)
return (column..<lastColumn).map({ finalColumnWidths[$0] }).reduce(0, { $0 + $1 })
finalColumnWidths.dropFirst(column).prefix(colspan).reduce(0, +)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice, I prefer that a lot!

Copy link
Contributor

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for posting the fix!

@QuietMisdreavus
Copy link
Contributor

@swift-ci Please test

@jrturton jrturton merged commit b2135f4 into swiftlang:main Nov 17, 2025
2 checks passed
@jrturton jrturton deleted the jrturton-fix-238-table-out-of-bounds branch November 17, 2025 16:06
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.

Visit Table Index Out Of Bounds Crash

3 participants