Page MenuHomePhabricator

Upgrade to the latest version of GeSHi (1.0.8)
Closed, ResolvedPublic

Description

Author: 0p9usag02

Description:
The latest version of GeSHi (1.0.7.20) has syntax highlighting for more languages, notably Haskell. This would be useful for the various Haskell wikibooks (en,ru,etc) as well as other programming language books.


Version: unspecified
Severity: minor

Details

Reference
bz10967

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 9:49 PM
bzimport set Reference to bz10967.

Moving this extension into SVN trunk instead of just update the version from the release is considerable, this would be simplify the update process.

http://geshi.svn.sourceforge.net/svnroot/geshi/trunk/geshi-1.0.X/src/

GeShi 1.0.8 also contains support for xhtml-valid pre-mode. GeSHi developer suggested using r1402 from their release branch.

  • Bug 14856 has been marked as a duplicate of this bug. ***

*** Bug 15175 has been marked as a duplicate of this bug. ***

  • Bug 17098 has been marked as a duplicate of this bug. ***

(In reply to comment #1)

Moving this extension into SVN trunk instead of just update the version from
the release is considerable, this would be simplify the update process.

http://geshi.svn.sourceforge.net/svnroot/geshi/trunk/geshi-1.0.X/src/

What about using svn externals? Then people don't have to necessarily checkout/download a separate project when they want the extension.

Added as an external in r46666. Using r1402 as suggested above. Will be sync'd on next scap (pending review).

0p9usag02 wrote:

When is this next scap going to be? I last checked http://en.wikibooks.org/wiki/User:Kowey (which has a sample of Haskell code) on 2009-02-14, but it's still not rendering. Thanks!

A scap was done last night but WMF servers are still using an old GeSHi version: http://test.wikipedia.org/wiki/Haskell

GeSHi could not find the language haskell (using path /home/wikipedia/common/php-1.5/lib/GeSHi-1.0.7.19-wm1/geshi/) (code 2)

herd wrote:

Just a note, if my prediction is correct, on the next upgrade of GeSHi the borders will disappear from all Geshi-generated <pre> with no easy way to re-add it (see bug 16324 for possible solution vector). So possibly get ready for several bugs "syntax hilight has no border".

  • Bug 18553 has been marked as a duplicate of this bug. ***

Note current release is 1.0.8.3.

Note also that SyntaxHighlight_GeSHi currently is pulling in a geshi subdirectory via foreign SVN... need to check which one's being used, and if it needs to be more solidly defined.

I was under the impression that WMF used a different directory than the geshi subdirectory/svn external? If so, is there any compelling reason not to, if we're providing safe external versions?

Per comment #2, the last good rev we were told to use was r1402. This is explicitly set within the external, so we're not running bleeding-edge.

The svn external in the extension is newer than our original deployment, so we just never used it. The include paths present load it before the other one, apparently...

Looks like we've currently got a patched 1.0.7.19 in there... only difference seems to be some regex fixes (I presume for anti-DoS purposes, that's the sort of thing Tim would fix ;) and those changes are present in the release that's checked out.

Do we have a strong reason for grabbing r1402 specifically instead of picking a release tag?

The latest GeSHi version is 1.0.8.4 (r2091 on branch), or we can grabbing the latest 1.0.x release instead of r1402?

BenBE> No. The only reason why 1402 seems to be mentioned seems to be a patch therein; which is applied to all subsequent revision.
BenBE> But as mentioned: they should test for 1.0.8.4 (and its RCs) to work and use this version.
BenBE> 1.0.8.2 is the latest other version without major breakages ...
BenBE> When they test, they should let me know of any defects they find so I can have a look at them.
BenBE> Then they should upgrade to 1.0.8.4 and stick with it ...

That was before 1.0.8.4 was released....

Tim, I'm sticking this on your plate since you're doing the code review & deployment management -- probably we just need to take out our additional GeSHi copy in deployment, and make sure the copy that's pulled in via SVN is updated to a more current release.

