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.

Sign up for GitHub

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

Fix/remove-webrick-dependency #108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix/remove-webrick-dependency #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion heroics.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,5 @@ Gem::Specification.new do |spec|
spec.add_dependency 'excon'
spec.add_dependency 'multi_json', '>= 1.9.2'
spec.add_dependency 'moneta'
spec.add_dependency 'webrick'
spec.add_dependency 'base64'
end
1 change: 0 additions & 1 deletion lib/heroics.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
require 'excon'
require 'multi_json'
require 'uri'
require 'webrick'
require 'zlib'

# Heroics is an HTTP client for an API described by a JSON schema.
Expand Down
34 changes: 33 additions & 1 deletion lib/heroics/schema.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,39 @@ def lookup_parameter(path, schema)
# @return [String] The formatted parameter.
def format_parameter(parameter)
formatted_parameter = parameter.instance_of?(Time) ? iso_format(parameter) : parameter.to_s
WEBrick::HTTPUtils.escape formatted_parameter
url_escape_parameter formatted_parameter
end

# URL escape a path parameter. Copied from WEBrick::HTTPUtils.escape:
# https://github.com/ruby/webrick/blob/0fb9de6788a3ba5fe903e63d778a0fb8c1dce786/lib/webrick/httputils.rb#L498
#
# @param [Fixnum,String,TrueClass,FalseClass,Time] The parameter to escape.
# @return [String] The escaped parameter.
def url_escape_parameter(parameter)
# Note: this RegEx is obtained by executing WEBrick::HTTPUtils::UNESCAPED,
# splitting it over multiple lines and using the x modifier to make this possible.
Comment on lines +346 to +347
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be better to either copy over the smaller pieces of this Regex, as WEBrick does it, or at least make that a perma-link to the version from which it was taken (i.e., https://github.com/ruby/webrick/blob/9350944141a3f15acda9c79edd5393289c098e04/lib/webrick/httputils.rb#L498). Or… both!?!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not copy the smaller pieces of the regex but used the complete result instead on purpose. We do not need the smaller piecers. We do not use them by ourselves. It may improve understandability of where this is coming from, but to use the smaller piecer, we would also need to copy the custom _make_regex method and logic of what the UNESCAPED constant is (UNESCAPED = _make_regex(control+space+delims+unwise+nonascii)). This would result in duplicating more code.

Instead I think it is better to just execute WEBrick::HTTPUtils::UNESCAPED and copying the result as I did here.

Good suggestion about the permalink though. Fixed that :)

regex = %r{
([
\x00\x01\x02\x03\x04\x05\x06\x07\x08
\t\n\v\f\r
\x0E\x0F\x10\x11\x12\x13\x14\x15\x16
\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F\x7F
\ <>\#%"\{\}\|\\\^\[\]`
\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8A\x8B\x8C\x8D\x8E\x8F
\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9A\x9B\x9C\x9D\x9E\x9F
\xA0\xA1\xA2\xA3\xA4\xA5\xA6\xA7\xA8\xA9\xAA\xAB\xAC\xAD\xAE\xAF
\xB0\xB1\xB2\xB3\xB4\xB5\xB6\xB7\xB8\xB9\xBA\xBB\xBC\xBD\xBE\xBF
\xC0\xC1\xC2\xC3\xC4\xC5\xC6\xC7\xC8\xC9\xCA\xCB\xCC\xCD\xCE\xCF
\xD0\xD1\xD2\xD3\xD4\xD5\xD6\xD7\xD8\xD9\xDA\xDB\xDC\xDD\xDE\xDF
\xE0\xE1\xE2\xE3\xE4\xE5\xE6\xE7\xE8\xE9\xEA\xEB\xEC\xED\xEE\xEF
\xF0\xF1\xF2\xF3\xF4\xF5\xF6\xF7\xF8\xF9\xFA\xFB\xFC\xFD\xFE\xFF
])
}nx

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

# Convert a time to an ISO 8601 combined data and time format.
Expand Down