From d43703624ce4ac3379a4632b3dbf1049cd96c918 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 10 Jun 2015 12:47:04 -0700 Subject: [PATCH] Allow trailing newlines in receive-pack C git's receive-pack.c strips trailing newlines in command lists when present[1], although send-pack.c does not send them, at least in the case of command lists[2]. Change JGit to match this behavior. Add tests. This also fixes parsing of commands in the push cert, which, unlike commands sent in the non-push case, always have trailing newlines. [1] https://github.com/git/git/blob/7974889a053574e449b55ca543a486e38e74864f/builtin/receive-pack.c#L1380 where packet_read_line chomps newlines: https://github.com/git/git/blob/7974889a053574e449b55ca543a486e38e74864f/pkt-line.c#L202 [2] https://github.com/git/git/blob/7974889a053574e449b55ca543a486e38e74864f/send-pack.c#L470 Change-Id: I4bca6342a7482a53c9a5815a94b3c181a479d04b --- .../jgit/transport/BaseReceivePackTest.java | 70 +++++++++++++++++++ .../jgit/transport/BaseReceivePack.java | 37 +++++++--- 2 files changed, 97 insertions(+), 10 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java new file mode 100644 index 000000000..98164d933 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2015, Google Inc. + * + * 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.transport; + +import static org.junit.Assert.assertEquals; + +import org.eclipse.jgit.lib.ObjectId; +import org.junit.Test; + +/** Tests for base receive-pack utilities. */ +public class BaseReceivePackTest { + @Test + public void chomp() { + assertEquals("foo", BaseReceivePack.chomp("foo")); + assertEquals("foo", BaseReceivePack.chomp("foo\n")); + assertEquals("foo\n", BaseReceivePack.chomp("foo\n\n")); + } + + @Test + public void parseCommand() { + String input = "0000000000000000000000000000000000000000" + + " deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + + " refs/heads/master"; + ReceiveCommand cmd = BaseReceivePack.parseCommand(input); + assertEquals(ObjectId.zeroId(), cmd.getOldId()); + assertEquals("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + cmd.getNewId().name()); + assertEquals("refs/heads/master", cmd.getRefName()); + } +} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index 9112ecbe3..7a1757722 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1037,16 +1037,18 @@ public void sendAdvertisedRefs(final RefAdvertiser adv) */ protected void recvCommands() throws IOException { for (;;) { - String line; + String rawLine; try { - line = pckIn.readStringRaw(); + rawLine = pckIn.readStringRaw(); } catch (EOFException eof) { if (commands.isEmpty()) return; throw eof; } - if (line == PacketLineIn.END) + if (rawLine == PacketLineIn.END) { break; + } + String line = chomp(rawLine); if (line.length() >= 48 && line.startsWith("shallow ")) { //$NON-NLS-1$ clientShallowCommits.add(ObjectId.fromString(line.substring(8, 48))); @@ -1066,8 +1068,11 @@ protected void recvCommands() throws IOException { if (line.equals("-----BEGIN PGP SIGNATURE-----\n")) //$NON-NLS-1$ pushCertificateParser.receiveSignature(pckIn); - if (pushCertificateParser.enabled()) - pushCertificateParser.addCommand(line); + if (pushCertificateParser.enabled()) { + // Must use raw line with optional newline so signed payload can be + // reconstructed. + pushCertificateParser.addCommand(rawLine); + } if (line.length() < 83) { final String m = JGitText.get().errorInvalidProtocolWantedOldNewRef; @@ -1075,11 +1080,8 @@ protected void recvCommands() throws IOException { throw new PackProtocolException(m); } - final ObjectId oldId = ObjectId.fromString(line.substring(0, 40)); - final ObjectId newId = ObjectId.fromString(line.substring(41, 81)); - final String name = line.substring(82); - final ReceiveCommand cmd = new ReceiveCommand(oldId, newId, name); - if (name.equals(Constants.HEAD)) { + final ReceiveCommand cmd = parseCommand(line); + if (cmd.getRefName().equals(Constants.HEAD)) { cmd.setResult(Result.REJECTED_CURRENT_BRANCH); } else { cmd.setRef(refs.get(cmd.getRefName())); @@ -1088,6 +1090,21 @@ protected void recvCommands() throws IOException { } } + static String chomp(String line) { + if (line != null && !line.isEmpty() + && line.charAt(line.length() - 1) == '\n') { + return line.substring(0, line.length() - 1); + } + return line; + } + + static ReceiveCommand parseCommand(String line) { + ObjectId oldId = ObjectId.fromString(line.substring(0, 40)); + ObjectId newId = ObjectId.fromString(line.substring(41, 81)); + String name = line.substring(82); + return new ReceiveCommand(oldId, newId, name); + } + /** Enable capabilities based on a previously read capabilities line. */ protected void enableCapabilities() { sideBand = isCapabilityEnabled(CAPABILITY_SIDE_BAND_64K);