Auto merge of #1809 - norchard:master, r=segiddins

Fixed broken links and overzealous URL encoding in gem server

# Description:

Bugs introduced in commit a58d893

Fix for Issue #1793: Bad hrefs in 'gem server'- incorrect non-alpha character encoding

1. Reintroduced erroneously deleted characters in links.

2. Removed URL encoding from homepage links, which broke links. For example, "http://rubyonrails.org" became "http%3A%2F%2Frubyonrails.org". Added a method called uri_encode instead of using the deprecated URI.encode to encode unsafe characters without encoding the URL itself.

3. Removed URL encoding from doc path. The doc_root function that generates doc path already encodes the gem name. The second encoding broke the link.
______________

# Tasks:

- [X] Describe the problem / feature
- [X] Write tests
- [X] Write code to solve the problem
- [X] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).

lib/rubygems/server.rb

@@ -2,6 +2,7 @@
 require 'webrick'
 require 'zlib'
 require 'erb'
+require 'uri'
 
 require 'rubygems'
 require 'rubygems/rdoc'
@@ -68,7 +69,7 @@ class Gem::Server
         <h1>Summary</h1>
   <p>There are <%=values["gem_count"]%> gems installed:</p>
   <p>
-  <%= values["specs"].map { |v| "<a href\"##{u v["name"]}\">#{h v["name"]}</a>" }.join ', ' %>.
+  <%= values["specs"].map { |v| "<a href=\"##{u v["name"]}\">#{h v["name"]}</a>" }.join ', ' %>.
   <h1>Gems</h1>
 
   <dl>
@@ -81,20 +82,20 @@ class Gem::Server
     <b><%=h spec["name"]%> <%=h spec["version"]%></b>
 
     <% if spec["ri_installed"] || spec["rdoc_installed"] then %>
-      <a href="<%=u spec["doc_path"]%>">[rdoc]</a>
+      <a href="<%=spec["doc_path"]%>">[rdoc]</a>
     <% else %>
       <span title="rdoc not installed">[rdoc]</span>
     <% end %>
 
     <% if spec["homepage"] then %>
-      <a href="<%=u spec["homepage"]%>" title="<%=h spec["homepage"]%>">[www]</a>
+      <a href="<%=uri_encode spec["homepage"]%>" title="<%=h spec["homepage"]%>">[www]</a>
     <% else %>
       <span title="no homepage available">[www]</span>
     <% end %>
 
     <% if spec["has_deps"] then %>
      - depends on
-      <%= spec["dependencies"].map { |v| "<a href=\"##{u v["name"]}>#{h v["name"]}</a>" }.join ', ' %>.
+      <%= spec["dependencies"].map { |v| "<a href=\"##{u v["name"]}\">#{h v["name"]}</a>" }.join ', ' %>.
     <% end %>
     </dt>
     <dd>
@@ -455,6 +456,12 @@ def add_date res
     end.max
   end
 
+  def uri_encode(str)
+    str.gsub(URI::UNSAFE) do |match|
+      match.each_byte.map { |c| sprintf('%%%02X', c.ord) }.join
+    end
+  end
+
   def doc_root gem_name
     if have_rdoc_4_plus? then
       "/doc_root/#{u gem_name}/"

test/rubygems/test_gem_server.rb

@@ -392,6 +392,22 @@ def test_specs_gz
                  Marshal.load(Gem.gunzip(@res.body))
   end
 
+  def test_uri_encode
+    url_safe = @server.uri_encode 'http://rubyonrails.org/">malicious_content</a>'
+    assert_equal url_safe, 'http://rubyonrails.org/%22%3Emalicious_content%3C/a%3E'
+  end
+
+  # Regression test for issue #1793: incorrect URL encoding.
+  # Checking that no URLs have had '://' incorrectly encoded
+  def test_regression_1793
+    data = StringIO.new "GET / HTTP/1.0\r\n\r\n"
+    @req.parse data
+
+    @server.root @req, @res
+
+    refute_match %r|%3A%2F%2F|, @res.body
+  end
+
   def util_listen
     webrick = Object.new
     webrick.instance_variable_set :@listeners, []