Skip to content

Commit

Permalink
Change URI#to_s to handle default ports for more schemes than http/ht…
Browse files Browse the repository at this point in the history 
…tps (crystal-lang#5233)

* Change URI#to_s to handle default ports for more schemes than http/https

* Change DEFAULT_PORTS to be a constant rather than class variable

* Rename method to default_port? and move nil check back to #to_s

* Remove unnecessary return statement from default_port? method

* Add URI.default_ports method, which allows for default ports to
be queried, registered, and unregistered, as follows.

Query the default port for a given scheme:

    URI.default_ports["http"]  #=> 80
    URI.default_ports["ponzi"] #=> nil

Register a default port for a given scheme:

    URI.default_ports["ponzi"] = 9999
    URI.default_ports["ponzi"] #=> 9999

Unregister a default port for a given scheme:

    URI.default_ports["ftp"] #=> 21
    URI.default_ports["ftp"] = nil
    URI.default_ports["ftp"] #=> nil

* Fix URI::DefaultPorts#[]= to downcase scheme on delete

* Add URI.default_port method for returning a scheme's the default port

    URI.default_port "http"  # => 80
    URI.default_port "ponzi" # => nil

* Rename URI.default_ports method to URI.default_port, allowing
default ports to be accessed via either a hash like accessor or
getter method with consistent naming:

    URI.default_port "http"  # => 80
    URI.default_port["http"] # => 80

* Fix method comments to use back ticks consistently

* Change DefaultPort class to use instance variable rather than class
variable for the internal hash of default ports, and to be more hash
like for unregistering a default port via the new delete method

* Simplify to use two methods: URI.default_port and URI.set_default_port

* Fix the method documentation formatting

* Add more well-known default ports sourced from IANA via Mahmoud's
[scheme_port_map.json][1]

[1]: https://gist.github.com/mahmoud/2fe281a8daaff26cfe9c15d2c5bf5c8b

* Fix #default_port? to correctly handle nil scheme

* Change URI default port API to return a mutable subclass of Hash
with case-insensitive keys

* Change DefaultPortHash seeding to use merge! method

* Fix default_port? to not call scheme.downcase unnecessarily

* Add spec for URI with nil scheme and non-nil port

* Change DefaultPortHash seeds to be an initialize parameter

* Fix DefaultPortHash case insensitive keys for methods other than []= and fetch

* Revert to using two methods: URI.default_port and URI.set_default_port

* Fix typo in URI.default_port comment

* Fix typo in URI.set_default_port comment

* Fix URI.set_default_port comment formatting for scheme parameter

* Fix URI#default_port? to be thread safe

* Change URI.set_default_port to always return nil
  • Loading branch information
@lachlan @lukeasrodgers
lachlan authored and lukeasrodgers committed Jan 7, 2018
1 parent 970263e commit e70c3cd
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 2 deletions.
61 changes: 60 additions & 1 deletion spec/std/uri_spec.cr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,69 @@ describe "URI" do
end
it { URI.new("http", "www.example.com", user: "@al:ce", password: "s/cr3t").to_s.should eq("http://%40al%3Ace:s%2Fcr3t@www.example.com") }
it { URI.new("http", "www.example.com", fragment: "top").to_s.should eq("http://www.example.com#top") }
it { URI.new("http", "www.example.com", 1234).to_s.should eq("http://www.example.com:1234") }
it { URI.new("http", "www.example.com", 80, "/hello").to_s.should eq("http://www.example.com/hello") }
it { URI.new("http", "www.example.com", 80, "/hello", "a=1").to_s.should eq("http://www.example.com/hello?a=1") }
it { URI.new("mailto", opaque: "foo@example.com").to_s.should eq("mailto:foo@example.com") }

it "removes default port" do
URI.new("http", "www.example.com", 80).to_s.should eq("http://www.example.com")
URI.new("https", "www.example.com", 443).to_s.should eq("https://www.example.com")
URI.new("ftp", "www.example.com", 21).to_s.should eq("ftp://www.example.com")
URI.new("sftp", "www.example.com", 22).to_s.should eq("sftp://www.example.com")
URI.new("ldap", "www.example.com", 389).to_s.should eq("ldap://www.example.com")
URI.new("ldaps", "www.example.com", 636).to_s.should eq("ldaps://www.example.com")
end

it "preserves non-default port" do
URI.new("http", "www.example.com", 1234).to_s.should eq("http://www.example.com:1234")
URI.new("https", "www.example.com", 1234).to_s.should eq("https://www.example.com:1234")
URI.new("ftp", "www.example.com", 1234).to_s.should eq("ftp://www.example.com:1234")
URI.new("sftp", "www.example.com", 1234).to_s.should eq("sftp://www.example.com:1234")
URI.new("ldap", "www.example.com", 1234).to_s.should eq("ldap://www.example.com:1234")
URI.new("ldaps", "www.example.com", 1234).to_s.should eq("ldaps://www.example.com:1234")
end

it "preserves port for unknown scheme" do
URI.new("xyz", "www.example.com").to_s.should eq("xyz://www.example.com")
URI.new("xyz", "www.example.com", 1234).to_s.should eq("xyz://www.example.com:1234")
end

it "preserves port for nil scheme" do
URI.new(nil, "www.example.com", 1234).to_s.should eq("www.example.com:1234")
end
end

describe ".default_port" do
it "returns default port for well known schemes" do
URI.default_port("http").should eq(80)
URI.default_port("https").should eq(443)
end

it "returns nil for unknown schemes" do
URI.default_port("xyz").should eq(nil)
end

it "treats scheme case insensitively" do
URI.default_port("Http").should eq(80)
URI.default_port("HTTP").should eq(80)
end
end

describe ".set_default_port" do
it "registers port for scheme" do
URI.set_default_port("ponzi", 9999)
URI.default_port("ponzi").should eq(9999)
end

it "unregisters port for scheme" do
URI.set_default_port("ftp", nil)
URI.default_port("ftp").should eq(nil)
end

it "treats scheme case insensitively" do
URI.set_default_port("UNKNOWN", 1234)
URI.default_port("unknown").should eq(1234)
end
end

describe ".unescape" do
Expand Down
89 changes: 88 additions & 1 deletion src/uri.cr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class URI
if host
io << host
end
if port && !((scheme == "http" && port == 80) || (scheme == "https" && port == 443))
unless port.nil? || default_port?
io << ':'
io << port
end
Expand Down Expand Up @@ -428,4 +428,91 @@ class URI
URI.escape(password, io)
end
end

# A map of schemes and their respective default ports, seeded
# with some well-known schemes sourced from the IANA [Uniform
# Resource Identifier (URI) Schemes][1] and [Service Name and
# Transport Protocol Port Number Registry][2] via Mahmoud
# Hashemi's [scheme_port_map.json][3].
#
# [1]: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
# [2]: https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml
# [3]: https://gist.github.com/mahmoud/2fe281a8daaff26cfe9c15d2c5bf5c8b
@@default_ports = {
"acap" => 674,
"afp" => 548,
"dict" => 2628,
"dns" => 53,
"ftp" => 21,
"ftps" => 990,
"git" => 9418,
"gopher" => 70,
"http" => 80,
"https" => 443,
"imap" => 143,
"ipp" => 631,
"ipps" => 631,
"irc" => 194,
"ircs" => 6697,
"ldap" => 389,
"ldaps" => 636,
"mms" => 1755,
"msrp" => 2855,
"mtqp" => 1038,
"nfs" => 111,
"nntp" => 119,
"nntps" => 563,
"pop" => 110,
"prospero" => 1525,
"redis" => 6379,
"rsync" => 873,
"rtsp" => 554,
"rtsps" => 322,
"rtspu" => 5005,
"scp" => 22,
"sftp" => 22,
"smb" => 445,
"snmp" => 161,
"ssh" => 22,
"svn" => 3690,
"telnet" => 23,
"ventrilo" => 3784,
"vnc" => 5900,
"wais" => 210,
"ws" => 80,
"wss" => 443,
}

# Returns the default port for the given *scheme* if known,
# otherwise returns `nil`.
#
# ```
# URI.default_port "http" # => 80
# URI.default_port "ponzi" # => nil
# ```
def self.default_port(scheme : String) : Int32?
@@default_ports[scheme.downcase]?
end

# Registers the default port for the given *scheme*.
#
# If *port* is `nil`, the existing default port for the
# *scheme*, if any, will be unregistered.
#
# ```
# URI.set_default_port "ponzi", 9999
# ```
def self.set_default_port(scheme : String, port : Int32?) : Nil
if port
@@default_ports[scheme.downcase] = port
else
@@default_ports.delete scheme.downcase
end
end

# Returns `true` if this URI's *port* is the default port for
# its *scheme*.
private def default_port?
(scheme = @scheme) && (port = @port) && port == URI.default_port(scheme)
end
end

0 comments on commit e70c3cd

Please sign in to comment.