From d2600693bd5fb8bda20bae41467132668caa1e14 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 21 Apr 2019 21:23:30 +0200 Subject: [PATCH] Fix off-by-one error in RebaseTodoFile when reading a todo file Commit messages of length 1 were not read. 'lineEnd' is the offset of the last character in the line before the terminating LF or CR-LF, and 'nextSpace' is actually the offset of the character _after_ the next space. With a one-character commit message, nextSpace == lineEnd. The code also assumes the commit message to be optional, but actually failed in that case because it read beyond the line ending. Fix that, too. Add a test case for reading a todo file. Bug: 546245 Change-Id: I368d63615930ea2398a6230e756442fd88870654 Signed-off-by: Thomas Wolf --- .../eclipse/jgit/lib/RebaseTodoFileTest.java | 142 ++++++++++++++++++ .../org/eclipse/jgit/lib/RebaseTodoFile.java | 12 +- 2 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RebaseTodoFileTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RebaseTodoFileTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RebaseTodoFileTest.java new file mode 100644 index 0000000000..5cfc75ca93 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RebaseTodoFileTest.java @@ -0,0 +1,142 @@ +/* + * Copyright (C) 2019, Thomas Wolf + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.lib; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; + +import org.eclipse.jgit.junit.RepositoryTestCase; +import org.junit.Test; + +public class RebaseTodoFileTest extends RepositoryTestCase { + + private static final String TEST_TODO = "test.todo"; + + private void createTodoList(String... lines) throws IOException { + Path p = Paths.get(db.getDirectory().getAbsolutePath(), TEST_TODO); + Files.write(p, Arrays.asList(lines)); + } + + @Test + public void testReadTodoFile() throws Exception { + String[] expected = { "reword " + ObjectId.zeroId().name() + " Foo", + "# A comment in the the todo list", + "pick " + ObjectId.zeroId().name() + " Foo fie", + "squash " + ObjectId.zeroId().name() + " F", + "fixup " + ObjectId.zeroId().name(), + "edit " + ObjectId.zeroId().name() + " f", + "edit " + ObjectId.zeroId().name() + ' ' }; + createTodoList(expected); + RebaseTodoFile todo = new RebaseTodoFile(db); + List lines = todo.readRebaseTodo(TEST_TODO, true); + assertEquals("Expected 7 lines", 7, lines.size()); + int i = 0; + for (RebaseTodoLine line : lines) { + switch (i) { + case 0: + assertEquals("Expected REWORD", RebaseTodoLine.Action.REWORD, + line.getAction()); + assertEquals("Unexpected ID", ObjectId.zeroId().abbreviate(40), + line.getCommit()); + assertEquals("Unexpected Message", "Foo", + line.getShortMessage()); + break; + case 1: + assertEquals("Expected COMMENT", RebaseTodoLine.Action.COMMENT, + line.getAction()); + assertEquals("Unexpected Message", + "# A comment in the the todo list", + line.getComment()); + break; + case 2: + assertEquals("Expected PICK", RebaseTodoLine.Action.PICK, + line.getAction()); + assertEquals("Unexpected ID", ObjectId.zeroId().abbreviate(40), + line.getCommit()); + assertEquals("Unexpected Message", "Foo fie", + line.getShortMessage()); + break; + case 3: + assertEquals("Expected SQUASH", RebaseTodoLine.Action.SQUASH, + line.getAction()); + assertEquals("Unexpected ID", ObjectId.zeroId().abbreviate(40), + line.getCommit()); + assertEquals("Unexpected Message", "F", line.getShortMessage()); + break; + case 4: + assertEquals("Expected FIXUP", RebaseTodoLine.Action.FIXUP, + line.getAction()); + assertEquals("Unexpected ID", ObjectId.zeroId().abbreviate(40), + line.getCommit()); + assertEquals("Unexpected Message", "", line.getShortMessage()); + break; + case 5: + assertEquals("Expected EDIT", RebaseTodoLine.Action.EDIT, + line.getAction()); + assertEquals("Unexpected ID", ObjectId.zeroId().abbreviate(40), + line.getCommit()); + assertEquals("Unexpected Message", "f", line.getShortMessage()); + break; + case 6: + assertEquals("Expected EDIT", RebaseTodoLine.Action.EDIT, + line.getAction()); + assertEquals("Unexpected ID", ObjectId.zeroId().abbreviate(40), + line.getCommit()); + assertEquals("Unexpected Message", "", line.getShortMessage()); + break; + default: + fail("Too many lines"); + return; + } + i++; + } + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java index 06b4b227c8..c0fcd4161f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RebaseTodoFile.java @@ -179,7 +179,7 @@ public class RebaseTodoFile { int nextSpace = RawParseUtils.next(buf, tokenBegin, ' '); int tokenCount = 0; - while (tokenCount < 3 && nextSpace < lineEnd) { + while (tokenCount < 3 && nextSpace <= lineEnd) { switch (tokenCount) { case 0: String actionToken = new String(buf, tokenBegin, @@ -191,8 +191,14 @@ public class RebaseTodoFile { break; case 1: nextSpace = RawParseUtils.next(buf, tokenBegin, ' '); - String commitToken = new String(buf, tokenBegin, - nextSpace - tokenBegin - 1, UTF_8); + String commitToken; + if (nextSpace > lineEnd + 1) { + commitToken = new String(buf, tokenBegin, + lineEnd - tokenBegin + 1, UTF_8); + } else { + commitToken = new String(buf, tokenBegin, + nextSpace - tokenBegin - 1, UTF_8); + } tokenBegin = nextSpace; commit = AbbreviatedObjectId.fromString(commitToken); break; -- 2.39.5