Skip to content

Remove URI.escape and add ruby 3 rexml dependency#55

Open
fusion2004 wants to merge 2 commits intogreatseth:masterfrom
IZEA:remove-uri-escape
Open

Remove URI.escape and add ruby 3 rexml dependency#55
fusion2004 wants to merge 2 commits intogreatseth:masterfrom
IZEA:remove-uri-escape

Conversation

@fusion2004
Copy link

@fusion2004 fusion2004 commented May 25, 2021

Hi! I'm not sure this is how you wanted to handle this, but I noted the TODO in the readme about this, and wanted to get this gem working on ruby 3.0.

I can't seem to even create a URI object with unescaped components, and there didn't seem to be a test for it that broke from this change.

Here are my failed attempts
irb(main):001:0> a = URI("https://example.com/what now/oh boy.html?umm=blep okay")
/Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/rfc3986_parser.rb:67:in `split': bad URI(is not URI?): "https://example.com/what now/oh boy.html?umm=blep okay" (URI::InvalidURIError)
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/rfc3986_parser.rb:72:in `parse'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/common.rb:171:in `parse'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/common.rb:674:in `URI'
	from (irb):1:in `<main>'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/gems/3.0.0/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
	from /Users/markoleson/.rbenv/versions/3.0.1/bin/irb:23:in `load'
	from /Users/markoleson/.rbenv/versions/3.0.1/bin/irb:23:in `<main>'
irb(main):002:0> a = URI("https://example.com/what%20now/oh%20boy.html?umm=blep%20okay")
=> #<URI::HTTPS https://example.com/what%20now/oh%20boy.html?umm=blep%20okay>
irb(main):003:0> a.path = "/what now/oh boy.html"
/Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/generic.rb:760:in `check_path': bad component(expected absolute path component): /what now/oh boy.html (URI::InvalidComponentError)
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/generic.rb:806:in `path='
	from (irb):3:in `<main>'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/gems/3.0.0/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
	from /Users/markoleson/.rbenv/versions/3.0.1/bin/irb:23:in `load'
	from /Users/markoleson/.rbenv/versions/3.0.1/bin/irb:23:in `<main>'
irb(main):004:0> URI::HTTPS.build(host: "example.com", path: "/what about this")
/Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/generic.rb:760:in `check_path': bad component(expected absolute path component): /what about this (URI::InvalidComponentError)
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/generic.rb:806:in `path='
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/generic.rb:192:in `initialize'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/generic.rb:136:in `new'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/generic.rb:136:in `build'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/3.0.0/uri/http.rb:61:in `build'
	from (irb):4:in `<main>'
	from /Users/markoleson/.rbenv/versions/3.0.1/lib/ruby/gems/3.0.0/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
	from /Users/markoleson/.rbenv/versions/3.0.1/bin/irb:23:in `load'
	from /Users/markoleson/.rbenv/versions/3.0.1/bin/irb:23:in `<main>'

Additionally, it seems that the rexml dependency must be declared explicitly to work with ruby 3.

Let me know if you want me to take a different direction with this!

@fusion2004 fusion2004 changed the title Remove URI.escape and add ruby 3 dependency Remove URI.escape and add ruby 3 rexml dependency May 25, 2021
@NorseGaud
Copy link
Collaborator

Hey @fusion2004, I'm unbelievably overwhelmed with some other project. I love what you're trying to do here and wanted to get around to this sooner or later. I should be able to get to the review soon, but can't right now. Thanks for your patience.

@NorseGaud NorseGaud self-requested a review June 3, 2021 02:12
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.

2 participants