Links with hex-encoded spaces in the path aren't processed correctly

stevecopley commented 2 years ago

Describe the bug

Links with hex-encoded spaces (%20) don't get processed correctly

Steps to reproduce the behavior

In a situation where you have files / directories with spaces in their names, e.g.

├── index.md
└── sub folder
        └── file with spaces.md

A link such as this...

[Link Name](sub%20folder/file%20with%20spaces.md)

... doesn't get processed correctly - I'm guessing because the parsing of the link URL doesn't match the .md file name with the hex-encoded spaces (%20)

(Note: I came across this as I'm trying to get a collection of MD files from Obsidian into GitHub pages. Yes, I can go through my entire vault and rename every file and link to have no spaces, but I'd rather not!)

Expected behaviour

A link such as this...

[Link Name](sub%20folder/file%20with%20spaces.md)

... should get processed to this...

[Link Name](sub%20folder/file%20with%20spaces.html)

benbalter commented 2 years ago

@stevecopley thanks for opening the issue. I suspect this is an issue with our link-detection regex. If you are able to submit a pull request, I'd be happy to review.

stevecopley commented 2 years ago

Hi @benbalter

I'm no Ruby programmer. I've had a quick go at installing Ruby, robocop, etc. but I'm flailing about in the dark, TBH!

Looking over the code, I don't think the issue is with the regex. Somewhere along the link, the MD links are decoded and compared against the relative links of the potential targets (given by Jekyll) and the comparison is failing.

Here is where I thought the issue was...

def url_for_path(path)
      path = CGI.unescape(path)
      target = potential_targets.find { |p| p.relative_path.sub(%r!\A/!, "") == path }
      relative_url(target.url) if target&.url
end

But CGI.unescape(...) should convert those %20 to spaces (I ran a little test for this in an online Ruby playground) so the resulting path should match the target relative path (unless the paths given by Jekyll for the pages have some sort on encoding in)

Sorry... Would love to have a solution and be able to submit a pull request, but without having a test environment setup, I'm stuck.

benbalter commented 2 years ago

@stevecopley I pushed up what should be a failing test for this scenario in https://github.com/benbalter/jekyll-relative-links/compare/spaces-test, but oddly it's passing. Do you have any idea how the test differs from what you're experiencing?

stevecopley commented 2 years ago

@benbalter

Here is a repo with some test links in that illustrate the issue: https://github.com/stevecopley/jekyll-relative-link-test

There's a clue in the rendered HTML version of the index.md page...

