asciidoctor / asciidoctor.js

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

With data-uri set UNC paths do not resolve correctly #1462

Open danyill opened 2 years ago

danyill commented 2 years ago

I'm trying to diagnose an issue with path resolution if data-uri is set for network shares and UNC paths.

I provide a base_dir to Asciidoctor.js of: \\wnfs1\groups\prot_pai\_112_Files\8 Projects\KMO-TMI-OWH-TRK-protection-replacements-2021-2023\Vivian\Adoc

I receive error messages along the lines of:

{
    "resource": "/groups/prot_pai/_112_Files/8 Projects/KMO-TMI-OWH-TRK-protection-replacements-2021-2023/Vivian/Adoc/HAM_BZ_Coupler_Feeder_SS_15sep21-Stuart_comments.adoc",
    "owner": "asciidoc",
    "severity": 4,
    "message": "<stdin>: stylesheet does not exist or cannot be read: /wnfs1/groups/prot_pai/_112_Files/8 Projects/KMO-TMI-OWH-TRK-protection-replacements-2021-2023/Vivian/Adoc/assets/asciidoctor.css",
    "source": "asciidoctor.js",
    "startLineNumber": 1,
    "startColumn": 1,
    "endLineNumber": 1,
    "endColumn": 31
}

{
    "resource": "/groups/prot_pai/_112_Files/8 Projects/KMO-TMI-OWH-TRK-protection-replacements-2021-2023/Vivian/Adoc/HAM_BZ_Coupler_Feeder_SS_15sep21-Stuart_comments.adoc",
    "owner": "asciidoc",
    "severity": 4,
    "message": "image to embed not found or not readable: /wnfs1/groups/prot_pai/_112_Files/8 Projects/KMO-TMI-OWH-TRK-protection-replacements-2021-2023/Vivian/Adoc/SVG/HAM VT selection.png",
    "source": "asciidoctor.js",
    "startLineNumber": 1,
    "startColumn": 1,
    "endLineNumber": 1,
    "endColumn": 31
}

What seems to be important is that it is reporting the path as having a single leading slash.

This occurs when normalize_system_path is called which calls below: start = $$$('::', 'File').$join(doc.$base_dir(), start)

      Opal.def(self, '$normalize_system_path', $AbstractNode_normalize_system_path$32 = function $$normalize_system_path(target, start, jail, opts) {
        var $a, self = this, doc = nil;

        if (start == null) {
          start = nil;
        };

        if (jail == null) {
          jail = nil;
        };

        if (opts == null) {
          opts = $hash2([], {});
        };
        if ($truthy($rb_lt((doc = self.document).$safe(), $$$($$($nesting, 'SafeMode'), 'SAFE')))) {
          if ($truthy(start)) {
            if ($truthy(doc.$path_resolver()['$root?'](start))) {
            } else {
              start = $$$('::', 'File').$join(doc.$base_dir(), start)
            }
          } else {
            start = doc.$base_dir()
          }
        } else {

$join removes one of the leading / at the start of the filename.

Here are some outputs:


$$$('::', 'File').$join(doc.$base_dir(), start) = '/wnfs1/groups/prot_pai/_112_Files/8 Projects/KMO-TMI-OWH-TRK-protection-replacements-2021-2023/Vivian/Adoc/SVG'

doc.$base_dir() = '//wnfs1/groups/prot_pai/_112_Files/8 Projects/KMO-TMI-OWH-TRK-protection-replacements-2021-2023/Vivian/Adoc'

start = 'SVG'

$join is defined as:

    Opal.defs(self, '$join', $File_join$5 = function $$join($a) {
      var $post_args, paths, self = this;

      $post_args = Opal.slice.call(arguments, 0, arguments.length);

      paths = $post_args;;
      return __path__.posix.join.apply(__path__, paths);
    }

This assumes the path is a posix path and removes the leading /:

__path__.posix.join.apply(__path__, ['//a','b'])
'/a/b'

This is destructive of UNC paths which is why Asciidoctor cannot resolve the image (and other resources) when data-uri is set.

We discussed this on Zulip and Dan helped me identify that this was part of Opal/Asciidoctor.js and there were tests for this situation in Asciidoctor Ruby.

mojavelinux commented 2 years ago

I think this fix needs to happen in Opal. File.$join needs to detect // at the start of the path and restore it after running path.join. By itself, path.join (or path.posix.join) normalizes the leading // to /.

ggrossetie commented 2 years ago

The "default" implementation in Opal is actually doing a better job (on this particular case): https://github.com/opal/opal/blob/6ad0f920e49df8e3f280286f01b2f0993f47d759/opal/corelib/file.rb#L173-L204

$ opal -e "puts File.join('//a', 'b')" 
//a/b
$ opal --require nodejs -e "puts File.join('//a', 'b')"
/a/b

Upstream issue: https://github.com/opal/opal/issues/2348

ggrossetie commented 2 years ago

Fix available in Opal 1.4.0

ggrossetie commented 2 years ago

Actually this is not enough. @mojavelinux I think we should also update absolute_path? to return true when the path starts with \\ or // (normalized):

https://github.com/asciidoctor/asciidoctor/blob/afa970836219b5655b63dc5b36aef7b696462665/lib/asciidoctor/path_resolver.rb#L145-L147