Skip to content

fix: validate direct connect url #676

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

Merged
KazuCocoa merged 5 commits into from
Apr 19, 2026
Merged

fix: validate direct connect url #676

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ab1960
fix: add validation for the direct connect
KazuCocoa Apr 19, 2026
84e178c
address exceptions
KazuCocoa Apr 19, 2026
d1b43d3
add actual url
KazuCocoa Apr 19, 2026
da553cd
add s
KazuCocoa Apr 19, 2026
f9bed99
modify syntax a bit
KazuCocoa Apr 19, 2026
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
58 changes: 55 additions & 3 deletions lib/appium_lib_core/driver.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
# limitations under the License.

require 'uri'
require 'socket'
require 'ipaddr'

module Appium
# The struct for 'location'
Expand Down Expand Up @@ -99,6 +101,44 @@ def initialize(capabilities)
@port = capabilities[W3C_KEYS[:port]] || capabilities[KEYS[:port]]
@path = capabilities[W3C_KEYS[:path]] || capabilities[KEYS[:path]]
end

def valid?
return false unless [@protocol, @host, @port, @path].none?(&:nil?)

addresses = resolve_addresses(@host)
return false if addresses.empty?

addresses.find { |ip| disallowed? ip }.nil?
end
Comment thread
KazuCocoa marked this conversation as resolved.

private

# Do not allow loopback, link-local, unspecified and multicast addresses for
# direct connect since they are not accessible from outside of the server.
LOOPBACK_RANGES = [IPAddr.new('127.0.0.0/8'), IPAddr.new('::1/128')].freeze
LINK_LOCAL_RANGES = [IPAddr.new('169.254.0.0/16'), IPAddr.new('fe80::/10')].freeze
UNSPECIFIED_RANGES = [IPAddr.new('0.0.0.0/32'), IPAddr.new('::/128')].freeze
MULTICAST_RANGES = [IPAddr.new('224.0.0.0/4'), IPAddr.new('ff00::/8')].freeze
DISALLOWED_RANGES = [
*LOOPBACK_RANGES,
*LINK_LOCAL_RANGES,
*UNSPECIFIED_RANGES,
*MULTICAST_RANGES
].freeze

def resolve_addresses(host)
normalized_host = host.to_s.delete_prefix('[').delete_suffix(']')
Socket.getaddrinfo(normalized_host, nil).map { |entry| entry[3] }.uniq
rescue SocketError => e
error_message = "Failed to resolve host '#{host}' for direct connect: #{e.message}"
::Appium::Logger.warn(error_message)
[]
end
Comment on lines +129 to +136
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

valid? calls Socket.getaddrinfo during start_driver, which can introduce blocking DNS resolution into session creation and may fail in restricted/offline environments even when the direct-connect URL would otherwise be usable. To reduce operational risk, consider short-circuiting resolution when host is already an IP literal, and/or making hostname resolution optional or bounded (e.g., a configurable timeout / non-blocking resolver) so start_driver cannot hang on DNS.

Copilot uses AI. Check for mistakes.

def disallowed?(ip)
address = IPAddr.new ip
DISALLOWED_RANGES.any? { |range| range.include? address }
end
end

class Driver
Expand Down Expand Up @@ -164,8 +204,7 @@ class Driver
# @return [Appium::Core::Base::Driver]
attr_reader :driver

# <b>[Experimental feature]</b><br>
# Enable an experimental feature updating Http client endpoint following below keys by Appium/Selenium server.<br>
# Enable updating Http client endpoint following below keys by Appium/Selenium server.<br>
# This works with {Appium::Core::Base::Http::Default}.
#
# If your Selenium/Appium server decorates the new session capabilities response with the following keys:<br>
Expand All @@ -177,6 +216,12 @@ class Driver
# ignore them if this parameter is <code>false</code>. Defaults to true.
# These keys can have <code>appium:</code> prefix.
#
# Note that the server should provide the keys with valid values. The host value must not be
# - loopback (for example `127.0.0.1`, `::1`)
# - link-local (for example `169.254.x.x`, `fe80::/10`)
# - unspecified/wildcard (`0.0.0.0`, `::`)
# - multicast (`224.0.0.0/4`, `ff00::/8`)
#
# @return [Bool]
attr_reader :direct_connect

