Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
danyill opened this issue Nov 3, 2021 · 4 comments
Open

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

danyill opened this issue Nov 3, 2021 · 4 comments
Labels
🐛 bug 🙃 upstream

Comments

@danyill
Copy link

danyill commented Nov 3, 2021

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
Copy link
Member

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
Copy link
Member

ggrossetie commented Nov 5, 2021 •
edited
Loading

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: opal/opal#2348

@ggrossetie
Copy link
Member

Fix available in Opal 1.4.0

@ggrossetie
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐛 bug 🙃 upstream
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mojavelinux @ggrossetie @danyill