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

WEBrick::HTTPUtils.escape(nil) raises a confusing exception #96

Open
kyoshidajp opened this issue Sep 9, 2022 · 1 comment
Open

WEBrick::HTTPUtils.escape(nil) raises a confusing exception #96

kyoshidajp opened this issue Sep 9, 2022 · 1 comment

Comments

@kyoshidajp
Copy link

% irb
> RUBY_VERSION
=> "3.1.2"

> require 'webrick'
> WEBrick::HTTPUtils.escape(nil)
/Users/katsuhiko.yoshida/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/webrick-1.7.0/lib/webrick/httputils.rb:444:in `_escape': undefined method `b' for nil:NilClass (NoMethodError)

      str = str.b
               ^^

(omit)
%irb
> RUBY_VERSION
=> "2.7.6"

> WEBrick::HTTPUtils.escape(nil)
Traceback (most recent call last):
        7: from /Users/katsuhiko.yoshida/.rbenv/versions/2.7.6/bin/irb:23:in `<main>'
        6: from /Users/katsuhiko.yoshida/.rbenv/versions/2.7.6/bin/irb:23:in `load'
        5: from /Users/katsuhiko.yoshida/.rbenv/versions/2.7.6/lib/ruby/gems/2.7.0/gems/irb-1.2.6/exe/irb:11:in `<top (required)>'
        4: from (irb):2
        3: from (irb):3:in `rescue in irb_binding'
        2: from /Users/katsuhiko.yoshida/.rbenv/versions/2.7.6/lib/ruby/2.7.0/webrick/httputils.rb:467:in `escape'
        1: from /Users/katsuhiko.yoshida/.rbenv/versions/2.7.6/lib/ruby/2.7.0/webrick/httputils.rb:443:in `_escape'
NoMethodError (undefined method `b' for nil:NilClass)

WEBrick::HTTPUtils.escape_form(nil) is also the same. If the exception was raised in complex situations (e.g. in third party library), it will be difficult to determine the cause (parameter is nil).

My suggestions are following.

  1. Raises a builtin exception
    • CGI.escape is this behavior (raises a TypeError)
  2. Raises a custom exception
    • Like a EscapeError
  3. Returns ""
    • URI.encode_www_form_component and ERB::Util.url_encode are this behavior
    • But, BREAKING CHANGE (doesn't stop)

Thank you for your great work.

@tisba
Copy link

tisba commented Nov 26, 2024

Indeed this is confusing and not nice to handle for 3rd party code.

re 1) AFAIK CGI.escape raising a TypeError (with no implicit conversion of nil into String) is not necessarily meant as input validation check, it just fails as part of its internal workings.

re 3) While just retuning nil or "" could be something for a new API, I think that might be a bad idea for WEBrick today.

I personally would suggest raising an ArgumentError in this case.

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

No branches or pull requests

2 participants