From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 11/15] Fix Patch.parse to honor the end point passed in
Date: Thu, 11 Dec 2008 18:46:17 -0800 [thread overview]
Message-ID: <1229049981-14152-12-git-send-email-spearce@spearce.org> (raw)
In-Reply-To: <1229049981-14152-11-git-send-email-spearce@spearce.org>
Otherwise we may over-read the patch script and identify trailer
data as part of a patch when it was requested that we ignore it.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
| 46 +++++++-----
.../src/org/spearce/jgit/patch/BinaryHunk.java | 5 +-
| 16 ++--
| 9 +--
.../src/org/spearce/jgit/patch/Patch.java | 73 ++++++++++----------
5 files changed, 77 insertions(+), 72 deletions(-)
--git a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
index 4c2140a..36c528e 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/patch/FileHeaderTest.java
@@ -45,28 +45,32 @@
public class FileHeaderTest extends TestCase {
public void testParseGitFileName_Empty() {
final FileHeader fh = data("");
- assertEquals(-1, fh.parseGitFileName(0));
+ assertEquals(-1, fh.parseGitFileName(0, fh.buf.length));
assertNotNull(fh.getHunks());
assertTrue(fh.getHunks().isEmpty());
assertFalse(fh.hasMetaDataChanges());
}
public void testParseGitFileName_NoLF() {
- assertEquals(-1, data("a/ b/").parseGitFileName(0));
+ final FileHeader fh = data("a/ b/");
+ assertEquals(-1, fh.parseGitFileName(0, fh.buf.length));
}
public void testParseGitFileName_NoSecondLine() {
- assertEquals(-1, data("\n").parseGitFileName(0));
+ final FileHeader fh = data("\n");
+ assertEquals(-1, fh.parseGitFileName(0, fh.buf.length));
}
public void testParseGitFileName_EmptyHeader() {
- assertEquals(1, data("\n\n").parseGitFileName(0));
+ final FileHeader fh = data("\n\n");
+ assertEquals(1, fh.parseGitFileName(0, fh.buf.length));
}
public void testParseGitFileName_Foo() {
final String name = "foo";
final FileHeader fh = header(name);
- assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
+ assertEquals(gitLine(name).length(), fh.parseGitFileName(0,
+ fh.buf.length));
assertEquals(name, fh.getOldName());
assertSame(fh.getOldName(), fh.getNewName());
assertFalse(fh.hasMetaDataChanges());
@@ -74,7 +78,7 @@ public void testParseGitFileName_Foo() {
public void testParseGitFileName_FailFooBar() {
final FileHeader fh = data("a/foo b/bar\n-");
- assertTrue(fh.parseGitFileName(0) > 0);
+ assertTrue(fh.parseGitFileName(0, fh.buf.length) > 0);
assertNull(fh.getOldName());
assertNull(fh.getNewName());
assertFalse(fh.hasMetaDataChanges());
@@ -83,7 +87,8 @@ public void testParseGitFileName_FailFooBar() {
public void testParseGitFileName_FooSpBar() {
final String name = "foo bar";
final FileHeader fh = header(name);
- assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
+ assertEquals(gitLine(name).length(), fh.parseGitFileName(0,
+ fh.buf.length));
assertEquals(name, fh.getOldName());
assertSame(fh.getOldName(), fh.getNewName());
assertFalse(fh.hasMetaDataChanges());
@@ -93,7 +98,8 @@ public void testParseGitFileName_DqFooTabBar() {
final String name = "foo\tbar";
final String dqName = "foo\\tbar";
final FileHeader fh = dqHeader(dqName);
- assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0));
+ assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0,
+ fh.buf.length));
assertEquals(name, fh.getOldName());
assertSame(fh.getOldName(), fh.getNewName());
assertFalse(fh.hasMetaDataChanges());
@@ -103,7 +109,8 @@ public void testParseGitFileName_DqFooSpLfNulBar() {
final String name = "foo \n\0bar";
final String dqName = "foo \\n\\0bar";
final FileHeader fh = dqHeader(dqName);
- assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0));
+ assertEquals(dqGitLine(dqName).length(), fh.parseGitFileName(0,
+ fh.buf.length));
assertEquals(name, fh.getOldName());
assertSame(fh.getOldName(), fh.getNewName());
assertFalse(fh.hasMetaDataChanges());
@@ -112,7 +119,8 @@ public void testParseGitFileName_DqFooSpLfNulBar() {
public void testParseGitFileName_SrcFooC() {
final String name = "src/foo/bar/argh/code.c";
final FileHeader fh = header(name);
- assertEquals(gitLine(name).length(), fh.parseGitFileName(0));
+ assertEquals(gitLine(name).length(), fh.parseGitFileName(0,
+ fh.buf.length));
assertEquals(name, fh.getOldName());
assertSame(fh.getOldName(), fh.getNewName());
assertFalse(fh.hasMetaDataChanges());
@@ -122,7 +130,7 @@ public void testParseGitFileName_SrcFooCNonStandardPrefix() {
final String name = "src/foo/bar/argh/code.c";
final String header = "project-v-1.0/" + name + " mydev/" + name + "\n";
final FileHeader fh = data(header + "-");
- assertEquals(header.length(), fh.parseGitFileName(0));
+ assertEquals(header.length(), fh.parseGitFileName(0, fh.buf.length));
assertEquals(name, fh.getOldName());
assertSame(fh.getOldName(), fh.getNewName());
assertFalse(fh.hasMetaDataChanges());
@@ -202,12 +210,12 @@ public void testParseRename100_NewStyle() {
+ "similarity index 100%\n"
+ "rename from a\n"
+ "rename to \" c/\\303\\205ngstr\\303\\266m\"\n");
- int ptr = fh.parseGitFileName(0);
+ int ptr = fh.parseGitFileName(0, fh.buf.length);
assertTrue(ptr > 0);
assertNull(fh.getOldName()); // can't parse names on a rename
assertNull(fh.getNewName());
- ptr = fh.parseGitHeaders(ptr);
+ ptr = fh.parseGitHeaders(ptr, fh.buf.length);
assertTrue(ptr > 0);
assertEquals("a", fh.getOldName());
@@ -231,12 +239,12 @@ public void testParseRename100_OldStyle() {
+ "similarity index 100%\n"
+ "rename old a\n"
+ "rename new \" c/\\303\\205ngstr\\303\\266m\"\n");
- int ptr = fh.parseGitFileName(0);
+ int ptr = fh.parseGitFileName(0, fh.buf.length);
assertTrue(ptr > 0);
assertNull(fh.getOldName()); // can't parse names on a rename
assertNull(fh.getNewName());
- ptr = fh.parseGitHeaders(ptr);
+ ptr = fh.parseGitHeaders(ptr, fh.buf.length);
assertTrue(ptr > 0);
assertEquals("a", fh.getOldName());
@@ -260,12 +268,12 @@ public void testParseCopy100() {
+ "similarity index 100%\n"
+ "copy from a\n"
+ "copy to \" c/\\303\\205ngstr\\303\\266m\"\n");
- int ptr = fh.parseGitFileName(0);
+ int ptr = fh.parseGitFileName(0, fh.buf.length);
assertTrue(ptr > 0);
assertNull(fh.getOldName()); // can't parse names on a copy
assertNull(fh.getNewName());
- ptr = fh.parseGitHeaders(ptr);
+ ptr = fh.parseGitHeaders(ptr, fh.buf.length);
assertTrue(ptr > 0);
assertEquals("a", fh.getOldName());
@@ -391,9 +399,9 @@ public void testParseAbbrIndexLine_NoMode() {
}
private static void assertParse(final FileHeader fh) {
- int ptr = fh.parseGitFileName(0);
+ int ptr = fh.parseGitFileName(0, fh.buf.length);
assertTrue(ptr > 0);
- ptr = fh.parseGitHeaders(ptr);
+ ptr = fh.parseGitHeaders(ptr, fh.buf.length);
assertTrue(ptr > 0);
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java b/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
index ce35286..3e07ec4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/BinaryHunk.java
@@ -91,7 +91,7 @@ public int getSize() {
return length;
}
- int parseHunk(int ptr) {
+ int parseHunk(int ptr, final int end) {
final byte[] buf = file.buf;
if (match(buf, ptr, LITERAL) >= 0) {
@@ -115,8 +115,7 @@ int parseHunk(int ptr) {
// encoded information in this hunk. To save time we don't do a
// validation of the binary data at this point.
//
- final int sz = buf.length;
- while (ptr < sz) {
+ while (ptr < end) {
final boolean empty = buf[ptr] == '\n';
ptr = nextLF(buf, ptr);
if (empty)
--git a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
index a6ff4a6..a58e978 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/FileHeader.java
@@ -291,12 +291,14 @@ public BinaryHunk getReverseBinaryHunk() {
*
* @param ptr
* first character after the "diff --git " or "diff --cc " part.
+ * @param end
+ * one past the last position to parse.
* @return first character after the LF at the end of the line; -1 on error.
*/
- int parseGitFileName(int ptr) {
+ int parseGitFileName(int ptr, final int end) {
final int eol = nextLF(buf, ptr);
final int bol = ptr;
- if (eol >= buf.length) {
+ if (eol >= end) {
return -1;
}
@@ -353,9 +355,8 @@ int parseGitFileName(int ptr) {
return eol;
}
- int parseGitHeaders(int ptr) {
- final int sz = buf.length;
- while (ptr < sz) {
+ int parseGitHeaders(int ptr, final int end) {
+ while (ptr < end) {
final int eol = nextLF(buf, ptr);
if (match(buf, ptr, HUNK_HDR) >= 0) {
// First hunk header; break out and parse them later.
@@ -428,9 +429,8 @@ int parseGitHeaders(int ptr) {
return ptr;
}
- int parseTraditionalHeaders(int ptr) {
- final int sz = buf.length;
- while (ptr < sz) {
+ int parseTraditionalHeaders(int ptr, final int end) {
+ while (ptr < end) {
final int eol = nextLF(buf, ptr);
if (match(buf, ptr, HUNK_HDR) >= 0) {
// First hunk header; break out and parse them later.
--git a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
index 20dd6e2..4fd9bae 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/HunkHeader.java
@@ -119,7 +119,7 @@ public int getLinesAdded() {
return nAdded;
}
- void parseHeader() {
+ void parseHeader(final int end) {
// Parse "@@ -236,9 +236,9 @@ protected boolean"
//
final byte[] buf = file.buf;
@@ -132,15 +132,14 @@ void parseHeader() {
newLineCount = parseBase10(buf, ptr.value + 1, ptr);
}
- int parseBody(final Patch script) {
+ int parseBody(final Patch script, final int end) {
final byte[] buf = file.buf;
- final int sz = buf.length;
int c = nextLF(buf, startOffset), last = c;
nDeleted = 0;
nAdded = 0;
- SCAN: for (; c < sz; last = c, c = nextLF(buf, c)) {
+ SCAN: for (; c < end; last = c, c = nextLF(buf, c)) {
switch (buf[c]) {
case ' ':
case '\n':
@@ -163,7 +162,7 @@ int parseBody(final Patch script) {
}
}
- if (last < sz && nContext + nDeleted - 1 == oldLineCount
+ if (last < end && nContext + nDeleted - 1 == oldLineCount
&& nContext + nAdded == newLineCount
&& match(buf, last, Patch.SIG_FOOTER) >= 0) {
// This is an extremely common occurrence of "corruption".
diff --git a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
index 5cc208c..9aca22d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/patch/Patch.java
@@ -157,12 +157,11 @@ public void parse(final InputStream is) throws IOException {
*/
public void parse(final byte[] buf, int ptr, final int end) {
while (ptr < end)
- ptr = parseFile(buf, ptr);
+ ptr = parseFile(buf, ptr, end);
}
- private int parseFile(final byte[] buf, int c) {
- final int sz = buf.length;
- while (c < sz) {
+ private int parseFile(final byte[] buf, int c, final int end) {
+ while (c < end) {
if (match(buf, c, HUNK_HDR) >= 0) {
// If we find a disconnected hunk header we might
// have missed a file header previously. The hunk
@@ -176,19 +175,19 @@ private int parseFile(final byte[] buf, int c) {
// Valid git style patch?
//
if (match(buf, c, DIFF_GIT) >= 0)
- return parseDiffGit(buf, c);
+ return parseDiffGit(buf, c, end);
if (match(buf, c, DIFF_CC) >= 0)
- return parseDiffCC(buf, c);
+ return parseDiffCC(buf, c, end);
// Junk between files? Leading junk? Traditional
// (non-git generated) patch?
//
final int n = nextLF(buf, c);
- if (n >= sz) {
+ if (n >= end) {
// Patches cannot be only one line long. This must be
// trailing junk that we should ignore.
//
- return sz;
+ return end;
}
if (n - c < 6) {
@@ -204,10 +203,10 @@ private int parseFile(final byte[] buf, int c) {
// a "@@ -0,0" smelling line next. We only check the "@@ -".
//
final int f = nextLF(buf, n);
- if (f >= sz)
- return sz;
+ if (f >= end)
+ return end;
if (match(buf, f, HUNK_HDR) >= 0)
- return parseTraditionalPatch(buf, c);
+ return parseTraditionalPatch(buf, c, end);
}
c = n;
@@ -215,53 +214,53 @@ private int parseFile(final byte[] buf, int c) {
return c;
}
- private int parseDiffGit(final byte[] buf, final int startOffset) {
- final FileHeader fh = new FileHeader(buf, startOffset);
- int ptr = fh.parseGitFileName(startOffset + DIFF_GIT.length);
+ private int parseDiffGit(final byte[] buf, final int start, final int end) {
+ final FileHeader fh = new FileHeader(buf, start);
+ int ptr = fh.parseGitFileName(start + DIFF_GIT.length, end);
if (ptr < 0)
- return skipFile(buf, startOffset);
+ return skipFile(buf, start, end);
- ptr = fh.parseGitHeaders(ptr);
- ptr = parseHunks(fh, ptr);
+ ptr = fh.parseGitHeaders(ptr, end);
+ ptr = parseHunks(fh, ptr, end);
fh.endOffset = ptr;
addFile(fh);
return ptr;
}
- private int parseDiffCC(final byte[] buf, final int startOffset) {
- final FileHeader fh = new FileHeader(buf, startOffset);
- int ptr = fh.parseGitFileName(startOffset + DIFF_CC.length);
+ private int parseDiffCC(final byte[] buf, final int start, final int end) {
+ final FileHeader fh = new FileHeader(buf, start);
+ int ptr = fh.parseGitFileName(start + DIFF_CC.length, end);
if (ptr < 0)
- return skipFile(buf, startOffset);
+ return skipFile(buf, start, end);
// TODO Support parsing diff --cc headers
// TODO parse diff --cc hunks
- warn(buf, startOffset, "diff --cc format not supported");
+ warn(buf, start, "diff --cc format not supported");
fh.endOffset = ptr;
addFile(fh);
return ptr;
}
- private int parseTraditionalPatch(final byte[] buf, final int startOffset) {
- final FileHeader fh = new FileHeader(buf, startOffset);
- int ptr = fh.parseTraditionalHeaders(startOffset);
- ptr = parseHunks(fh, ptr);
+ private int parseTraditionalPatch(final byte[] buf, final int start,
+ final int end) {
+ final FileHeader fh = new FileHeader(buf, start);
+ int ptr = fh.parseTraditionalHeaders(start, end);
+ ptr = parseHunks(fh, ptr, end);
fh.endOffset = ptr;
addFile(fh);
return ptr;
}
- private static int skipFile(final byte[] buf, int ptr) {
+ private static int skipFile(final byte[] buf, int ptr, final int end) {
ptr = nextLF(buf, ptr);
if (match(buf, ptr, OLD_NAME) >= 0)
ptr = nextLF(buf, ptr);
return ptr;
}
- private int parseHunks(final FileHeader fh, int c) {
+ private int parseHunks(final FileHeader fh, int c, final int end) {
final byte[] buf = fh.buf;
- final int sz = buf.length;
- while (c < sz) {
+ while (c < end) {
// If we see a file header at this point, we have all of the
// hunks for our current file. We should stop and report back
// with this position so it can be parsed again later.
@@ -277,11 +276,11 @@ private int parseHunks(final FileHeader fh, int c) {
if (match(buf, c, HUNK_HDR) >= 0) {
final HunkHeader h = new HunkHeader(fh, c);
- h.parseHeader();
- c = h.parseBody(this);
+ h.parseHeader(end);
+ c = h.parseBody(this, end);
h.endOffset = c;
fh.addHunk(h);
- if (c < sz && buf[c] != '@' && buf[c] != 'd'
+ if (c < end && buf[c] != '@' && buf[c] != 'd'
&& match(buf, c, SIG_FOOTER) < 0) {
warn(buf, c, "Unexpected hunk trailer");
}
@@ -291,7 +290,7 @@ private int parseHunks(final FileHeader fh, int c) {
final int eol = nextLF(buf, c);
if (fh.getHunks().isEmpty() && match(buf, c, GIT_BINARY) >= 0) {
fh.patchType = FileHeader.PatchType.GIT_BINARY;
- return parseGitBinary(fh, eol);
+ return parseGitBinary(fh, eol, end);
}
if (fh.getHunks().isEmpty() && BIN_TRAILER.length < eol - c
@@ -321,9 +320,9 @@ private int parseHunks(final FileHeader fh, int c) {
return c;
}
- private int parseGitBinary(final FileHeader fh, int c) {
+ private int parseGitBinary(final FileHeader fh, int c, final int end) {
final BinaryHunk postImage = new BinaryHunk(fh, c);
- final int nEnd = postImage.parseHunk(c);
+ final int nEnd = postImage.parseHunk(c, end);
if (nEnd < 0) {
// Not a binary hunk.
//
@@ -335,7 +334,7 @@ private int parseGitBinary(final FileHeader fh, int c) {
fh.forwardBinaryHunk = postImage;
final BinaryHunk preImage = new BinaryHunk(fh, c);
- final int oEnd = preImage.parseHunk(c);
+ final int oEnd = preImage.parseHunk(c, end);
if (oEnd >= 0) {
c = oEnd;
preImage.endOffset = c;
--
1.6.1.rc2.306.ge5d5e
next prev parent reply other threads:[~2008-12-12 2:48 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-12 2:46 [JGIT PATCH 00/15] More patch parsing support Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 01/15] Correct use of TemporaryBuffer in Patch Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 02/15] Add tests for TemporaryBuffer Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer> Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 04/15] Add lineMap computer to RawParseUtils to index locations of line starts Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 05/15] Define FileHeader.PatchType to report the style of patch used Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 06/15] Test for non-git binary files and mark them as PatchType.BINARY Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 07/15] Set empty patches with no Git metadata to PatchType.BINARY Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 08/15] Always use the FileHeader buffer during Patch.parseHunks Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 09/15] Parse "GIT binary patch" style patch metadata Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 10/15] Record patch parsing errors for later inspection by applications Shawn O. Pearce
2008-12-12 2:46 ` Shawn O. Pearce [this message]
2008-12-12 2:46 ` [JGIT PATCH 12/15] Correctly handle hunk headers such as "@@ -0,0 +1 @@" Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 13/15] Patch parse test comparing "git log -p" output to "git log --numstat" Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 14/15] Abstract the hunk header testing into a method Shawn O. Pearce
2008-12-12 2:46 ` [JGIT PATCH 15/15] Treat "diff --combined" the same as "diff --cc" Shawn O. Pearce
2008-12-12 23:11 ` Robin Rosenberg
2008-12-12 23:18 ` [JGIT PATCH 15/15 v2] " Shawn O. Pearce
[not found] ` <bd6139dc0812120243y2b1a3dddu4975162114280e17@mail.gmail.com>
2008-12-12 15:15 ` [JGIT PATCH 03/15] Add IntList as a more efficient representation of List<Integer> Shawn O. Pearce
2008-12-12 15:33 ` Sverre Rabbelier
2008-12-12 15:41 ` Shawn O. Pearce
2008-12-12 15:50 ` Sverre Rabbelier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1229049981-14152-12-git-send-email-spearce@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=robin.rosenberg@dewire.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).