herd wrote:

Documenting this here: as of 1.15 <source lang="php">"\<"</source> produces \&lt;&lt;/span&gt; in render with the old GeSHI.

(In reply to comment #14)

Looks like we've currently got a patched 1.0.7.19 in there... only difference
seems to be some regex fixes (I presume for anti-DoS purposes, that's the sort
of thing Tim would fix ;) and those changes are present in the release that's
checked out.

The version of GeSHi we're using was reviewed for security by me. I fixed several regexes that were vulnerable to algorithmic DoS attack and submitted the patches upstream.

So are we clear to update to a more recent release, as it includes those fixes?

Well, it's very likely that they will repeat the same mistakes and reintroduce O(N^2) regexes in new code, since despite my efforts they didn't understand what I was talking about and accepted my patches on trust. But reviewing for that kind of thing is time-consuming, and there are easier DoS attacks available. Their XSS security model is based on input-side escaping, it's ugly but seems to be robust against clueless contributed code. I didn't find any near misses.

I'll just put a profiling section around the parser hook to track any DoS attempts, and do the update.

I'm not at all keen about having SVN externals checked out on Wikimedia, I think we should remove them at least from that working copy, if not from the repo. Any of their developers could commit to the external moments before we do an svn up, whether it's trunk or a stable branch, and it would not show up on MW CodeReview.

As long as the external hits a specific revision (as I believe this one did) it should be reasonably safe, barring failure or malicious attacks on the other repository.

But I do worry it's less robust (what if the other projects' SVN server moves, drops offline, etc?) and it has performance issues -- my svn up's take a lot longer with the externals since it's contacting multiple servers.

(In reply to comment #23)

As long as the external hits a specific revision (as I believe this one did) it
should be reasonably safe, barring failure or malicious attacks on the other
repository.

It does. We're currently pulling r1402 from their repo. Concerns about stability should be pretty ok, as long as we use an external that has been checked out for safety, and only update to later ones once they've been checked as well.

But I do worry it's less robust (what if the other projects' SVN server moves,
drops offline, etc?) and it has performance issues -- my svn up's take a lot
longer with the externals since it's contacting multiple servers.

Yeah :\ There's two externals on trunk/extensions right now, and they do slow down checkout/update a bit. I like the ease of use that having the externals provides, but it is a bottleneck. Isn't there a way to do a svn update without externals (I know you can checkout without them)?

(In reply to comment #24)

Yeah :\ There's two externals on trunk/extensions right now, and they do slow
down checkout/update a bit. I like the ease of use that having the externals
provides, but it is a bottleneck. Isn't there a way to do a svn update without
externals (I know you can checkout without them)?

Yes, use --ignore-externals in your svn up commands.

(In reply to comment #26)

The update created this bug I think:
https://bugzilla.wikimedia.org/show_bug.cgi?id=11274

Reclosing this bug, no need for it to be open. The upgrade happened, and its consequences are separate bugs.

That, and I highly doubt software updated yesterday caused a bug filed 2 years ago ;-)

herd wrote:

(In reply to comment #29)

Well I got the bug ids from here:

http://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#GeSHi_update

Then you didn't read it. The upgrade exposed a previous intrinsic flaw in the logic that was SUPPOSED to remove borders from the <pre> (but didn't), it did not "create" a bug.

This means, that for several years, the old version of GeSHi that Wikimedia has been using has ACCIDENTALLY been leaving the borders on the <pre>, and everyone has gotten used to it. When they were to be removed on upgrade, then people were likely to have complained, and so bug 16324 was opened in anticipation. A workaround for restoring the technically broken but widely seen <pre>-with-borders can be recreated with bug 16324 fixed (a class to give the border to). Bug 11274 is an alternate motivation to make an altenrate change that would have also been a solution vector (but really, bug 16324 was fixed and bug 11274 is either an upstream problem (the extension can not alter the pre classes, only the topmost div), or WORKSFORME.