Page MenuHomePhabricator
Create Task
Maniphest T358762

Gerrit commit message formatting does not handle angle-bracketed URLs well, adds extra semicolon
Closed, ResolvedPublic
Actions

Description

Test case: https://gerrit.wikimedia.org/r/c/test/gerrit-ping/+/1043020

In this WikibaseLexeme Gerrit change, the commit message line

Pulls in <https://github.com/wmde/new-lexeme-special-page/pull/641>.

gets rendered as:

Pulls in <https://github.com/wmde/new-lexeme-special-page/pull/641>;.

Notice that the > became part of the link, and there’s also an extra ; for no apparent reason; angle brackets are supposed to be a way to mark the boundaries of a URL in Markdown-like text.

From duplicated T359878:

In this change: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1009851

The Gerrit interface shows this:

image.png (89×1 px, 21 KB)

he Zend hack is no longer needed, as this existed to support PHP 5.5
as per <https://3v4l.org/EU6Ro>;.

But in the actual commit, as seen e.g. at https://gerrit.wikimedia.org/g/mediawiki/core/+/6194cf19ce73a90bb3bedf01a9c97eb5fbc40d17, the text is written as:

The Zend hack is no longer needed, as this existed to support PHP 5.5
as per <https://3v4l.org/EU6Ro>.

Gerrit shows an extra ; that appears out of nowhere.

Details

SubjectRepoBranchLines +/-
Angle bracketed URL in commit messagetest/gerrit-pingmaster+0 -0
Update to a snapshot of Gerrit 3.9.6operations/software/gerritdeploy/wmf/stable-3.9+3 -3
Merge commit 'stable-3.9@7380128525' into wmf/stable-3.9operations/software/gerritwmf/stable-3.9+0 -0
Customize query in gerrit

Related Objects

Event Timeline

Lucas_Werkmeister_WMDE created this task.Feb 29 2024, 1:48 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 29 2024, 1:48 PM
Lucas_Werkmeister_WMDE added a comment.Feb 29 2024, 1:48 PM

I don’t know if this is anything specific in our config or an upstream issue – I figured I’d report it here first.

Lucas_Werkmeister_WMDE added a comment.Feb 29 2024, 1:50 PM

I’ve now pushed a new patch set that removes the angle brackets; unfortunately, Gerrit always shows the latest commit message even when looking at an older patch set, so that means you can’t easily reproduce this anymore :S

Paladox subscribed.Feb 29 2024, 1:56 PM

This is a upstream issue but testing on master it appears it's fixed.

Paladox added a comment.Feb 29 2024, 1:59 PM

I believe gerrit 3.8 may fix the problem. It got rid of a library that had problems. Although I can't guarantee as I've only tested on master.

Lucas_Werkmeister_WMDE added a comment.Feb 29 2024, 2:03 PM

Alright, thanks! Feel free to close this already then. (Or keep it open and link it to T354886, I guess?)

hashar merged a task: T359878: External link in commit message rendered incorrectly in Gerrit.Mar 20 2024, 2:19 PM
hashar updated the task description. (Show Details)
hashar added subscribers: matmarex, Krinkle.
hashar edited projects, added Gerrit (Gerrit 3.8); removed Gerrit.Mar 20 2024, 4:45 PM
hashar subscribed.
In T358762#9587036, @Lucas_Werkmeister_WMDE wrote:

Alright, thanks! Feel free to close this already then. (Or keep it open and link it to T354886, I guess?)

I keep the bug reports opens and mark them with a milestone associated with the version that fixes it, in this case #gerrit3.8: Gerrit (Gerrit 3.8). Then once upgraded, we can crawl through the milestone, verify all those have indeed been fixed and closed them as needed (else move them to the next milestone or back in the default Gerrit).

hashar closed this task as Resolved.Apr 17 2024, 12:20 PM
hashar claimed this task.

The issue has been fixed by upgrading to Gerrit 3.8 on Monday 15th :)

Lucas_Werkmeister_WMDE reopened this task as Open.May 4 2024, 11:23 AM

Still seems to be happening at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/cldr/+/1027084 :/

image.png (670×1 px, 138 KB)

hashar added a comment.May 7 2024, 7:10 AM

Indeed that was not fixed. The reason I have marked the task resolved was that the first link in this task points to https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseLexeme/+/1007609/1 which does not show the issue and I thus assumed it to have been fixed but it does show https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1009851 which I guess is a variation.

Lucas_Werkmeister_WMDE added a comment.May 13 2024, 8:42 AM
In T358762#9776514, @hashar wrote:

Indeed that was not fixed. The reason I have marked the task resolved was that the first link in this task points to https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseLexeme/+/1007609/1 which does not show the issue

Yeah, that’s because of:

In T358762#9586967, @Lucas_Werkmeister_WMDE wrote:

I’ve now pushed a new patch set that removes the angle brackets; unfortunately, Gerrit always shows the latest commit message even when looking at an older patch set, so that means you can’t easily reproduce this anymore :S

hashar added a comment.Jun 10 2024, 2:08 PM

We have upgraded to Gerrit 3.9.5 and this is still happening.

gerritbot added a comment.Jun 13 2024, 11:44 AM

Change #1043020 had a related patch set uploaded (by Hashar; author: Hashar):

[test/gerrit-ping@master] Angle bracketed URL in commit message

https://gerrit.wikimedia.org/r/1043020

gerritbot added a project: Patch-For-Review.Jun 13 2024, 11:44 AM
hashar edited projects, added Upstream; removed Patch-For-Review.Jun 13 2024, 11:45 AM
hashar updated the task description. (Show Details)
hashar added a comment.Jun 13 2024, 12:06 PM

