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

Escape file paths when generating URI #107

Merged
merged 1 commit into from
May 5, 2022
Merged

Escape file paths when generating URI #107

merged 1 commit into from
May 5, 2022

Conversation

davishmcclurg
Copy link
Owner

File paths with spaces are causing URI::InvalidURIError errors:

>> URI.parse(File.join('file:', '/path/t o/schema.json'))
/Users/dharsha/.rbenv/versions/3.1.2/lib/ruby/3.1.0/uri/rfc3986_parser.rb:67:in `split': bad URI(is not URI?): "file:/path/t o/schema.json" (URI::InvalidURIError)

I think the correct fix is to escape paths when generating URIs and
unescape when reading files. I had a hard time finding the right
escape/unescape methods, though, since I think we want %20 for spaces
instead of + and to leave slashes unescaped. URI.escape previously
did that but it's been deprecated for a while and removed in Ruby 3.
URI::DEFAULT_PARSER still has the same escape method and it doesn't
emit a warning, so I'm going with it.

Fixes: #104

File paths with spaces are causing `URI::InvalidURIError` errors:

```
>> URI.parse(File.join('file:', '/path/t o/schema.json'))
/Users/dharsha/.rbenv/versions/3.1.2/lib/ruby/3.1.0/uri/rfc3986_parser.rb:67:in `split': bad URI(is not URI?): "file:/path/t o/schema.json" (URI::InvalidURIError)
```

I think the correct fix is to escape paths when generating URIs and
unescape when reading files. I had a hard time finding the right
escape/unescape methods, though, since I think we want `%20` for spaces
instead of `+` and to leave slashes unescaped. `URI.escape` previously
did that but it's been deprecated for a while and removed in Ruby 3.
`URI::DEFAULT_PARSER` still has the same escape method and it doesn't
emit a warning, so I'm going with it.

Fixes: #104
@davishmcclurg davishmcclurg merged commit 9300715 into master May 5, 2022
@davishmcclurg davishmcclurg deleted the uri-escape branch May 5, 2022 17:49
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

Successfully merging this pull request may close these issues.

URI Escaping Error
1 participant