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

[GoogleCodeExporter]

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.

Original issue reported on code.google.com by tw201...@gmail.com on 18 Jun 2015 at 8:12

[GoogleCodeExporter]

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.

Original comment by tw201...@gmail.com on 18 Jun 2015 at 8:38

[GoogleCodeExporter]

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.

Original comment by david.pu...@sonymobile.com on 22 Jun 2015 at 6:42

Attachments:

• with.png
• without.png

[GoogleCodeExporter]

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

Original comment by david.pu...@sonymobile.com on 22 Jun 2015 at 6:51

[GoogleCodeExporter]

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.

Original comment by tw201...@gmail.com on 22 Jun 2015 at 7:56

[GoogleCodeExporter]

Original comment by david.pu...@sonymobile.com on 25 Jun 2015 at 2:44

• Changed state: Accepted

[GoogleCodeExporter]

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.

Original comment by fishyw...@google.com on 25 Jun 2015 at 4:31

[GoogleCodeExporter]

Scratch that, we don't actually use highlight.js.

But other parts stand correct. :)

Original comment by fishyw...@google.com on 25 Jun 2015 at 4:48

[GoogleCodeExporter]

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.

Original comment by fishyw...@google.com on 25 Jun 2015 at 6:22

[GoogleCodeExporter]

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.

Original comment by tw201...@gmail.com on 25 Jun 2015 at 8:41

[GoogleCodeExporter]

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.

Original comment by dborowitz@google.com on 25 Jun 2015 at 8:46

[GoogleCodeExporter]

https://gerrit-review.googlesource.com/#/c/69080

Original comment by david.pu...@sonymobile.com on 26 Jun 2015 at 12:34

• Changed state: ChangeUnderReview

[GoogleCodeExporter]

Thank you, especially also for the cherry-pick to 2.11.

Original comment by tw201...@gmail.com on 26 Jun 2015 at 5:12

[GoogleCodeExporter]

https://gerrit-review.googlesource.com/69090

Original comment by david.pu...@sonymobile.com on 26 Jun 2015 at 5:39

• Changed state: Submitted
• Added labels: FixedIn-2.11.2