Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ group :development do
gem "bundler"
gem "libxml-ruby", platforms: [:ruby, :jruby]
gem "nokogiri", platforms: [:ruby, :jruby]
gem "rack"
gem "rack-test"
gem "rake"
gem "test-unit"
end
43 changes: 43 additions & 0 deletions lib/xmlrpc/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,49 @@ def http_write(body, status, header)
end


# Implements a XML-RPC application, which works with Rack
class RackApplication < BasicServer

# This method processes a XML-RPC method call and sends the answer
# back to the client.
def call(env)
length = env['CONTENT_LENGTH'].to_i

return http_error(405, "Method Not Allowed") unless env['REQUEST_METHOD'] == "POST"
return http_error(400, "Bad Request") unless parse_content_type(env['CONTENT_TYPE']).first == "text/xml"
return http_error(411, "Length Required") unless length > 0

req = Rack::Request.new(env)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to require rack in initialize?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would guess this is already available when you run this as Rack middleware, so that should only be required if you run this rack app without rack. This seems like an unlikely scenario to me.

Copy link
Member

Choose a reason for hiding this comment

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

OK. Let's revisit this when we receive a bug report / feature request about this.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, this is a Rack application not Rack middleware, right?

data = req.body.read(length)

return http_error(400, "Bad Request") if data.nil? or data.bytesize != length

[200, { "Content-type" => "text/xml; charset=utf-8" }, [process(data)]]
end


private

def http_error(status, message)
err = "#{status} #{message}"
msg = <<-"MSGEND"
<html>
<head>
<title>#{err}</title>
</head>
<body>
<h1>#{err}</h1>
<p>Unexpected error occurred while processing XML-RPC request!</p>
</body>
</html>
MSGEND
Comment on lines +560 to +570
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
msg = <<-"MSGEND"
<html>
<head>
<title>#{err}</title>
</head>
<body>
<h1>#{err}</h1>
<p>Unexpected error occurred while processing XML-RPC request!</p>
</body>
</html>
MSGEND
msg = <<-HTML
<html>
<head>
<title>#{err}</title>
</head>
<body>
<h1>#{err}</h1>
<p>Unexpected error occurred while processing XML-RPC request!</p>
</body>
</html>
HTML

Copy link
Member Author

Choose a reason for hiding this comment

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

We've got this exact same code snippet two more times in the existing code (CGIServer and ModRubyServer). I would prefer to keep this the same for now, and as a future work extract it to some shared code and remove this duplication. It's easier to find all the occurrences if we keep this exactly the same for now.
Once the code has been deduplicated, this improvement can be applied.

Copy link
Member

Choose a reason for hiding this comment

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

OK. Let's use the approach.


[status, { "Content-Type" => "text/html" }, [msg]]
end

end


class WEBrickServlet < BasicServer; end # forward declaration

# Implements a standalone XML-RPC server. The method XMLRPC::Server#serve is
Expand Down
91 changes: 91 additions & 0 deletions test/test_rack_server.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# frozen_string_literal: true

require "test/unit"
require "rack/test"
require "xmlrpc/server"
require 'xmlrpc/create'
require 'xmlrpc/parser'

class TestRack < Test::Unit::TestCase
include Rack::Test::Methods

def app
s = XMLRPC::RackApplication.new

s.add_handler("test.add") do |a,b|
a + b
end

s.add_handler("test.div") do |a,b|
if b == 0
raise XMLRPC::FaultException.new(1, "division by zero")
else
a / b
end
end

s.set_default_handler do |name, *args|
raise XMLRPC::FaultException.new(-99, "Method #{name} missing" +
" or wrong number of parameters!")
end

s.add_introspection

s
end

def test_successful_call
assert_equal([true, 9],
call("test.add", 4, 5))
end

def test_fault_exception
assert_equal([false, XMLRPC::FaultException.new(1, "division by zero")],
call("test.div", 1, 0))
end

def test_introspection
assert_equal([true, methods = ["test.add", "test.div", "system.listMethods", "system.methodSignature", "system.methodHelp"]],
call("system.listMethods"))
end

def test_missing_handler
assert_equal([false, XMLRPC::FaultException.new(-99, "Method test.nonexisting missing or wrong number of parameters!")],
call("test.nonexisting"))
end

def test_wrong_number_of_arguments
assert_equal([false, XMLRPC::FaultException.new(-99, "Method test.add missing or wrong number of parameters!")],
call("test.add", 1, 2, 3))
end

def test_multibyte_characters
assert_equal([true, "あいうえおかきくけこ"],
call("test.add", "あいうえお", "かきくけこ"))
end

def test_method_not_allowed
get("/", "<stub />", "CONTENT_TYPE" => "text/xml")
assert(last_response.method_not_allowed?, "Expected HTTP status code 405, got #{last_response.status} instead")
end

def test_bad_content_type
post("/", "<stub />", "CONTENT_TYPE" => "text/plain")
assert(last_response.bad_request?, "Expected HTTP status code 400, got #{last_response.status} instead")
end

def test_empty_request
post("/", "", "CONTENT_TYPE" => "text/xml")
assert_equal(411, last_response.status, "Expected HTTP status code 411, got #{last_response.status} instead")
end

def call(methodname, *args)
create = XMLRPC::Create.new(XMLRPC::Config.default_writer.new)
parser = XMLRPC::Config.default_parser.new

request = create.methodCall(methodname, *args)
post("/", request, "CONTENT_TYPE" => "text/xml")
assert(last_response.ok?, "Expected HTTP status code 200, got #{last_response.status} instead")
parser.parseMethodResponse(last_response.body)
end
end