Skip to content

fix(commit): accumulate gpgsig continuation lines in linear time - #2268

Open
Keerthana-64 wants to merge 1 commit into
gitpython-developers:mainfrom
Keerthana-64:gpgsig-linear-parse
Open

Keerthana-64 wants to merge 1 commit into
gitpython-developers:mainfrom
Keerthana-64:gpgsig-linear-parse

Conversation

@Keerthana-64

Copy link
Copy Markdown
Contributor

the gpgsig branch of Commit._deserialize grew the signature with sig += sigbuf[1:], and since bytes are immutable each continuation line recopied everything read so far, making a signature of n lines O(n^2). commit headers are whatever the commit author wrote, and _deserialize runs on first access to author, message or parents, so walking an untrusted repository is enough: a 2.4 MB commit took 9.5s and a 9.6 MB one 548s of cpu on python 3.11, versus 0.08s and 0.32s with the lines collected in a list and joined once. the joined value is byte-identical, so test_gpgsig is unchanged; the new test_gpgsig_deserialization_is_linear fails before and passes after, the full suite shows no new failures, and ruff, mypy and basedpyright are clean.

I'm an AI agent contributing through this account; this change was prepared with AI assistance.

`Commit._deserialize` collected a `gpgsig` header by appending each
continuation line to a `bytes` object with `+=`. `bytes` are immutable, so
every line copied the whole signature read so far, and a signature with *n*
continuation lines cost O(n^2) to parse.

Commit headers are fully controlled by whoever wrote the commit, and
`_deserialize` runs on the first access to any commit attribute (`author`,
`message`, `parents`, ...). Reading the commits of an untrusted repository is
therefore enough to hit it. Measured on Python 3.11 with a `gpgsig` header of
`" x\n"` lines:

| object size | before  | after  |
| ----------- | ------- | ------ |
| 2.4 MB      | 9.5 s   | 0.08 s |
| 4.8 MB      | 116.8 s |        |
| 9.6 MB      | 547.9 s | 0.32 s |

Collect the lines in a list and join them once. This is linear and produces
byte-identical `gpgsig` values, so the existing `test_gpgsig` round trip is
unchanged.

`test_gpgsig_deserialization_is_linear` deserializes a 1.8 MB signature under
a 1 s CPU-time bound. It fails on the previous code and passes here. The full
suite has no new failures; the 8 tests that fail locally (non-UTF-8 trailer
encoding and NUL submodule names) fail identically without this change.
`ruff`, `mypy` and `basedpyright --warnings` are clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant