<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/libgit2.git/tests/patch, branch ethomson/test_https</title>
<subtitle>github.com: libgit2/libgit2.git
</subtitle>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/'/>
<entry>
<title>tests: declare functions statically where appropriate</title>
<updated>2021-11-11T22:31:43+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2021-11-11T18:28:08+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=ca14942e19788bd01334af64554c3095f3ff0d4a'/>
<id>ca14942e19788bd01334af64554c3095f3ff0d4a</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>str: introduce `git_str` for internal, `git_buf` is external</title>
<updated>2021-10-17T13:49:01+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2021-09-07T21:53:49+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=f0e693b18afbe1de37d7da5b5a8967b6c87d8e53'/>
<id>f0e693b18afbe1de37d7da5b5a8967b6c87d8e53</id>
<content type='text'>
libgit2 has two distinct requirements that were previously solved by
`git_buf`.  We require:

1. A general purpose string class that provides a number of utility APIs
   for manipulating data (eg, concatenating, truncating, etc).
2. A structure that we can use to return strings to callers that they
   can take ownership of.

By using a single class (`git_buf`) for both of these purposes, we have
confused the API to the point that refactorings are difficult and
reasoning about correctness is also difficult.

Move the utility class `git_buf` to be called `git_str`: this represents
its general purpose, as an internal string buffer class.  The name also
is an homage to Junio Hamano ("gitstr").

The public API remains `git_buf`, and has a much smaller footprint.  It
is generally only used as an "out" param with strict requirements that
follow the documentation.  (Exceptions exist for some legacy APIs to
avoid breaking callers unnecessarily.)

Utility functions exist to convert a user-specified `git_buf` to a
`git_str` so that we can call internal functions, then converting it
back again.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
libgit2 has two distinct requirements that were previously solved by
`git_buf`.  We require:

1. A general purpose string class that provides a number of utility APIs
   for manipulating data (eg, concatenating, truncating, etc).
2. A structure that we can use to return strings to callers that they
   can take ownership of.

By using a single class (`git_buf`) for both of these purposes, we have
confused the API to the point that refactorings are difficult and
reasoning about correctness is also difficult.

Move the utility class `git_buf` to be called `git_str`: this represents
its general purpose, as an internal string buffer class.  The name also
is an homage to Junio Hamano ("gitstr").

The public API remains `git_buf`, and has a much smaller footprint.  It
is generally only used as an "out" param with strict requirements that
follow the documentation.  (Exceptions exist for some legacy APIs to
avoid breaking callers unnecessarily.)

Utility functions exist to convert a user-specified `git_buf` to a
`git_str` so that we can call internal functions, then converting it
back again.
</pre>
</div>
</content>
</entry>
<entry>
<title>patch: correctly handle mode changes for renames</title>
<updated>2020-03-26T13:21:35+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2020-03-26T13:16:41+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=5f47cb48d388279f92d339a5a791040254ee4d1c'/>
<id>5f47cb48d388279f92d339a5a791040254ee4d1c</id>
<content type='text'>
When generating a patch for a renamed file whose mode bits have changed
in addition to the rename, then we currently fail to parse the generated
patch. Furthermore, when generating a diff we output mode bits after the
similarity metric, which is different to how upstream git handles it.

Fix both issues by adding another state transition that allows
similarity indices after mode changes and by printing mode changes
before the similarity index.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When generating a patch for a renamed file whose mode bits have changed
in addition to the rename, then we currently fail to parse the generated
patch. Furthermore, when generating a diff we output mode bits after the
similarity metric, which is different to how upstream git handles it.

Fix both issues by adding another state transition that allows
similarity indices after mode changes and by printing mode changes
before the similarity index.
</pre>
</div>
</content>
</entry>
<entry>
<title>patch_parse: fix out-of-bounds reads caused by integer underflow</title>
<updated>2019-11-28T14:26:36+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-11-28T14:26:36+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=33e6c40276dec95caa8120387622daf71265e143'/>
<id>33e6c40276dec95caa8120387622daf71265e143</id>
<content type='text'>
The patch format for binary files is a simple Base85 encoding with a
length byte as prefix that encodes the current line's length. For each
line, we thus check whether the line's actual length matches its
expected length in order to not faultily apply a truncated patch. This
also acts as a check to verify that we're not reading outside of the
line's string:

	if (encoded_len &gt; ctx-&gt;parse_ctx.line_len - 1) {
		error = git_parse_err(...);
		goto done;
	}

There is the possibility for an integer underflow, though. Given a line
with a single prefix byte, only, `line_len` will be zero when reaching
this check. As a result, subtracting one from that will result in an
integer underflow, causing us to assume that there's a wealth of bytes
available later on. Naturally, this may result in an out-of-bounds read.

