-
-
Notifications
You must be signed in to change notification settings - Fork 399
Fix Signature repr() #1155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Signature repr() #1155
Conversation
Good catch. I think it would be more correct to represent pygit2.Signature('Foo Ibáñez', '[email protected]', 1322174594, 60, None) Then for this to work if (!PyArg_ParseTupleAndKeywords(
args, kwds, "Os|Liz", keywords,
&py_name, &email, &time, &offset, &encoding)) |
Agreed that representation has better semantics. Updated to reflect! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, please rebase, and squash into a single commit.
23fd575
to
e5aa6d0
Compare
Fixed up! Let me know if there is anything else I can do. Thanks |
One more thing, class Signature:
_encoding: str | None
...
def __init__(self, name: str, email: str, time: int, offset: int, encoding: str | None) -> None: ... |
`to_unicode()` expects a non-NULL `x` argument (due to `strlen(x)`); however `self->encoding` _may_ be NULL, especially if built with `build_signature()` (i.e. if called from the `commit.author` property). As a result, any `repr()` on a `Signature` with a `NULL` `encoding` will cause a segfault. While `encoding` defaults to "utf-8" on `__init__()`, the API now allows users to explicitly initialize `encoding` to `None`. Within class methods, any use of a `None` `encoding` defaults to "utf-8". Note some APIs like `to_unicode()` accept `NULL` for `encoding`, so we do not perform explicit checks when used. Additionally add `encoding` comparison to `richcompare()` and fix a test that used default encoding.
e5aa6d0
to
3631a6d
Compare
Merged, thanks! |
FYI, after a second thought I've done follow up commit. The main change is that |
1.10.1 (2022-08-28) ------------------------- - Fix segfault in ``Signature`` repr `#1155 <https://github.com/libgit2/pygit2/pull/1155>`_ - Linux and macOS wheels for Python 3.11 `#1154 <https://github.com/libgit2/pygit2/pull/1154>`_ 1.10.0 (2022-07-24) ------------------------- - Upgrade to libgit2 1.5 - Add support for ``GIT_OPT_GET_OWNER_VALIDATION`` and ``GIT_OPT_SET_OWNER_VALIDATION`` `#1150 <https://github.com/libgit2/pygit2/pull/1150>`_ - New ``untracked_files`` and ``ignored`` optional arguments for ``Repository.status(...)`` `#1151 <https://github.com/libgit2/pygit2/pull/1151>`_ 1.9.2 (2022-05-24) ------------------------- - New ``Repository.create_commit_string(...)`` and ``Repository.create_commit_with_signature(...)`` `#1142 <https://github.com/libgit2/pygit2/pull/1142>`_ - Linux and macOS wheels updated to libgit2 v1.4.3 - Remove redundant line `#1139 <https://github.com/libgit2/pygit2/pull/1139>`_ 1.9.1 (2022-03-22) ------------------------- - Type hints: added to C code and Branches/References `#1121 <https://github.com/libgit2/pygit2/pull/1121>`_ `#1132 <https://github.com/libgit2/pygit2/pull/1132>`_ - New ``Signature`` supports ``str()`` and ``repr()`` `#1135 <https://github.com/libgit2/pygit2/pull/1135>`_ - Fix ODB backend's read in big endian architectures `#1130 <https://github.com/libgit2/pygit2/pull/1130>`_ - Fix install with poetry `#1129 <https://github.com/libgit2/pygit2/pull/1129>`_ `#1128 <https://github.com/libgit2/pygit2/issues/1128>`_ - Wheels: update to libgit2 v1.4.2 - Tests: fix testing ``parse_diff`` `#1131 <https://github.com/libgit2/pygit2/pull/1131>`_ - CI: various fixes after migration to libgit2 v1.4 1.9.0 (2022-02-22) ------------------------- - Upgrade to libgit2 v1.4 - Documentation, new recipes for committing and cloning `#1125 <https://github.com/libgit2/pygit2/pull/1125>`_ 1.8.0 (2022-02-04) ------------------------- - Rename ``RemoteCallbacks.progress(...)`` callback to ``.sideband_progress(...)`` `#1120 <https://github.com/libgit2/pygit2/pull/1120>`_ - New ``Repository.merge_base_many(...)`` and ``Repository.merge_base_octopus(...)`` `#1112 <https://github.com/libgit2/pygit2/pull/1112>`_ - New ``Repository.listall_stashes()`` `#1117 <https://github.com/libgit2/pygit2/pull/1117>`_ - Code cleanup `#1118 <https://github.com/libgit2/pygit2/pull/1118>`_ Backward incompatible changes: - The ``RemoteCallbacks.progress(...)`` callback has been renamed to ``RemoteCallbacks.sideband_progress(...)``. This matches the documentation, but may break existing code that still uses the old name. 1.7.2 (2021-12-06) ------------------------- - Universal wheels for macOS `#1109 <https://github.com/libgit2/pygit2/pull/1109>`_ 1.7.1 (2021-11-19) ------------------------- - New ``Repository.amend_commit(...)`` `#1098 <https://github.com/libgit2/pygit2/pull/1098>`_ - New ``Commit.message_trailers`` `#1101 <https://github.com/libgit2/pygit2/pull/1101>`_ - Windows wheels for Python 3.10 `#1103 <https://github.com/libgit2/pygit2/pull/1103>`_ - Changed: now ``DiffDelta.is_binary`` returns ``None`` if the file data has not yet been loaded, cf. `#962 <https://github.com/libgit2/pygit2/issues/962>`_ - Document ``Repository.get_attr(...)`` and update theme `#1017 <https://github.com/libgit2/pygit2/issues/1017>`_ `#1105 <https://github.com/libgit2/pygit2/pull/1105>`_ 1.7.0 (2021-10-08) ------------------------- - Upgrade to libgit2 1.3.0 `#1089 <https://github.com/libgit2/pygit2/pull/1089>`_ - Linux wheels now bundled with libssh2 1.10.0 (instead of 1.9.0) - macOS wheels now include libssh2 - Add support for Python 3.10 `#1092 <https://github.com/libgit2/pygit2/pull/1092>`_ `#1093 <https://github.com/libgit2/pygit2/pull/1093>`_ - Drop support for Python 3.6 - New `pygit2.GIT_CHECKOUT_SKIP_LOCKED_DIRECTORIES` `#1087 <https://github.com/libgit2/pygit2/pull/1087>`_ - New optional argument ``location`` in ``Repository.applies(..)`` and ``Repository.apply(..)`` `#1091 <https://github.com/libgit2/pygit2/pull/1091>`_ - Fix: Now the `flags` argument in `Repository.blame()` is passed through `#1083 <https://github.com/libgit2/pygit2/pull/1083>`_ - CI: Stop using Travis, move to GitHub actions Caveats: - Windows wheels for Python 3.10 not yet available.
1.10.1 (2022-08-28) ------------------------- - Fix segfault in ``Signature`` repr `#1155 <https://github.com/libgit2/pygit2/pull/1155>`_ - Linux and macOS wheels for Python 3.11 `#1154 <https://github.com/libgit2/pygit2/pull/1154>`_ 1.10.0 (2022-07-24) ------------------------- - Upgrade to libgit2 1.5 - Add support for ``GIT_OPT_GET_OWNER_VALIDATION`` and ``GIT_OPT_SET_OWNER_VALIDATION`` `#1150 <https://github.com/libgit2/pygit2/pull/1150>`_ - New ``untracked_files`` and ``ignored`` optional arguments for ``Repository.status(...)`` `#1151 <https://github.com/libgit2/pygit2/pull/1151>`_ 1.9.2 (2022-05-24) ------------------------- - New ``Repository.create_commit_string(...)`` and ``Repository.create_commit_with_signature(...)`` `#1142 <https://github.com/libgit2/pygit2/pull/1142>`_ - Linux and macOS wheels updated to libgit2 v1.4.3 - Remove redundant line `#1139 <https://github.com/libgit2/pygit2/pull/1139>`_ 1.9.1 (2022-03-22) ------------------------- - Type hints: added to C code and Branches/References `#1121 <https://github.com/libgit2/pygit2/pull/1121>`_ `#1132 <https://github.com/libgit2/pygit2/pull/1132>`_ - New ``Signature`` supports ``str()`` and ``repr()`` `#1135 <https://github.com/libgit2/pygit2/pull/1135>`_ - Fix ODB backend's read in big endian architectures `#1130 <https://github.com/libgit2/pygit2/pull/1130>`_ - Fix install with poetry `#1129 <https://github.com/libgit2/pygit2/pull/1129>`_ `#1128 <https://github.com/libgit2/pygit2/issues/1128>`_ - Wheels: update to libgit2 v1.4.2 - Tests: fix testing ``parse_diff`` `#1131 <https://github.com/libgit2/pygit2/pull/1131>`_ - CI: various fixes after migration to libgit2 v1.4 1.9.0 (2022-02-22) ------------------------- - Upgrade to libgit2 v1.4 - Documentation, new recipes for committing and cloning `#1125 <https://github.com/libgit2/pygit2/pull/1125>`_ 1.8.0 (2022-02-04) ------------------------- - Rename ``RemoteCallbacks.progress(...)`` callback to ``.sideband_progress(...)`` `#1120 <https://github.com/libgit2/pygit2/pull/1120>`_ - New ``Repository.merge_base_many(...)`` and ``Repository.merge_base_octopus(...)`` `#1112 <https://github.com/libgit2/pygit2/pull/1112>`_ - New ``Repository.listall_stashes()`` `#1117 <https://github.com/libgit2/pygit2/pull/1117>`_ - Code cleanup `#1118 <https://github.com/libgit2/pygit2/pull/1118>`_ Backward incompatible changes: - The ``RemoteCallbacks.progress(...)`` callback has been renamed to ``RemoteCallbacks.sideband_progress(...)``. This matches the documentation, but may break existing code that still uses the old name. 1.7.2 (2021-12-06) ------------------------- - Universal wheels for macOS `#1109 <https://github.com/libgit2/pygit2/pull/1109>`_ 1.7.1 (2021-11-19) ------------------------- - New ``Repository.amend_commit(...)`` `#1098 <https://github.com/libgit2/pygit2/pull/1098>`_ - New ``Commit.message_trailers`` `#1101 <https://github.com/libgit2/pygit2/pull/1101>`_ - Windows wheels for Python 3.10 `#1103 <https://github.com/libgit2/pygit2/pull/1103>`_ - Changed: now ``DiffDelta.is_binary`` returns ``None`` if the file data has not yet been loaded, cf. `#962 <https://github.com/libgit2/pygit2/issues/962>`_ - Document ``Repository.get_attr(...)`` and update theme `#1017 <https://github.com/libgit2/pygit2/issues/1017>`_ `#1105 <https://github.com/libgit2/pygit2/pull/1105>`_ 1.7.0 (2021-10-08) ------------------------- - Upgrade to libgit2 1.3.0 `#1089 <https://github.com/libgit2/pygit2/pull/1089>`_ - Linux wheels now bundled with libssh2 1.10.0 (instead of 1.9.0) - macOS wheels now include libssh2 - Add support for Python 3.10 `#1092 <https://github.com/libgit2/pygit2/pull/1092>`_ `#1093 <https://github.com/libgit2/pygit2/pull/1093>`_ - Drop support for Python 3.6 - New `pygit2.GIT_CHECKOUT_SKIP_LOCKED_DIRECTORIES` `#1087 <https://github.com/libgit2/pygit2/pull/1087>`_ - New optional argument ``location`` in ``Repository.applies(..)`` and ``Repository.apply(..)`` `#1091 <https://github.com/libgit2/pygit2/pull/1091>`_ - Fix: Now the `flags` argument in `Repository.blame()` is passed through `#1083 <https://github.com/libgit2/pygit2/pull/1083>`_ - CI: Stop using Travis, move to GitHub actions Caveats: - Windows wheels for Python 3.10 not yet available.
`to_unicode(..., ..., NULL)` is equivalent to `to_unicode(..., ..., "strict")`. It returns NULL if the string cannot be decoded to the desired encoding. PyUnicode_FromFormat will crash if we pass NULL pointers to be formatted as %R or %U. Instead, let's use `to_unicode(..., ..., "replace")` so that we always get a non-NULL object to pass to PyUnicode_FromFormat. This ties in with libgit2#1155.
to_unicode()
expects a non-NULLx
argument (due tostrlen(x)
);however
self->encoding
may be NULL, especially if built withbuild_signature()
(i.e. if called from thecommit.author
property).As a result, any
repr()
on aSignature
with a NULLencoding
willcause a segfault.
Other functions manually map NULL to "utf-8" (see
Signature__encoding__get__()
), so we'll do the same thing here.