Fun thing I have noticed is the <a> element shallows the closing bracket and the href contains the closing bracket. So it is more complicated than just a ; being added. I think the link is generated by:

polygerrit-ui/app/utils/link-util.ts
function createLinkTemplate(
  href: string,
  displayText: string,
  prefix?: string,
  suffix?: string
) {
  return `${
    prefix ?? ''
  }<a href="${href}" rel="noopener noreferrer" target="_blank">${displayText}</a>${
    suffix ?? ''
  }`;
}

Which would be fed:

prefix<
hrefhttps://example.com>
displayTexthttps://example.com>
suffix;

The method is apparently called solely for processing commentlink which are regex feeding the above function. Maybe one of our configured comment link is the cause of the issue?

Paladox added a comment.Jun 13 2024, 12:19 PM
In T358762#9888333, @hashar wrote:

Fun thing I have noticed is the <a> element shallows the closing bracket and the href contains the closing bracket. So it is more complicated than just a ; being added. I think the link is generated by:

polygerrit-ui/app/utils/link-util.ts
function createLinkTemplate(
  href: string,
  displayText: string,
  prefix?: string,
  suffix?: string
) {
  return `${
    prefix ?? ''
  }<a href="${href}" rel="noopener noreferrer" target="_blank">${displayText}</a>${
    suffix ?? ''
  }`;
}

Which would be fed:

prefix<
hrefhttps://example.com>
displayTexthttps://example.com>
suffix;

The method is apparently called solely for processing commentlink which are regex feeding the above function. Maybe one of our configured comment link is the cause of the issue?

It works for me on master:

Screenshot 2024-06-13 at 13.19.47.png (552×1 px, 136 KB)

gerritbot added a comment.Jun 13 2024, 12:42 PM

Change #1043020 abandoned by Hashar:

[test/gerrit-ping@master] Angle bracketed URL in commit message

Reason:

https://gerrit.wikimedia.org/r/1043020

Paladox added a comment.Jun 13 2024, 12:44 PM

Found the commit that fixes this https://gerrit-review.googlesource.com/c/gerrit/+/393975. This was incorrectly reverted in https://gerrit-review.googlesource.com/c/gerrit/+/395140 by mistake. I've re-done it with https://gerrit-review.googlesource.com/c/gerrit/+/429837.

gerritbot added a comment.Jun 13 2024, 1:41 PM

Change #1043066 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@wmf/stable-3.9] Merge commit 'stable-3.9@7380128525' into wmf/stable-3.9

https://gerrit.wikimedia.org/r/1043066

gerritbot added a project: Patch-For-Review.Jun 13 2024, 1:41 PM
gerritbot added a comment.Jun 13 2024, 1:53 PM

Change #1043066 merged by jenkins-bot:

[operations/software/gerrit@wmf/stable-3.9] Merge commit 'stable-3.9@7380128525' into wmf/stable-3.9

https://gerrit.wikimedia.org/r/1043066

gerritbot added a comment.Jun 13 2024, 2:21 PM

Change #1043084 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.9] Update to a snapshot of Gerrit 3.9.6

https://gerrit.wikimedia.org/r/1043084

gerritbot added a comment.Jun 13 2024, 2:27 PM

Change #1043084 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.9] Update to a snapshot of Gerrit 3.9.6

https://gerrit.wikimedia.org/r/1043084

hashar reassigned this task from hashar to Paladox.Jun 13 2024, 2:29 PM
Stashbot added a comment.Jun 13 2024, 2:32 PM

Mentioned in SAL (#wikimedia-operations) [2024-06-13T14:32:11Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@89042ad]: Gerrit to snapshot version 3.9.5-22-g7380128525 on gerrit2002 # T358762

Stashbot added a comment.Jun 13 2024, 2:32 PM

Mentioned in SAL (#wikimedia-operations) [2024-06-13T14:32:18Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@89042ad]: Gerrit to snapshot version 3.9.5-22-g7380128525 on gerrit2002 # T358762 (duration: 00m 07s)

Stashbot added a comment.Jun 13 2024, 2:40 PM

Mentioned in SAL (#wikimedia-operations) [2024-06-13T14:40:58Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@89042ad]: Gerrit to snapshot version 3.9.5-22-g7380128525 on gerrit1003 # T358762

Stashbot added a comment.Jun 13 2024, 2:41 PM

Mentioned in SAL (#wikimedia-operations) [2024-06-13T14:41:03Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@89042ad]: Gerrit to snapshot version 3.9.5-22-g7380128525 on gerrit1003 # T358762 (duration: 00m 05s)

hashar closed this task as Resolved.Jun 13 2024, 2:46 PM

That got fixed on the test case https://gerrit.wikimedia.org/r/c/test/gerrit-ping/+/1043020 . The fix was made during the 3.9 release candidate and got erroneously reverted. It will be released again in v3.9.6.

Huge thanks to @Paladox for the backporting/war building etc ;)

Lucas_Werkmeister_WMDE added a comment.Jun 13 2024, 2:54 PM
In T358762#9770312, @Lucas_Werkmeister_WMDE wrote:

Still seems to be happening at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/cldr/+/1027084 :/

image.png (670×1 px, 138 KB)

Also fixed here now \o/

gerritbot added a comment.Jun 19 2024, 9:56 PM

Change #1043020 restored by Hashar:

[test/gerrit-ping@master] Angle bracketed URL in commit message

https://gerrit.wikimedia.org/r/1043020

gerritbot added a comment.Jun 20 2024, 1:20 PM

Change #1043020 abandoned by Hashar:

[test/gerrit-ping@master] Angle bracketed URL in commit message

Reason:

https://gerrit.wikimedia.org/r/1043020

hashar mentioned this in T373897: Upgrade to Gerrit 3.10.2.Sep 3 2024, 4:02 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits