CreateChange: Strip comment lines out of input commit message

Attempting to create a change with only a commit message subject where
the subject begins with a # fails due to an invalid (zeros) Change-Id.

The zero Change-Id is appended to the change because the JGit utility
method used to generate it strips the commit message of any comment
lines, which in this case results in an empty commit message, and then
the commit message does not have enough content.

Strip comment lines out of the input commit message before attempting
to use it to create the change. If it is empty after stripping the
comments, return 400 Bad Request.

Update the 'ChangeInput' documentation to mention that comment lines
will be stripped. Also change the description to reflect the fact that
the `subject` field is actually used for the entire commit message,
rather than only the subject.

Bug: Issue 9195
Change-Id: I5a2eed97aeb187801550f3a6b35e3dcfd2baf34e
This commit is contained in:
David Pursehouse 2018-06-07 14:44:33 +09:00
parent 70428fc5da
commit 38cb32d7b7
3 changed files with 33 additions and 3 deletions

View File

@ -5230,7 +5230,8 @@ The `ChangeInput` entity contains information about creating a new change.
The name of the target branch. +
The `refs/heads/` prefix is omitted.
|`subject` ||
The subject of the change (header line of the commit message).
The commit message of the change. Comment lines (beginning with `#`) will
be removed.
|`topic` |optional|The topic to which this change belongs.
|`status` |optional, default to `NEW`|
The status of the change (only `NEW` and `DRAFT` accepted here).

View File

@ -115,6 +115,13 @@ public class CreateChangeIT extends AbstractDaemonTest {
"missing subject; Change-Id must be in commit message footer");
}
@Test
public void createNewChange_InvalidCommentInCommitMessage() throws Exception {
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "#12345 Test";
assertCreateFails(ci, BadRequestException.class, "commit message must be non-empty");
}
@Test
public void createNewChange() throws Exception {
ChangeInfo info = assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
@ -122,6 +129,15 @@ public class CreateChangeIT extends AbstractDaemonTest {
.contains("Change-Id: " + info.changeId);
}
@Test
public void createNewChangeWithCommentsInCommitMessage() throws Exception {
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject += "\n# Comment line";
ChangeInfo info = gApi.changes().create(ci).get();
assertThat(info.revisions.get(info.currentRevision).commit.message)
.doesNotContain("# Comment line");
}
@Test
public void createNewChangeWithChangeId() throws Exception {
ChangeInput ci = newChangeInput(ChangeStatus.NEW);

View File

@ -156,7 +156,8 @@ public class CreateChange implements RestModifyView<TopLevelResource, ChangeInpu
throw new BadRequestException("branch must be non-empty");
}
if (Strings.isNullOrEmpty(input.subject)) {
String subject = clean(Strings.nullToEmpty(input.subject));
if (Strings.isNullOrEmpty(subject)) {
throw new BadRequestException("commit message must be non-empty");
}
@ -229,7 +230,7 @@ public class CreateChange implements RestModifyView<TopLevelResource, ChangeInpu
GeneralPreferencesInfo info = account.getAccount().getGeneralPreferencesInfo();
// Add a Change-Id line if there isn't already one
String commitMessage = input.subject;
String commitMessage = subject;
if (ChangeIdUtil.indexOfChangeId(commitMessage, "\n") == -1) {
ObjectId treeId = mergeTip == null ? emptyTreeId(oi) : mergeTip.getTree();
ObjectId id = ChangeIdUtil.computeChangeId(treeId, mergeTip, author, author, commitMessage);
@ -347,4 +348,16 @@ public class CreateChange implements RestModifyView<TopLevelResource, ChangeInpu
private static ObjectId emptyTreeId(ObjectInserter inserter) throws IOException {
return inserter.insert(new TreeFormatter());
}
/**
* Remove comment lines from a commit message.
*
* <p>Based on {@link org.eclipse.jgit.util.ChangeIdUtil#clean}.
*
* @param msg
* @return message without comment lines, possibly empty.
*/
private String clean(String msg) {
return msg.replaceAll("(?m)^#.*$\n?", "").trim();
}
}