My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 3440: Built-in documentation pages should not load prettify from cdnjs.cloudfront.com
2 people starred this issue and may be notified of changes. Back to list
Status:  Released
Owner:  ----
Closed:  Jun 2015
Cc:  fishyw...@google.com


Sign in to add a comment
 
Reported by tw201...@gmail.com, Jun 18, 2015
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.
Jun 18, 2015
#1 tw201...@gmail.com
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.
Jun 21, 2015
Project Member #2 david.pu...@sonymobile.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.

with.png
73.4 KB   View   Download
without.png
67.2 KB   View   Download
Jun 21, 2015
Project Member #3 david.pu...@sonymobile.com
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

Jun 22, 2015
#4 tw201...@gmail.com
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.
Jun 24, 2015
Project Member #5 david.pu...@sonymobile.com
(No comment was entered for this change.)
Status: Accepted
Cc: fishyw...@google.com
Jun 24, 2015
Project Member #6 fishyw...@google.com
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.
Jun 24, 2015
Project Member #7 fishyw...@google.com
Scratch that, we don't actually use highlight.js.

But other parts stand correct. :)
Jun 25, 2015
Project Member #8 fishyw...@google.com
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.
Jun 25, 2015
#9 tw201...@gmail.com
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.
Jun 25, 2015
#10 dborowitz@google.com
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.
Jun 25, 2015
Project Member #11 david.pu...@sonymobile.com
https://gerrit-review.googlesource.com/#/c/69080
Status: ChangeUnderReview
Jun 25, 2015
#12 tw201...@gmail.com
Thank you, especially also for the cherry-pick to 2.11.
Jun 25, 2015
Project Member #13 david.pu...@sonymobile.com
https://gerrit-review.googlesource.com/69090
Status: Submitted
Labels: FixedIn-2.11.2
Jul 14, 2015
Project Member #14 david.pu...@sonymobile.com
(No comment was entered for this change.)
Status: Released
Sign in to add a comment

Powered by Google Project Hosting