Fix the issue by checking both `encoded_len` and `line_len` for a
non-zero value. The binary format doesn't make use of zero-length lines
anyway, so we need to know that there are both encoded bytes and
remaining characters available at all.

This patch also adds a test that works based on the last error message.
Checking error messages is usually too tightly coupled, but in fact
parsing the patch failed even before the change. Thus the only
possibility is to use e.g. Valgrind, but that'd result in us not
catching issues when run without Valgrind. As a result, using the error
message is considered a viable tradeoff as we know that we didn't start
decoding Base85 in the first place.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The patch format for binary files is a simple Base85 encoding with a
length byte as prefix that encodes the current line's length. For each
line, we thus check whether the line's actual length matches its
expected length in order to not faultily apply a truncated patch. This
also acts as a check to verify that we're not reading outside of the
line's string:

	if (encoded_len &gt; ctx-&gt;parse_ctx.line_len - 1) {
		error = git_parse_err(...);
		goto done;
	}

There is the possibility for an integer underflow, though. Given a line
with a single prefix byte, only, `line_len` will be zero when reaching
this check. As a result, subtracting one from that will result in an
integer underflow, causing us to assume that there's a wealth of bytes
available later on. Naturally, this may result in an out-of-bounds read.

Fix the issue by checking both `encoded_len` and `line_len` for a
non-zero value. The binary format doesn't make use of zero-length lines
anyway, so we need to know that there are both encoded bytes and
remaining characters available at all.

This patch also adds a test that works based on the last error message.
Checking error messages is usually too tightly coupled, but in fact
parsing the patch failed even before the change. Thus the only
possibility is to use e.g. Valgrind, but that'd result in us not
catching issues when run without Valgrind. As a result, using the error
message is considered a viable tradeoff as we know that we didn't start
decoding Base85 in the first place.
</pre>
</div>
</content>
</entry>
<entry>
<title>diff: make patchid computation work with all types of commits.</title>
<updated>2019-11-28T13:17:50+00:00</updated>
<author>
<name>Gregory Herrero</name>
<email>gregory.herrero@oracle.com</email>
</author>
<published>2019-11-07T13:10:00+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=ece5bb5e7d6e35e50096bac3d7bf17342daaec77'/>
<id>ece5bb5e7d6e35e50096bac3d7bf17342daaec77</id>
<content type='text'>
Current implementation of patchid is not computing a correct patchid
when given a patch where, for example, a new file is added or removed.
Some more corner cases need to be handled to have same behavior as git
patch-id command.
Add some more tests to cover those corner cases.

Signed-off-by: Gregory Herrero &lt;gregory.herrero@oracle.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Current implementation of patchid is not computing a correct patchid
when given a patch where, for example, a new file is added or removed.
Some more corner cases need to be handled to have same behavior as git
patch-id command.
Add some more tests to cover those corner cases.

Signed-off-by: Gregory Herrero &lt;gregory.herrero@oracle.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>patch_parse: correct parsing of patch containing not shown binary data.</title>
<updated>2019-11-19T08:33:12+00:00</updated>
<author>
<name>Gregory Herrero</name>
<email>gregory.herrero@oracle.com</email>
</author>
<published>2019-11-07T13:13:14+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=048e94adbba3c21b9ad739640cce11a8b387df48'/>
<id>048e94adbba3c21b9ad739640cce11a8b387df48</id>
<content type='text'>
When not shown binary data is added or removed in a patch, patch parser
is currently returning 'error -1 - corrupt git binary header at line 4'.
Fix it by correctly handling case where binary data is added/removed.

Signed-off-by: Gregory Herrero &lt;gregory.herrero@oracle.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When not shown binary data is added or removed in a patch, patch parser
is currently returning 'error -1 - corrupt git binary header at line 4'.
Fix it by correctly handling case where binary data is added/removed.

Signed-off-by: Gregory Herrero &lt;gregory.herrero@oracle.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>patch_parse: use paths from "---"/"+++" lines for binary patches</title>
<updated>2019-11-10T17:54:01+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-11-10T17:44:56+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=de7659ccc8cacc4d375e47021a1d634a931c78e7'/>
<id>de7659ccc8cacc4d375e47021a1d634a931c78e7</id>
<content type='text'>
For some patches, it is not possible to derive the old and new file
paths from the patch header's first line, most importantly when they
contain spaces. In such a case, we derive both paths from the "---" and
"+++" lines, which allow for non-ambiguous parsing. We fail to use these
paths when parsing binary patches without data, though, as we always
expect the header paths to be filled in.

