sup-heliotrope / sup

A curses threads-with-tags style email client (mailing list: supmua@googlegroups.com)
http://sup-heliotrope.github.io
GNU General Public License v2.0
900 stars 97 forks source link

undefined method `encode' for URI:Module #588

Closed perkun closed 2 years ago

perkun commented 3 years ago

There seems to be a problem with the URI module.

I'm on manjaro linux with ruby 2.7.2p137

content of exception-log.txt:

--- NoMethodError from thread: main
undefined method `encode' for URI:Module
.../.gem/ruby/2.7.0/gems/sup-1.0/lib/sup/maildir.rb:19:in `initialize'
.../.gem/ruby/2.7.0/gems/sup-1.0/lib/sup.rb:32:in `block in yaml_properties'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:413:in `init_with'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:405:in `revive'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:164:in `visit_Psych_Nodes_Mapping'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/visitor.rb:30:in `visit'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/visitor.rb:6:in `accept'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:34:in `accept'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:336:in `block in register_empty'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:336:in `each'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:336:in `register_empty'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:145:in `visit_Psych_Nodes_Sequence'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/visitor.rb:30:in `visit'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/visitor.rb:6:in `accept'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:34:in `accept'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:317:in `visit_Psych_Nodes_Document'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/visitor.rb:30:in `visit'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/visitor.rb:6:in `accept'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/visitors/to_ruby.rb:34:in `accept'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych/nodes/node.rb:50:in `to_ruby'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych.rb:282:in `load'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych.rb:582:in `block in load_file'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych.rb:581:in `open'
.../.gem/ruby/2.7.0/gems/psych-3.3.0/lib/psych.rb:581:in `load_file'
.../.gem/ruby/2.7.0/gems/sup-1.0/lib/sup.rb:145:in `load_yaml_obj'
.../.gem/ruby/2.7.0/gems/sup-1.0/lib/sup/source.rb:227:in `load_sources'
.../.gem/ruby/2.7.0/gems/sup-1.0/lib/sup/util.rb:605:in `method_missing'
.../.gem/ruby/2.7.0/gems/sup-1.0/lib/sup/index.rb:102:in `load'
.../.gem/ruby/2.7.0/gems/sup-1.0/lib/sup/util.rb:605:in `method_missing'
.../.gem/ruby/2.7.0/gems/sup-1.0/bin/sup:146:in `<module:Redwood>'
.../.gem/ruby/2.7.0/gems/sup-1.0/bin/sup:76:in `<top (required)>'
.../.gem/ruby/2.7.0/bin/sup:23:in `load'
.../.gem/ruby/2.7.0/bin/sup:23:in `<main>'
danc86 commented 3 years ago

Thanks for the report. This cropped up in Fedora after the switch from Ruby 2.7.2 to 3.0.0. The URI.encode method had been deprecated and I guess it was finally removed. Some useful looking hints here: https://docs.knapsackpro.com/2020/uri-escape-is-obsolete-percent-encoding-your-query-string

mtasaka commented 3 years ago

Using WEBrick::HTTPUtils.#escape seems one of the alternative choice.

danc86 commented 3 years ago

Frustratingly, the Ruby standard library nowadays only has URI.encode_www_form_component, which encodes query string values (space becomes +). There is also ERB::Util.url_encode which is closer, it encodes arbitrary URI components but it doesn't accept a set of characters to leave alone.

What we actually need is slightly unusual: the code wants to accept maildir and mbox source URIs where the user has put URI-incompatible characters in the path and fix it up for them. So:
maildir:///home/me/My Maildir becomes
maildir:///home/me/My%20Maildir

ERB::Util.url_encode won't work for that, because it escapes / as %2F.

Some of this code for handling source URIs is a bit of a mess, with paths flying around sometimes percent-encoded and sometimes not percent-encoded. There are even comments complaining about how messy it is... But anyway, in the interest of preserving the existing behaviour I think the best option is to replace URI.encode with our own helper function using gsub which obeys the specific set of characters we want to escape. And URI.decode can be replaced with URI.decode_www_form_component, since we already encode + to %2B.

po1vo commented 3 years ago

There is a related error in sup-add:

/home/plv/.local/share/gem/ruby/3.0.0/gems/sup-1.0/bin/sup-add:97:in `block in <top (required)>': undefined method `escape' for URI:Module (NoMethodError)
        from /home/plv/.local/share/gem/ruby/3.0.0/gems/sup-1.0/bin/sup-add:89:in `each'
        from /home/plv/.local/share/gem/ruby/3.0.0/gems/sup-1.0/bin/sup-add:89:in `<top (required)>'
        from /home/plv/.local/share/gem/ruby/3.0.0/bin/sup-add:23:in `load'
        from /home/plv/.local/share/gem/ruby/3.0.0/bin/sup-add:23:in `<main>'
danc86 commented 2 years ago

Thanks, I had missed the one in sup-add because there's no tests for it.

I've amended PR #593 to also fix the sup-add crash, and will add some unit test coverage for it as well (need to fix #577 first though).