Project

General

Profile

Misc #16417

Mark WEBrick::HTTPUtils.escape as obsolete in line with URI.escape/encode deprecation

Added by mjrbrennan (Martin Brennan) 10 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
[ruby-core:96210]

Description

URI.escape has been deprecated for some time now with a warning. This calls DEFAULT_PARSER.escape which is RFC2396_Parser.escape. At Discourse we have just done some cleanup to remove usage of URI.escape and noticed that WEBrick::HTTPUtils.escape is still not marked as deprecated with a warning, though it has a very similar implementation to URI.escape. Consider the two implementations:

URI.escape (via RFC2396_Parser)

# URI
def escape(*arg)
  warn "URI.escape is obsolete", uplevel: 1
  DEFAULT_PARSER.escape(*arg)
end

# DEFAULT_PARSER
def escape(str, unsafe = @regexp[:UNSAFE])
  unless unsafe.kind_of?(Regexp)
    # perhaps unsafe is String object
    unsafe = Regexp.new("[#{Regexp.quote(unsafe)}]", false)
  end
  str.gsub(unsafe) do
    us = $&
    tmp = ''
    us.each_byte do |uc|
      tmp << sprintf('%%%02X', uc)
    end
    tmp
  end.force_encoding(Encoding::US_ASCII)
end

WEBrick::HTTPUtils.escape

def escape(str)
  _escape(str, UNESCAPED)
end

_escape(str, regex)
  str = str.b
  str.gsub!(regex) {"%%%02X" % $1.ord}
  # %-escaped string should contain US-ASCII only
  str.force_encoding(Encoding::US_ASCII)
end

The two methods produce identical encoding with the following URL, except one shows the warning:

> WEBrick::HTTPUtils.escape("https://a a.com?a='a\"")
=> "https://a%20a.com?a='a%22"

> URI.escape("https://a a.com?a='a\"")
(pry):16: warning: URI.escape is obsolete
=> "https://a%20a.com?a='a%22"

Would you consider adding this warning in here so people do not run into the same problem when they think they are being safe? We also propose the removal of URI.escape/encode altogether, which may already be in your plans. The deprecation warning was upgraded to a non-verbose warning 6 months ago here https://github.com/ruby/ruby/commit/869e2dd8c8efc1e7a043c9eee82d97c47befbcc7 and that commit mentions the warning itself has been there for 10 years.

No data to display

Also available in: Atom PDF