The code snippets I added to the MD to show the syntax of each link have also been processed by Jekyll (not sure any code snippets should be processed, but that's another matter!) and this is the outcome...

Link 2, the one with the spaces in the MD link, has been processed to become an encoded HTML link
Link 3, the one with an encoded MD file name, has not been processed to an HTML link

stevecopley commented 2 years ago

@stevecopley I pushed up what should be a failing test for this scenario in https://github.com/benbalter/jekyll-relative-links/compare/spaces-test, but oddly it's passing. Do you have any idea how the test differs from what you're experiencing?

No... That test should pick up the issue! I'm not sure how that passes, yet my demo repo doesn't work.

Could it be some interaction with another Jekyll plugin used by GH Pages?

necopinus commented 1 year ago

I also recently ran into this issue, and have confirmed that it occurs with vanilla Jekyll 4.3.1 and no plugins besides jekyll-relative-links on an M1 Mac running macOS 13.1 + Homebrew.

Repository used for the build (including HTML): necopinus / jekyll-relative-links-spaces-test

Build command:

rm -rf .bundle .jekyll-cache vendor _site
bundle config set path vendor/bundle
bundle install
bundle exec jekyll build --profile --verbose --trace

Example output (some directory names obfuscated):

Fetching gem metadata from https://rubygems.org/...........
Using bundler 2.3.26
Fetching colorator 1.1.0
Fetching concurrent-ruby 1.1.10
Fetching http_parser.rb 0.8.0
Fetching ffi 1.15.5
Fetching rb-fsevent 0.11.2
Fetching public_suffix 5.0.1
Fetching eventmachine 1.2.7
Fetching forwardable-extended 2.6.0
Installing colorator 1.1.0
Fetching rexml 3.2.5
Installing rb-fsevent 0.11.2
Fetching liquid 4.0.3
Installing forwardable-extended 2.6.0
Fetching mercenary 0.4.0
Installing http_parser.rb 0.8.0 with native extensions
Installing concurrent-ruby 1.1.10
Fetching rouge 4.0.1
Installing public_suffix 5.0.1
Fetching safe_yaml 1.0.5
Installing mercenary 0.4.0
Fetching unicode-display_width 2.3.0
Fetching webrick 1.7.0
Installing ffi 1.15.5 with native extensions
Installing rexml 3.2.5
Fetching pathutil 0.16.2
Installing eventmachine 1.2.7 with native extensions
Installing unicode-display_width 2.3.0
Fetching i18n 1.12.0
Installing liquid 4.0.3
Fetching addressable 2.8.1
Installing safe_yaml 1.0.5
Fetching kramdown 2.4.0
Installing pathutil 0.16.2
Fetching terminal-table 3.0.2
Installing webrick 1.7.0
Installing i18n 1.12.0
Installing rouge 4.0.1
Installing terminal-table 3.0.2
Installing addressable 2.8.1
Installing kramdown 2.4.0
Fetching kramdown-parser-gfm 1.1.0
Fetching sassc 2.4.0
Fetching rb-inotify 0.10.1
Installing kramdown-parser-gfm 1.1.0
Installing rb-inotify 0.10.1
Fetching listen 3.7.1
Installing listen 3.7.1
Fetching jekyll-watch 2.2.1
Installing sassc 2.4.0 with native extensions
Fetching em-websocket 0.5.3
Installing jekyll-watch 2.2.1
Installing em-websocket 0.5.3
Fetching jekyll-sass-converter 2.2.0
Installing jekyll-sass-converter 2.2.0
Fetching jekyll 4.3.1
Installing jekyll 4.3.1
Fetching jekyll-relative-links 0.6.1
Installing jekyll-relative-links 0.6.1
Bundle complete! 2 Gemfile dependencies, 30 gems now installed.
Bundled gems are installed into `./vendor/bundle`
bundle install  30.18s user 5.44s system 84% cpu 41.926 total
  Logging at level: debug
    Jekyll Version: 4.3.1
Configuration file: /Users/████████████████████/jekyll-relative-links-spaces-test/_config.yml
         Requiring: jekyll-relative-links
            Source: /Users/████████████████████/jekyll-relative-links-spaces-test
       Destination: /Users/████████████████████/jekyll-relative-links-spaces-test/_site
 Incremental build: disabled. Enable with --incremental
      Generating... 
       EntryFilter: excluded /Gemfile
       EntryFilter: excluded /Gemfile.lock
       EntryFilter: excluded /.jekyll-cache
       EntryFilter: excluded /vendor/bundle
           Reading: Test Page.md
           Reading: index.md
        Generating: JekyllRelativeLinks::Generator finished in 0.001876 seconds.
         Rendering: Test Page.md
  Pre-Render Hooks: Test Page.md
  Rendering Markup: Test Page.md
Post-Convert Hooks: Test Page.md
  Rendering Layout: Test Page.md
         Rendering: index.md
  Pre-Render Hooks: index.md
  Rendering Markup: index.md
Post-Convert Hooks: index.md
  Rendering Layout: index.md
           Writing: /Users/████████████████████/jekyll-relative-links-spaces-test/_site/Test Page.html
           Writing: /Users/████████████████████/jekyll-relative-links-spaces-test/_site/index.html

Build Process Summary: 

| PHASE      |   TIME |
+------------+--------+
| RESET      | 0.0000 |
| READ       | 0.0078 |
| GENERATE   | 0.0019 |
| RENDER     | 0.1301 |
| CLEANUP    | 0.0005 |
| WRITE      | 0.0003 |
| TOTAL TIME | 0.1406 |

Site Render Stats: 

| Filename            | Count | Bytes |  Time |
+---------------------+-------+-------+-------+
| TOTAL (for 0 files) |     0 | 0.00K | 0.000 |

                    done in 0.147 seconds.
 Auto-regeneration: disabled. Use --watch to enable.

benbalter / jekyll-relative-links