Fix parsing of multiple authors in PersonIdent. PersonIdent should be parsable for an invalid commit which contains multiple authors, like "A <a@a.org>, B <b@b.org>". PersonIdent(String) constructor now delegates to RawParseUtils.parsePersonIdent(). Change-Id: Ie9798d36d9ecfcc0094ca795f5a44b003136eaf7
diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java index b26dde3..dd33a16 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Commit.java
@@ -47,6 +47,7 @@ import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.util.RawParseUtils; import org.kohsuke.args4j.Option; @Command(common = true, usage = "usage_recordChangesToRepository") @@ -65,7 +66,7 @@ protected void run() throws NoHeadException, NoMessageException, ConcurrentRefUpdateException, JGitInternalException, Exception { CommitCommand commitCmd = new Git(db).commit(); if (author != null) - commitCmd.setAuthor(new PersonIdent(author)); + commitCmd.setAuthor(RawParseUtils.parsePersonIdent(author)); if (message != null) commitCmd.setMessage(message); Ref head = db.getRef(Constants.HEAD);
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdent.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdent.java index aaa88c0..b3aeb81 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdent.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/T0001_PersonIdent.java
@@ -55,57 +55,17 @@ public void test001_NewIdent() { assertEquals("A U Thor", p.getName()); assertEquals("author@example.com", p.getEmailAddress()); assertEquals(1142878501000L, p.getWhen().getTime()); - assertEquals("A U Thor <author@example.com> 1142878501 -0500", p - .toExternalString()); + assertEquals("A U Thor <author@example.com> 1142878501 -0500", + p.toExternalString()); } - public void test002_ParseIdent() { - final String i = "A U Thor <author@example.com> 1142878501 -0500"; - final PersonIdent p = new PersonIdent(i); - assertEquals(i, p.toExternalString()); + public void test002_NewIdent() { + final PersonIdent p = new PersonIdent("A U Thor", "author@example.com", + new Date(1142878501000L), TimeZone.getTimeZone("GMT+0230")); assertEquals("A U Thor", p.getName()); assertEquals("author@example.com", p.getEmailAddress()); assertEquals(1142878501000L, p.getWhen().getTime()); - } - - public void test003_ParseIdent() { - final String i = "A U Thor <author@example.com> 1142878501 +0230"; - final PersonIdent p = new PersonIdent(i); - assertEquals(i, p.toExternalString()); - assertEquals("A U Thor", p.getName()); - assertEquals("author@example.com", p.getEmailAddress()); - assertEquals(1142878501000L, p.getWhen().getTime()); - } - - public void test004_ParseIdent() { - final String i = "A U Thor<author@example.com> 1142878501 +0230"; - final PersonIdent p = new PersonIdent(i); - assertEquals("A U Thor", p.getName()); - assertEquals("author@example.com", p.getEmailAddress()); - assertEquals(1142878501000L, p.getWhen().getTime()); - } - - public void test005_ParseIdent() { - final String i = "A U Thor<author@example.com>1142878501 +0230"; - final PersonIdent p = new PersonIdent(i); - assertEquals("A U Thor", p.getName()); - assertEquals("author@example.com", p.getEmailAddress()); - assertEquals(1142878501000L, p.getWhen().getTime()); - } - - public void test006_ParseIdent() { - final String i = "A U Thor <author@example.com>1142878501 +0230"; - final PersonIdent p = new PersonIdent(i); - assertEquals("A U Thor", p.getName()); - assertEquals("author@example.com", p.getEmailAddress()); - assertEquals(1142878501000L, p.getWhen().getTime()); - } - - public void test007_ParseIdent() { - final String i = "A U Thor<author@example.com>1142878501 +0230 "; - final PersonIdent p = new PersonIdent(i); - assertEquals("A U Thor", p.getName()); - assertEquals("author@example.com", p.getEmailAddress()); - assertEquals(1142878501000L, p.getWhen().getTime()); + assertEquals("A U Thor <author@example.com> 1142878501 +0230", + p.toExternalString()); } }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java index 7be7dbc..0ad7247 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevCommitParseTest.java
@@ -45,6 +45,7 @@ import java.io.ByteArrayOutputStream; import java.io.UnsupportedEncodingException; +import java.util.TimeZone; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Constants; @@ -59,10 +60,12 @@ public void testParse_NoParents() throws Exception { final String authorName = "A U. Thor"; final String authorEmail = "a_u_thor@example.com"; final int authorTime = 1218123387; + final String authorTimeZone = "+0700"; final String committerName = "C O. Miter"; final String committerEmail = "comiter@example.com"; final int committerTime = 1218123390; + final String committerTimeZone = "-0500"; final StringBuilder body = new StringBuilder(); body.append("tree "); @@ -75,7 +78,9 @@ public void testParse_NoParents() throws Exception { body.append(authorEmail); body.append("> "); body.append(authorTime); - body.append(" +0700\n"); + body.append(" "); + body.append(authorTimeZone); + body.append(" \n"); body.append("committer "); body.append(committerName); @@ -83,7 +88,9 @@ public void testParse_NoParents() throws Exception { body.append(committerEmail); body.append("> "); body.append(committerTime); - body.append(" -0500\n"); + body.append(" "); + body.append(committerTimeZone); + body.append("\n"); body.append("\n"); @@ -107,11 +114,15 @@ public void testParse_NoParents() throws Exception { assertNotNull(cAuthor); assertEquals(authorName, cAuthor.getName()); assertEquals(authorEmail, cAuthor.getEmailAddress()); + assertEquals((long)authorTime * 1000, cAuthor.getWhen().getTime()); + assertEquals(TimeZone.getTimeZone("GMT" + authorTimeZone), cAuthor.getTimeZone()); final PersonIdent cCommitter = c.getCommitterIdent(); assertNotNull(cCommitter); assertEquals(committerName, cCommitter.getName()); assertEquals(committerEmail, cCommitter.getEmailAddress()); + assertEquals((long)committerTime * 1000, cCommitter.getWhen().getTime()); + assertEquals(TimeZone.getTimeZone("GMT" + committerTimeZone), cCommitter.getTimeZone()); } private RevCommit create(final String msg) throws Exception {
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java index a15cadf..c4adde3 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java
@@ -61,10 +61,10 @@ public class ChangeIdUtilTest extends TestCase { private final String SOB2 = "Signed-off-by: J Committer <jc@example.com>\n"; - final PersonIdent p = new PersonIdent( + final PersonIdent p = RawParseUtils.parsePersonIdent( "A U Thor <author@example.com> 1142878501 -0500"); - final PersonIdent q = new PersonIdent( + final PersonIdent q = RawParseUtils.parsePersonIdent( "W Riter <writer@example.com> 1142878502 -0500"); ObjectId treeId = ObjectId
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_ParsePersonIdentTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_ParsePersonIdentTest.java index 2981e31..e76cd48 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_ParsePersonIdentTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/RawParseUtils_ParsePersonIdentTest.java
@@ -43,58 +43,97 @@ package org.eclipse.jgit.util; -import java.io.UnsupportedEncodingException; import java.util.Date; import java.util.TimeZone; -import org.eclipse.jgit.lib.PersonIdent; - import junit.framework.TestCase; +import org.eclipse.jgit.lib.PersonIdent; + public class RawParseUtils_ParsePersonIdentTest extends TestCase { - public void testParsePersonIdent_legalCases() - throws UnsupportedEncodingException { + public void testParsePersonIdent_legalCases() { final Date when = new Date(1234567890000l); final TimeZone tz = TimeZone.getTimeZone("GMT-7"); - assertPersonIdent("Me <me@example.com> 1234567890 -0700", 0, + assertPersonIdent("Me <me@example.com> 1234567890 -0700", new PersonIdent("Me", "me@example.com", when, tz)); - assertPersonIdent(" Me <me@example.com> 1234567890 -0700", 1, - new PersonIdent("Me", "me@example.com", when, tz)); + assertPersonIdent(" Me <me@example.com> 1234567890 -0700", + new PersonIdent(" Me", "me@example.com", when, tz)); - assertPersonIdent("Me <> 1234567890 -0700", 0, new PersonIdent("Me", - "", when, tz)); + assertPersonIdent("A U Thor <author@example.com> 1234567890 -0700", + new PersonIdent("A U Thor", "author@example.com", when, tz)); - assertPersonIdent(" <me@example.com> 1234567890 -0700", 0, + assertPersonIdent("A U Thor<author@example.com> 1234567890 -0700", + new PersonIdent("A U Thor", "author@example.com", when, tz)); + + assertPersonIdent("A U Thor<author@example.com>1234567890 -0700", + new PersonIdent("A U Thor", "author@example.com", when, tz)); + + assertPersonIdent( + " A U Thor < author@example.com > 1234567890 -0700", + new PersonIdent(" A U Thor ", " author@example.com ", when, tz)); + + assertPersonIdent("A U Thor<author@example.com>1234567890 -0700", + new PersonIdent("A U Thor", "author@example.com", when, tz)); + } + + public void testParsePersonIdent_fuzzyCases() { + final Date when = new Date(1234567890000l); + final TimeZone tz = TimeZone.getTimeZone("GMT-7"); + + assertPersonIdent( + "A U Thor <author@example.com>, C O. Miter <comiter@example.com> 1234567890 -0700", + new PersonIdent("A U Thor", "author@example.com", when, tz)); + + assertPersonIdent( + "A U Thor <author@example.com> and others 1234567890 -0700", + new PersonIdent("A U Thor", "author@example.com", when, tz)); + } + + public void testParsePersonIdent_incompleteCases() { + final Date when = new Date(1234567890000l); + final TimeZone tz = TimeZone.getTimeZone("GMT-7"); + + assertPersonIdent("Me <> 1234567890 -0700", new PersonIdent("Me", "", + when, tz)); + + assertPersonIdent(" <me@example.com> 1234567890 -0700", new PersonIdent("", "me@example.com", when, tz)); - assertPersonIdent(" <> 1234567890 -0700", 0, new PersonIdent("", "", - when, tz)); + assertPersonIdent(" <> 1234567890 -0700", new PersonIdent("", "", when, + tz)); + + assertPersonIdent("<>", new PersonIdent("", "", 0, 0)); + + assertPersonIdent(" <>", new PersonIdent("", "", 0, 0)); + + assertPersonIdent("<me@example.com>", new PersonIdent("", + "me@example.com", 0, 0)); + + assertPersonIdent(" <me@example.com>", new PersonIdent("", + "me@example.com", 0, 0)); + + assertPersonIdent("Me <>", new PersonIdent("Me", "", 0, 0)); + + assertPersonIdent("Me <me@example.com>", new PersonIdent("Me", + "me@example.com", 0, 0)); + + assertPersonIdent("Me <me@example.com> 1234567890", new PersonIdent( + "Me", "me@example.com", 0, 0)); + + assertPersonIdent("Me <me@example.com> 1234567890 ", new PersonIdent( + "Me", "me@example.com", 0, 0)); } - public void testParsePersonIdent_malformedCases() - throws UnsupportedEncodingException { - assertPersonIdent("Me me@example.com> 1234567890 -0700", 0, null); - assertPersonIdent("Me <me@example.com 1234567890 -0700", 0, null); - - assertPersonIdent("<>", 0, null); - assertPersonIdent("<me@example.com>", 0, null); - assertPersonIdent(" <>", 0, null); - assertPersonIdent(" <me@example.com>", 0, null); - assertPersonIdent("Me <>", 0, null); - assertPersonIdent("Me <me@example.com>", 0, null); - - assertPersonIdent("Me <me@example.com> 1234567890", 0, null); - assertPersonIdent("<me@example.com> 1234567890 -0700", 0, null); - assertPersonIdent("<> 1234567890 -0700", 0, null); + public void testParsePersonIdent_malformedCases() { + assertPersonIdent("Me me@example.com> 1234567890 -0700", null); + assertPersonIdent("Me <me@example.com 1234567890 -0700", null); } - private void assertPersonIdent(String line, int nameB, PersonIdent expected) - throws UnsupportedEncodingException { - PersonIdent actual = RawParseUtils.parsePersonIdent(line - .getBytes("UTF-8"), nameB); + private void assertPersonIdent(String line, PersonIdent expected) { + PersonIdent actual = RawParseUtils.parsePersonIdent(line); assertEquals(expected, actual); } }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java index 25acee0..5361c0f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/PersonIdent.java
@@ -52,6 +52,7 @@ import java.util.TimeZone; import org.eclipse.jgit.JGitText; +import org.eclipse.jgit.util.RawParseUtils; import org.eclipse.jgit.util.SystemReader; /** @@ -195,37 +196,19 @@ public PersonIdent(final PersonIdent pi, final long aWhen, final int aTZ) { * * @param in * a Git internal format author/committer string. + * + * @deprecated Use {@link RawParseUtils#parsePersonIdent(String)} instead. */ public PersonIdent(final String in) { - final int lt = in.indexOf('<'); - if (lt == -1) { + final PersonIdent self = RawParseUtils.parsePersonIdent(in); + if (self == null) throw new IllegalArgumentException(MessageFormat.format( JGitText.get().malformedpersonIdentString, in)); - } - final int gt = in.indexOf('>', lt); - if (gt == -1) { - throw new IllegalArgumentException(MessageFormat.format( - JGitText.get().malformedpersonIdentString, in)); - } - final int sp = in.indexOf(' ', gt + 2); - if (sp == -1) { - when = 0; - tzOffset = -1; - } else { - final String tzHoursStr = in.substring(sp + 1, sp + 4).trim(); - final int tzHours; - if (tzHoursStr.charAt(0) == '+') { - tzHours = Integer.parseInt(tzHoursStr.substring(1)); - } else { - tzHours = Integer.parseInt(tzHoursStr); - } - final int tzMins = Integer.parseInt(in.substring(sp + 4).trim()); - when = Long.parseLong(in.substring(gt + 1, sp).trim()) * 1000; - tzOffset = tzHours * 60 + tzMins; - } - name = in.substring(0, lt).trim(); - emailAddress = in.substring(lt + 1, gt).trim(); + this.name = self.name; + this.emailAddress = self.emailAddress; + this.when = self.when; + this.tzOffset = self.tzOffset; } /**
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java index 6259f7c..38e0dde 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/RawParseUtils.java
@@ -653,6 +653,21 @@ public static Charset parseEncoding(final byte[] b) { } /** + * Parse a name string (e.g. author, committer, tagger) into a PersonIdent. + * <p> + * Leading spaces won't be trimmed from the string, i.e. will show up in the + * parsed name afterwards. + * + * @param in + * the string to parse a name from. + * @return the parsed identity or null in case the identity could not be + * parsed. + */ + public static PersonIdent parsePersonIdent(final String in) { + return parsePersonIdent(Constants.encode(in), 0); + } + + /** * Parse a name line (e.g. author, committer, tagger) into a PersonIdent. * <p> * When passing in a value for <code>nameB</code> callers should use the @@ -667,32 +682,42 @@ public static Charset parseEncoding(final byte[] b) { * first position after the space which delimits the header field * name (e.g. "author" or "committer") from the rest of the * identity line. - * @return the parsed identity. Never null. + * @return the parsed identity or null in case the identity could not be + * parsed. */ public static PersonIdent parsePersonIdent(final byte[] raw, final int nameB) { final Charset cs = parseEncoding(raw); final int emailB = nextLF(raw, nameB, '<'); final int emailE = nextLF(raw, emailB, '>'); - if (emailB <= nameB + 1 || // No name - emailB >= raw.length || // No email start - raw[emailB] == '\n' || - emailE >= raw.length - 1 || // No email end at all or no trailing date - raw[emailE] == '\n') { + if (emailB >= raw.length || raw[emailB] == '\n' || + (emailE >= raw.length - 1 && raw[emailE - 1] != '>')) return null; - } - final String name = decode(cs, raw, nameB, emailB - 2); + final int nameEnd = emailB - 2 >= 0 && raw[emailB - 2] == ' ' ? emailB - 2 + : emailB - 1; + final String name = decode(cs, raw, nameB, nameEnd); final String email = decode(cs, raw, emailB, emailE - 1); - final MutableInteger ptrout = new MutableInteger(); - final long when = parseLongBase10(raw, emailE + 1, ptrout); - final int whenE = ptrout.value; - if (whenE >= raw.length || // No trailing timezone - raw[whenE] == '\n') { - return null; - } + // Start searching from end of line, as after first name-email pair, + // another name-email pair may occur. We will ignore all kinds of + // "junk" following the first email. + // + // We've to use (emailE - 1) for the case that raw[email] is LF, + // otherwise we would run too far. "-2" is necessary to position + // before the LF in case of LF termination resp. the penultimate + // character if there is no trailing LF. + final int tzBegin = lastIndexOfTrim(raw, ' ', + nextLF(raw, emailE - 1) - 2) + 1; + if (tzBegin <= emailE) // No time/zone, still valid + return new PersonIdent(name, email, 0, 0); - final int tz = parseTimeZoneOffset(raw, whenE); + final int whenBegin = Math.max(emailE, + lastIndexOfTrim(raw, ' ', tzBegin - 1) + 1); + if (whenBegin >= tzBegin - 1) // No time/zone, still valid + return new PersonIdent(name, email, 0, 0); + + final long when = parseLongBase10(raw, whenBegin, null); + final int tz = parseTimeZoneOffset(raw, tzBegin); return new PersonIdent(name, email, when * 1000L, tz); } @@ -713,7 +738,8 @@ public static PersonIdent parsePersonIdent(final byte[] raw, final int nameB) { * identity line. * @return the parsed identity. Never null. */ - public static PersonIdent parsePersonIdentOnly(final byte[] raw, final int nameB) { + public static PersonIdent parsePersonIdentOnly(final byte[] raw, + final int nameB) { int stop = nextLF(raw, nameB); int emailB = nextLF(raw, nameB, '<'); int emailE = nextLF(raw, emailB, '>'); @@ -1022,6 +1048,16 @@ public static final int endOfParagraph(final byte[] b, final int start) { return ptr; } + private static int lastIndexOfTrim(byte[] raw, char ch, int pos) { + while (pos >= 0 && raw[pos] == ' ') + pos--; + + while (pos >= 0 && raw[pos] != ch) + pos--; + + return pos; + } + private RawParseUtils() { // Don't create instances of a static only utility. }