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.

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

Path with spaces throws a bad URI(is not URI?) error #687

Open
marcamillion opened this issue Jun 30, 2020 · 3 comments
Open

Path with spaces throws a bad URI(is not URI?) error #687

marcamillion opened this issue Jun 30, 2020 · 3 comments

Comments

@marcamillion
Copy link

marcamillion commented Jun 30, 2020

Expected behavior

When a Rails or Sprockets project is placed in a directory with spaces in the folder names in the path, it shouldn't throw any errors. It should just escape the path name and work like normal.

Actual behavior

Once you put your project in a folder that has spaces in the path name, it throws the following error:

bad URI(is not URI?): "file-digest:///Users/username/Company Name Dropbox/User Name/ProjectName/myapp/app/assets/config/bootstrap"`

System configuration

  • Sprockets 4.0.2 (also been able to replicate it in 3.7.2)
  • Ruby 2.7.1 (also been able to replicate it in 2.5.1)
  • Rails 6.0.3.2 (also been able to replicate it in 5.2.4.3)

Example App (Reproduction)

I have successfully created a failing test on a fork of Sprockets. You can simply pull the following branch, run bundle exec rake test and you will see the failing test.

This is what you should see:

Finished in 46.357470s, 19.4143 runs/s, 84.9701 assertions/s.

  1) Error:
TestURIUtils#test_split_file_uri:
URI::InvalidURIError: bad URI(is not URI?): "file:///usr/Company Name Dropbox/local/bin/myapp/assets"
    /.rvm/rubies/ruby-2.7.1/lib/ruby/2.7.0/uri/rfc3986_parser.rb:67:in `split'
    /.rvm/rubies/ruby-2.7.1/lib/ruby/2.7.0/uri/common.rb:197:in `split'
    /sprockets/lib/sprockets/uri_utils.rb:49:in `split_file_uri'
    /sprockets/test/test_uri_utils.rb:39:in `test_split_file_uri'

900 runs, 3939 assertions, 0 failures, 1 errors, 4 skips

You have skipped tests. Run with --verbose for details.
rake aborted!
Command failed with status (1)
/.rvm/gems/ruby-2.7.1@myapp/gems/rake-12.3.3/exe/rake:27:in `<top (required)>'
/.rvm/gems/ruby-2.7.1@myapp/bin/ruby_executable_hooks:24:in `eval'
/.rvm/gems/ruby-2.7.1@myapp/bin/ruby_executable_hooks:24:in `<main>'
Tasks: TOP => test
(See full trace by running task with --trace)

Steps Taken To Begin Fixing

I have isolated the problem to the following method:

    def split_file_uri(uri)
      # We need to parse out any potential spaces in the path
      scheme, _, host, _, _, path, _, query, _ = URI.split(uri)

      path = URI::Generic::DEFAULT_PARSER.unescape(path)
      path.force_encoding(Encoding::UTF_8)

      # Hack for parsing Windows "/C:/Users/IEUser" paths
      if File::ALT_SEPARATOR && path[2] == ':'
        path = path[1..-1]
      end

      [scheme, host, path, query]
    end

One solution I have tried is to essentially escape uri at the beginning of the method, so I inserted this line at the top of that method:

      uri = URI::Generic::DEFAULT_PARSER.escape(uri)

The issue this creates is that it escapes paths that were previously escaped, e.g. the following test from test/test_uri_utils.rb has begun to fail with that fix:

    parts = split_file_uri("file:///usr/local/bin/ruby%20on%20rails")
    assert_equal ['file', nil, '/usr/local/bin/ruby on rails', nil], parts

Primarily because it returns the original string as a URI, i.e. file:///usr/local/bin/ruby%20on%20rails, as opposed to the unescaped version.

The reason this is the case is because the first thing it does is now escapes the previously escaped string and then unescapes that double escaped string, which reverts it to the original string.

There are also other Failures, Errors & Skips that this small change has created.

So rather than digging much further, I figured I would reach out for some help.

@schneems
Copy link
Member

Hello! Thanks for opening up an issue. In order to move forwards with this issue I'll need an example app that reproduces the behavior (https://www.codetriage.com/example_app).

@iainad
Copy link

iainad commented Dec 19, 2021

For anyone else who ends up here via Google I've got a workaround for now.

This is a frustrating problem if you migrate code stored in a Dropbox to Dropbox business where it suddenly moves all files to a path that has spaces and you can't rename the files to exclude spaces. Nice one Dropbox - there's a thread about this total failure to consider customer use cases here

Here's a simple but most likely not very nice hack to fix it if you've got a rails project

  1. Fork sprockets in your own github account
  2. On your fork of sprockets, in the master branch (or whichever branch you need - e.g. 3.x) make the following change to the first line of the split_file_uri method in uri_utils.rb (this simply replaces any spaces [and only spaces] with an escape sequence avoiding the issue of re-encoding previously encoded urls)
scheme, _, host, _, _, path, _, query, _ = URI.split(uri.gsub(' ', '%20'))
  1. In your Gemfile change the source for sprockets to point to your newly patched version (including the branch if required):
gem 'sprockets', git: 'https://github.com/github-username/sprockets.git', branch: '3.x'

Not ideal but at least a way of making it work until there's a proper fix in sprockets for when your project is contained within a folder that has had spaces added in the pathname and you can't rename it because Dropbox thinks it's ok to have a path with a space in it if you suddenly pay them more than you used to. Go figure.

@2called-chaos
Copy link

This should be a general discussion about urlencoding path segments. For "reasons" I'm stuck with images that have + in the filename. My dirty hack for now is to image_tag(image_url(...).gsub("+", "%2B")) which is frankly not ideal

@schneems

You can reproduce this by generating any old rails project, create those 3 pngs and compare expectation to result.

<% ["test1.png", "te st2.png", "te+st3.png"].each do |img| %>
  <%= "valid" if URI.parse(image_url(img)) rescue "invalid" %><br>
  <%= image_tag(img) %><br>
<% end -%>

Whilst 1 and 2 will be displayed, 2 is not a valid URL and 3 won't display whilst being valid (+ essentially the same as %20 or a space)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants