From e70c3cdc3aa2ab22ac036a408436b52cdde61692 Mon Sep 17 00:00:00 2001 From: Lachlan Dowding Date: Wed, 3 Jan 2018 02:00:27 +1000 Subject: [PATCH] Change URI#to_s to handle default ports for more schemes than http/https (#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 --- spec/std/uri_spec.cr | 61 +++++++++++++++++++++++++++++- src/uri.cr | 89 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 2 deletions(-) diff --git a/spec/std/uri_spec.cr b/spec/std/uri_spec.cr index 0270a361f117..7d34065911d3 100644 --- a/spec/std/uri_spec.cr +++ b/spec/std/uri_spec.cr @@ -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 diff --git a/src/uri.cr b/src/uri.cr index e6d07b6abcf0..b75717d72755 100644 --- a/src/uri.cr +++ b/src/uri.cr @@ -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 @@ -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