asciidoctor / asciidoctor.js

:scroll: A JavaScript port of Asciidoctor, a modern implementation of AsciiDoc
https://asciidoctor.org
MIT License
729 stars 135 forks source link

stylesheet attribute prepended with "./" #62

Closed nerk closed 4 years ago

nerk commented 9 years ago

If a URL is passed through the 'stylesheet' attribute, URL in the generated HTML contains an extra './'.

This code (may include syntax errors)

var asciidoctor = require('asciidoctor.js')();
var opal = asciidoctor.Opal;

var  processor = asciidoctor.Asciidoctor(true);

var options = opal.hash2(
    ['doctype', 'header_footer', 'attributes'],
    {'doctype': 'article', 
     'header_footer': true,
     'attributes': 'stylesheet=file:///home/tom/asciidoctor.css'
});
var html = processor.$convert("asciidoctor.js!", options);
console.log(html);

will include the following link:

<link rel="stylesheet" href="./file:///home/tom/asciidoctor.css">

Same results on Windows and Linux.

mojavelinux commented 9 years ago

What would work today is if you set the stylesdir in conjunction with the stylesheet.

-a stylesdir=file:///home/tom -a stylesheet=stylesheet.css

Since setting the stylesdir attribute has side-effects, I see that we need to support both ways of specifying the location.

ggrossetie commented 9 years ago

I believe this issue is now resolved ? @nerk can you please confirm that ?

nerk commented 9 years ago

Just checked, it's resolved.

ggrossetie commented 9 years ago

:+1:

bodtx commented 5 years ago

Hi I'm working to update the thunderbird's asciidoctor plugin. source here.

with asciidoctorjs 1.5.5 I could reference a dedicated stylesheet url

let selection = editor.selection.toString();
var attrs = {
            'nofooter': '',
            'stylesheet': 'asciidoctor-plus.css',
            'stylesdir': 'chrome://asciidoctor_tb/content/',
            'copycss!': '',
            'icons': '',
            'source-highlighter': 'highlight.js'
        };
var options = {
            doctype: 'article',
            safe: 'unsafe',
            header_footer: true,
            attributes: attrs
        };
html = Asciidoctor().convert(selection, options);

but with asciidoctorjs 2.0.3 this leads me to 🛑 IOError: Error reading file or directory: ./chrome://asciidoctor_tb/content/asciidoctor-plus.css; reason: Access to restricted URI denied

@Mogztter I do not see special note in the doc

ggrossetie commented 5 years ago

I think it's because chrome://asciidoctor_tb/content/ is not recognized as an absolute (root) path:

https://github.com/asciidoctor/asciidoctor/blob/bb47c333df0aaab6daefa1f4acd3ba3d2623c0c8/lib/asciidoctor/path_resolver.rb#L165

I'm not sure how/why it was working with Asciidoctor.js 1.5.5... I will add a test to debug it further.

bodtx commented 5 years ago

Is there a workaround actually? Would be great if you could allow special protocol in the path.

ggrossetie commented 5 years ago

We could "patch" the method in Asciidoctor.js to add this special protocol and/or configure a list of authorized protocols... I didn't had the opportunity to debug it but as far as I know there's no workaround at the moment 😓

You could of course create your own converter but I don't think it's the right choice.

mojavelinux commented 5 years ago

If chrome is a recognized, cross-browser URI prefix, then I don't see any reason why the referenced logic shouldn't allow for it. You could possibly think about security concerns, but I'm pretty sure the browser is going to disallow anything it doesn't want to show you, so creating a URI with a forbidden prefix in certain contexts should be benign.

thom4parisot commented 5 years ago

Technically speaking, as I understand it, installed software are able to handle call to non-http schemes, such as atom://, spotify:, etc.

Although I discovered the well-known schemes are registered against a IANA database to be found here: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml (chrome:// is amongst them).

These are the ones which can be considered as "safe".

bodtx commented 5 years ago

As a work around I've directly added the special protocol in the embedded asciidoctor.js source:

return ($truthy($a = self['$absolute_path?'](path)) ? $a : path['$start_with?']("file://", "http://", "https://","chrome://"))

keep looking here.

ggrossetie commented 5 years ago

Although I discovered the well-known schemes are registered against a IANA database to be found here: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml (chrome:// is amongst them).

Thanks for the reference. It's a pretty big list... not sure if we want to add it in Asciidoctor Ruby.

If chrome is a recognized, cross-browser URI prefix, then I don't see any reason why the referenced logic shouldn't allow for it. You could possibly think about security concerns, but I'm pretty sure the browser is going to disallow anything it doesn't want to show you, so creating a URI with a forbidden prefix in certain contexts should be benign.

@mojavelinux What about a regular expression ? If the path starts with [a-z\-0-9]+:\/\/ then it's most likely an absolute path with a protocol right ?

Arguably adding just chrome is "safer" but maybe in some contexts an URI with the git, nfs or sftp protocol makes sense to reference an external ressource.

mojavelinux commented 5 years ago

The best bet is to agree on a method you can override and just move the code into Asciidoctor.js. That gives you complete control for a feature which is only relevant in Asciidoctor.js. There's no reason Asciidoctor.js running the browser needs to have this security feature since the browser sandbox already provides it.

ggrossetie commented 5 years ago

Sounds good, how should we proceed ? Do you want to introduce an higher logic API in Asciidoctor Ruby or should we just override PathResolver#root? in Asciidoctor.js ?

mojavelinux commented 5 years ago

I'd say just override root? as we're already doing (https://github.com/asciidoctor/asciidoctor/blob/35c1f9a8f7c1cee310d6c1d1ad46ec5ae4702d54/lib/asciidoctor/path_resolver.rb#L163-L169) unless you find you need more control. Then we can reevaluate once you've identified that criteria.

ggrossetie commented 4 years ago

Closing in favor of https://github.com/asciidoctor/asciidoctor.js/issues/816