New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users

Issue metadata

Status: Released
Owner: ----
Closed: Jun 2015
Cc:



Sign in to add a comment

Built-in documentation pages should not load prettify from cdnjs.cloudfront.com

Reported by tw201...@gmail.com, Jun 18 2015

Issue description

Gerrit's built-in documentation pages at <GERRIT_URL>/Documentation/*.html load prettify.min.css and prettify.min.js from cdnjs.cloudflare.com.

Apart from the fact that I don't quite see what these are used for, they could also simply be included in /Documentation and loaded from there instead of reaching out to cloudflare.com, which may cause trouble if the Gerrit instance is behind a firewall on a machine not allowed to access the Internet at large.

In my case, I have to patch the gerrit.war on every update:

jar xf gerrit.war Documentation
cd Documentation
cp ~/kits/prettify.min.* .
find . -name "*.html" -exec sed -i -r 's/https:\/\/cdnjs.cloudflare.com\/ajax\/libs\/prettify\/r[0-9]+\///' {} \;
cd ..
jar uf gerrit.war Documentation
rm -rf Documentation

Would be really great if Gerrit had its own copy of prettify (if it's really needed at all) and load that included prettify instead of going to cloudflare.
 

Comment 1 by tw201...@gmail.com, Jun 18 2015

Of course, it's not the fact that the Gerrit instance is behind that firewall, but that the users' machines running the browser accessing the Gerrit UI also are, and also cannot access cloudflare.com.
Prettify is used for code syntax highlighting in the documentation.

See the attached screenshots to see what the documentation looks like with and without prettify.

without.png
67.2 KB View Download
with.png
73.4 KB View Download
Gerrit does have its own copy of prettify, used by the unifed diff view.  However, it looks like the cloudflare.com link is hardcoded in asciidoctor:

https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/converter/html5.rb#L36

https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/converter/html5.rb#L106

Comment 4 by tw201...@gmail.com, Jun 22 2015

As far as I understand the code (I'm no ruby expert), the hardcoded cdn_base is just the default fallback. It should be possible to override this by setting the attribute 'prettifydir'.

Something like 

  asciidoctor -a prettifydir=. <other options>

assuming prettify was in the Documentation directory.
Cc: fishywang@google.com
Status: Accepted
One concern is that if we set prettifydir, then we'd better maintain the same version of prettify as asciidoctor uses, otherwise some unexpected thing might happen.

There's also highlight.js hosted on cloudflare used by asciidoctor, and we don't have it inside gerrit already. See:

https://github.com/asciidoctor/asciidoctor/blob/v1.5.0/lib/asciidoctor/converter/html5.rb#L99

If you can live without prettify and highlight.js, maybe it's easier to just block cloudflare quicker (e.g. resolve to 127.0.0.1) on your firewall?

Or if you are willing to build gerrit locally, you can make a easy patch on Documentation/config.def to set prettifydir and highlightjsdir. You can have that patch on your local branch, and everytime when gerrit updates, you just rebase to the upstream tag before building, so you local patch will always be there.
Scratch that, we don't actually use highlight.js.

But other parts stand correct. :)
Also to clarify, we don't actually currently have prettify js and css files in our war package. The only thing we have is a prettify jar package, and some class files inside it. So we can't just use that for our Documentations.

Comment 9 by tw201...@gmail.com, Jun 25 2015

1. Using the same version as asciidoctor does: surely it's possible to do a "curl -O https://cdnjs..." with the right prettify.min.js and prettify.min.css URLs in the buck build of the documentation? I the very worst case, if you can't figure out those URLs up front, generate the documentation as is now, but add a postprocess step that extracts the URLs from Documentation/index.html, then downloads the two files, and then does the "find ... | sed" command I've given in the OP. (My patch sequence doesn't do that because--guess what?--cloudflare is inaccessible in the environment I have to work in.)

2. Blocking cloudflare myself: I have no control over these firewalls. Corporate setup at a customer location that won't change because of Gerrit. I have no control over DNS resolution either. And I can't really expect all users to map cdnjs.couldflare.com to localhost in /etc/hosts or whatever equivalent exists on Windows. The users on Linux don't have write-access to /etc/hosts anyway.

3. No, I'm not willing to build Gerrit myself. Besides, doing this song and dance that you propose with having my own local branch and rebasing and then rebuilding myself is *way* more complicated than the patch sequence I've given in the OP.
Project Member

Comment 10 by dborowitz@google.com, Jun 25 2015

IMHO we don't need to use the same version of prettify as asciidoctor does. The library is old, stable, and has a very very minimal interface.
Status: ChangeUnderReview
https://gerrit-review.googlesource.com/#/c/69080

Comment 12 by tw201...@gmail.com, Jun 26 2015

Thank you, especially also for the cherry-pick to 2.11.
Labels: FixedIn-2.11.2
Status: Submitted
https://gerrit-review.googlesource.com/69090
Status: Released

Sign in to add a comment