Expand Down Expand Up @@ -419,7 +464,14 @@ def start_driver(server_url: nil,

if @direct_connect
d_c = DirectConnections.new(@driver.capabilities)
@driver.update_sending_request_to(protocol: d_c.protocol, host: d_c.host, port: d_c.port, path: d_c.path)
if d_c.valid?
@driver.update_sending_request_to(protocol: d_c.protocol, host: d_c.host, port: d_c.port, path: d_c.path)
else
::Appium::Logger.warn(
"Direct connect is enabled but the server did not provide valid direct connect information (#{d_c.protocol}, #{d_c.host}, #{d_c.port}, #{d_c.path}). " \
"Continue with the original URL (#{@custom_url})"
)
end
end
rescue Errno::ECONNREFUSED => e
raise "ERROR: Unable to connect to Appium. Is the server running on #{@custom_url}? Error: #{e}"
Expand Down
81 changes: 62 additions & 19 deletions test/unit/driver_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def test_default_timeout_for_http_client_with_direct
appActivity: 'io.appium.android.apis.ApiDemos',
someCapability: 'some_capability',
directConnectProtocol: 'http',
directConnectHost: 'localhost',
directConnectHost: '1.1.1.1',
directConnectPort: '8888',
directConnectPath: '/wd/hub'
}
Expand All @@ -159,14 +159,14 @@ def test_default_timeout_for_http_client_with_direct
stub_request(:post, 'http://127.0.0.1:4723/session')
.to_return(headers: HEADER, status: 200, body: response)

stub_request(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts')
stub_request(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts')
.with(body: { implicit: 30_000 }.to_json)
.to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)

driver = core.start_driver

assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
assert_requested(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts',
body: { implicit: 30_000 }.to_json, times: 1)
driver
end
Expand All @@ -179,7 +179,7 @@ def test_default_timeout_for_http_client_with_direct
uri = driver.send(:bridge).http.send(:server_url)
assert core.direct_connect
assert_equal 'http', uri.scheme
assert_equal 'localhost', uri.host
assert_equal '1.1.1.1', uri.host
assert_equal 8888, uri.port
assert_equal '/wd/hub/', uri.path
end
Expand All @@ -198,8 +198,8 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix
appPackage: 'io.appium.android.apis',
appActivity: 'io.appium.android.apis.ApiDemos',
someCapability: 'some_capability',
'appium:directConnectProtocol' => 'http',
'appium:directConnectHost' => 'localhost',
'appium:directConnectProtocol' => 'https',
'appium:directConnectHost' => 'appium.io',
'appium:directConnectPort' => '8888',
'appium:directConnectPath' => '/wd/hub'
}
Comment on lines 199 to 205
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

This test now depends on external DNS resolution for appium.io because the implementation validates direct-connect hosts using Socket.getaddrinfo. In CI/offline environments, DNS failures would cause d_c.valid? to return false and the driver to fall back to the original URL, making this test flaky. Prefer using an IP literal (e.g., a TEST-NET address like 203.0.113.10) or stubbing Socket.getaddrinfo in the test.

Copilot uses AI. Check for mistakes.
Expand All @@ -209,14 +209,14 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix
stub_request(:post, 'http://127.0.0.1:4723/session')
.to_return(headers: HEADER, status: 200, body: response)

stub_request(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts')
stub_request(:post, 'https://appium.io:8888/wd/hub/session/1234567890/timeouts')
.with(body: { implicit: 30_000 }.to_json)
.to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)

driver = core.start_driver

assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
assert_requested(:post, 'https://appium.io:8888/wd/hub/session/1234567890/timeouts',
body: { implicit: 30_000 }.to_json, times: 1)
driver
end
Expand All @@ -228,8 +228,8 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix
assert_equal 999_999, driver.send(:bridge).http.read_timeout
uri = driver.send(:bridge).http.send(:server_url)
assert core.direct_connect
assert_equal 'http', uri.scheme
assert_equal 'localhost', uri.host
assert_equal 'https', uri.scheme
assert_equal 'appium.io', uri.host
assert_equal 8888, uri.port
assert_equal '/wd/hub/', uri.path
end
Expand All @@ -249,7 +249,7 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix_prior_than_no
appActivity: 'io.appium.android.apis.ApiDemos',
someCapability: 'some_capability',
'appium:directConnectProtocol' => 'http',
'appium:directConnectHost' => 'localhost',
'appium:directConnectHost' => '1.1.1.1',
'appium:directConnectPort' => '8888',
'appium:directConnectPath' => '/wd/hub',
directConnectProtocol: 'https',
Expand All @@ -263,14 +263,14 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix_prior_than_no
stub_request(:post, 'http://127.0.0.1:4723/session')
.to_return(headers: HEADER, status: 200, body: response)

