Last modified: 2014-11-16 16:12:22 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T75160, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 73160 - Phabricator links in commit messages mangled if linking to defunct Labs test instance
Phabricator links in commit messages mangled if linking to defunct Labs test ...
Status: NEW
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
wmf-deployment
All All
: Lowest minor (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-11-07 23:22 UTC by Rainer Rillke @commons.wikimedia
Modified: 2014-11-16 16:12 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Rainer Rillke @commons.wikimedia 2014-11-07 23:22:32 UTC
* Raw commit message: https://gerrit.wikimedia.org/r/#/c/135756/13//COMMIT_MSG
** http://fab.wmflabs.org/T352
* Parsed commit message: https://gerrit.wikimedia.org/r/#/c/135756
** <a href="http://fab.wmflabs.org/<a href=" https:="" phabricator.wikimedia.org="" t352"="">T352</a>" target="_blank"&gt;http://fab.wmflabs.org/<a href="https://phabricator.wikimedia.org/T352">T352</a>
Comment 1 Rainer Rillke @commons.wikimedia 2014-11-07 23:25:02 UTC
It's a link to the test instance, so probably no one needs this in future. Still, the malformed HTML should not show up.
Comment 2 Bryan Davis 2014-11-07 23:27:46 UTC
We added a linking for /T\d+/ or something similar. Since it's using a regex and not a parser it doesn't notice that the task reference is in a quoted string.
Comment 3 Chad H. 2014-11-08 00:17:16 UTC
There's probably a dupe of this bug.(In reply to Bryan Davis from comment #2)
> We added a linking for /T\d+/ or something similar. Since it's using a regex
> and not a parser it doesn't notice that the task reference is in a quoted
> string.

Far too simple of a regex :)
Comment 4 christian 2014-11-16 16:12:22 UTC
This request sounds so harmless, but we're hitting some edge-cases here:

* Gerrit's Issue Tracker framework relies on the first group being the
  id of the issue tracker.

  So we cannot do something in the spirit of

    ([^a-zA-Z0-9/])T(\d+)

  as then, Gerrit would take the ([^a-zA-Z0-9/]) as task number.


* Gerrit's commentlinks cannot do non-capturing groups.

  So we cannot do something in the spirit of

    (?:[^a-zA-Z0-9/])T(\d+)

  .

* Gerrit's commentlinks cannot do lookbehinds

  So we cannot do something in the spirit of

    (?<![a-zA-Z0-9/])T(\d+)

  .


Since the Gerrit's commentlinks code is doing weird things to make
sure it is safe html, I guess it's easiest thing would be to patch Gerrit's
Issue Tracker framework to allow to specify which group number is the
issue tracker id.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links