Fix this by using the "---"/"+++" paths by default and only fall back to
header paths if they aren't set. If neither of those paths are set, we
just return an error. Add two tests to verify this behaviour, one of
which would have previously caused a segfault.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
For some patches, it is not possible to derive the old and new file
paths from the patch header's first line, most importantly when they
contain spaces. In such a case, we derive both paths from the "---" and
"+++" lines, which allow for non-ambiguous parsing. We fail to use these
paths when parsing binary patches without data, though, as we always
expect the header paths to be filled in.

Fix this by using the "---"/"+++" paths by default and only fall back to
header paths if they aren't set. If neither of those paths are set, we
just return an error. Add two tests to verify this behaviour, one of
which would have previously caused a segfault.
</pre>
</div>
</content>
</entry>
<entry>
<title>patch_parse: fix segfault when header path contains whitespace only</title>
<updated>2019-11-05T21:50:41+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-11-05T21:44:27+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=de543e297d85f1ac700c2e5a83e70b1bd32b9518'/>
<id>de543e297d85f1ac700c2e5a83e70b1bd32b9518</id>
<content type='text'>
When parsing header paths from a patch, we reject any patches with empty
paths as malformed patches. We perform the check whether a path is empty
before sanitizing it, though, which may lead to a path becoming empty
after the check, e.g. if we have trimmed whitespace. This may lead to a
segfault later when any part of our patching logic actually references
such a path, which may then be a `NULL` pointer.

Fix the issue by performing the check after sanitizing. Add tests to
catch the issue as they would have produced a segfault previosuly.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When parsing header paths from a patch, we reject any patches with empty
paths as malformed patches. We perform the check whether a path is empty
before sanitizing it, though, which may lead to a path becoming empty
after the check, e.g. if we have trimmed whitespace. This may lead to a
segfault later when any part of our patching logic actually references
such a path, which may then be a `NULL` pointer.

Fix the issue by performing the check after sanitizing. Add tests to
catch the issue as they would have produced a segfault previosuly.
</pre>
</div>
</content>
</entry>
<entry>
<title>patch_parse: detect overflow when calculating old/new line position</title>
<updated>2019-10-21T18:07:42+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-10-21T16:56:59+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=37141ff7701e45ab0d97f311a4e0cc95cf527aa9'/>
<id>37141ff7701e45ab0d97f311a4e0cc95cf527aa9</id>
<content type='text'>
When the patch contains lines close to INT_MAX, then it may happen that
we end up with an integer overflow when calculating the line of the
current diff hunk. Reject such patches as unreasonable to avoid the
integer overflow.

As the calculation is performed on integers, we introduce two new
helpers `git__add_int_overflow` and `git__sub_int_overflow` that perform
the integer overflow check in a generic way.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When the patch contains lines close to INT_MAX, then it may happen that
we end up with an integer overflow when calculating the line of the
current diff hunk. Reject such patches as unreasonable to avoid the
integer overflow.

As the calculation is performed on integers, we introduce two new
helpers `git__add_int_overflow` and `git__sub_int_overflow` that perform
the integer overflow check in a generic way.
</pre>
</div>
</content>
</entry>
<entry>
<title>patch_parse: fix out-of-bounds read with No-NL lines</title>
<updated>2019-10-19T15:11:48+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-10-19T14:48:11+00:00</published>
<link rel='alternate' type='text/html' href='http://91.123.203.49/cgit/delta/libgit2.git/commit/?id=468e3ddc344d6374a615fb2199005956ad0eb531'/>
<id>468e3ddc344d6374a615fb2199005956ad0eb531</id>
<content type='text'>
We've got two locations where we copy lines into the patch. The first
one is when copying normal " ", "-" or "+" lines, while the second
location gets executed when we copy "\ No newline at end of file" lines.
While the first one correctly uses `git__strndup` to copy only until the
newline, the other one doesn't. Thus, if the line occurs at the end of
the patch and if there is no terminating NUL character, then it may
result in an out-of-bounds read.

Fix the issue by using `git__strndup`, as was already done in the other
location. Furthermore, add allocation checks to both locations to detect
out-of-memory situations.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We've got two locations where we copy lines into the patch. The first
one is when copying normal " ", "-" or "+" lines, while the second
location gets executed when we copy "\ No newline at end of file" lines.
While the first one correctly uses `git__strndup` to copy only until the
newline, the other one doesn't. Thus, if the line occurs at the end of
the patch and if there is no terminating NUL character, then it may
result in an out-of-bounds read.

Fix the issue by using `git__strndup`, as was already done in the other
location. Furthermore, add allocation checks to both locations to detect
out-of-memory situations.
</pre>
</div>
</content>
</entry>
</feed>