stub_request(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts')
stub_request(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts')
.with(body: { implicit: 30_000 }.to_json)
.to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)

driver = core.start_driver

assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
assert_requested(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts',
body: { implicit: 30_000 }.to_json, times: 1)
driver
end
Expand All @@ -283,11 +283,54 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix_prior_than_no
uri = driver.send(:bridge).http.send(:server_url)
assert core.direct_connect
assert_equal 'http', uri.scheme
assert_equal 'localhost', uri.host
assert_equal '1.1.1.1', uri.host
assert_equal 8888, uri.port
assert_equal '/wd/hub/', uri.path
end

def test_direct_connect_loopback_host_falls_back_to_original_url
response = {
value: {
sessionId: '1234567890',
capabilities: {
platformName: :android,
automationName: ENV['APPIUM_DRIVER'] || 'uiautomator2',
app: 'test/functional/app/ApiDemos-debug.apk',
platformVersion: '7.1.1',
deviceName: 'Android Emulator',
appPackage: 'io.appium.android.apis',
appActivity: 'io.appium.android.apis.ApiDemos',
someCapability: 'some_capability',
directConnectProtocol: 'http',
directConnectHost: 'localhost',
directConnectPort: '8888',
directConnectPath: '/wd/hub'
}
}
}.to_json

stub_request(:post, 'http://127.0.0.1:4723/session')
.to_return(headers: HEADER, status: 200, body: response)

stub_request(:post, 'http://127.0.0.1:4723/session/1234567890/timeouts')
.with(body: { implicit: 30_000 }.to_json)
.to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)

core = ::Appium::Core.for(Caps.android_direct)
driver = core.start_driver

uri = driver.send(:bridge).http.send(:server_url)
assert_equal 'http', uri.scheme
assert_equal '127.0.0.1', uri.host
assert_equal 4723, uri.port
assert_equal '/', uri.path

assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
assert_requested(:post, 'http://127.0.0.1:4723/session/1234567890/timeouts',
body: { implicit: 30_000 }.to_json, times: 1)
assert_not_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts')
end

def test_default_timeout_for_http_client_with_direct_no_path
android_mock_create_session_w3c_direct_no_path = lambda do |core|
response = {
Expand All @@ -303,7 +346,7 @@ def test_default_timeout_for_http_client_with_direct_no_path
appActivity: 'io.appium.android.apis.ApiDemos',
someCapability: 'some_capability',
directConnectProtocol: 'http',
directConnectHost: 'localhost',
directConnectHost: '1.1.1.1',
directConnectPort: '8888'
}
}
Expand Down Expand Up @@ -352,7 +395,7 @@ def test_default_timeout_for_http_client_with_direct_no_supported_client
appActivity: 'io.appium.android.apis.ApiDemos',
someCapability: 'some_capability',
directConnectProtocol: 'http',
directConnectHost: 'localhost',
directConnectHost: '1.1.1.1',
directConnectPort: '8888',
directConnectPath: '/wd/hub'
}
Expand Down Expand Up @@ -517,7 +560,7 @@ def test_attach_to_an_existing_session
appActivity: 'io.appium.android.apis.ApiDemos',
someCapability: 'some_capability',
directConnectProtocol: 'http',
directConnectHost: 'localhost',
directConnectHost: '1.1.1.1',
directConnectPort: '8888',
directConnectPath: '/wd/hub'
}
Expand All @@ -527,14 +570,14 @@ def test_attach_to_an_existing_session
stub_request(:post, 'http://127.0.0.1:4723/session')
.to_return(headers: HEADER, status: 200, body: response)

stub_request(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts')
stub_request(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts')
.with(body: { implicit: 30_000 }.to_json)
.to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)

driver = core.start_driver

assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
assert_requested(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts',
body: { implicit: 30_000 }.to_json, times: 1)
driver
end
Expand Down
Loading