Skip to content

Commit 3bbc416

Browse files
KazuCocoaKazuCocoa
authored
fix: validate direct connect url (#676)
* fix: add validation for the direct connect * address exceptions * add actual url * add s * modify syntax a bit
1 parent b1a9007 commit 3bbc416

2 files changed

Lines changed: 117 additions & 22 deletions

File tree

‎lib/appium_lib_core/driver.rb‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
# limitations under the License.
1414

1515
require 'uri'
16+
require 'socket'
17+
require 'ipaddr'
1618

1719
module Appium
1820
# The struct for 'location'
@@ -99,6 +101,44 @@ def initialize(capabilities)
99101
@port = capabilities[W3C_KEYS[:port]] || capabilities[KEYS[:port]]
100102
@path = capabilities[W3C_KEYS[:path]] || capabilities[KEYS[:path]]
101103
end
104+
105+
def valid?
106+
return false unless [@protocol, @host, @port, @path].none?(&:nil?)
107+
108+
addresses = resolve_addresses(@host)
109+
return false if addresses.empty?
110+
111+
addresses.find { |ip| disallowed? ip }.nil?
112+
end
113+
114+
private
115+
116+
# Do not allow loopback, link-local, unspecified and multicast addresses for
117+
# direct connect since they are not accessible from outside of the server.
118+
LOOPBACK_RANGES = [IPAddr.new('127.0.0.0/8'), IPAddr.new('::1/128')].freeze
119+
LINK_LOCAL_RANGES = [IPAddr.new('169.254.0.0/16'), IPAddr.new('fe80::/10')].freeze
120+
UNSPECIFIED_RANGES = [IPAddr.new('0.0.0.0/32'), IPAddr.new('::/128')].freeze
121+
MULTICAST_RANGES = [IPAddr.new('224.0.0.0/4'), IPAddr.new('ff00::/8')].freeze
122+
DISALLOWED_RANGES = [
123+
*LOOPBACK_RANGES,
124+
*LINK_LOCAL_RANGES,
125+
*UNSPECIFIED_RANGES,
126+
*MULTICAST_RANGES
127+
].freeze
128+
129+
def resolve_addresses(host)
130+
normalized_host = host.to_s.delete_prefix('[').delete_suffix(']')
131+
Socket.getaddrinfo(normalized_host, nil).map { |entry| entry[3] }.uniq
132+
rescue SocketError => e
133+
error_message = "Failed to resolve host '#{host}' for direct connect: #{e.message}"
134+
::Appium::Logger.warn(error_message)
135+
[]
136+
end
137+
138+
def disallowed?(ip)
139+
address = IPAddr.new ip
140+
DISALLOWED_RANGES.any? { |range| range.include? address }
141+
end
102142
end
103143

104144
class Driver
@@ -164,8 +204,7 @@ class Driver
164204
# @return [Appium::Core::Base::Driver]
165205
attr_reader :driver
166206

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

@@ -419,7 +464,14 @@ def start_driver(server_url: nil,
419464

420465
if @direct_connect
421466
d_c = DirectConnections.new(@driver.capabilities)
422-
@driver.update_sending_request_to(protocol: d_c.protocol, host: d_c.host, port: d_c.port, path: d_c.path)
467+
if d_c.valid?
468+
@driver.update_sending_request_to(protocol: d_c.protocol, host: d_c.host, port: d_c.port, path: d_c.path)
469+
else
470+
::Appium::Logger.warn(
471+
"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}). " \
472+
"Continue with the original URL (#{@custom_url})"
473+
)
474+
end
423475
end
424476
rescue Errno::ECONNREFUSED => e
425477
raise "ERROR: Unable to connect to Appium. Is the server running on #{@custom_url}? Error: #{e}"

‎test/unit/driver_test.rb‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ def test_default_timeout_for_http_client_with_direct
149149
appActivity: 'io.appium.android.apis.ApiDemos',
150150
someCapability: 'some_capability',
151151
directConnectProtocol: 'http',
152-
directConnectHost: 'localhost',
152+
directConnectHost: '1.1.1.1',
153153
directConnectPort: '8888',
154154
directConnectPath: '/wd/hub'
155155
}
@@ -159,14 +159,14 @@ def test_default_timeout_for_http_client_with_direct
159159
stub_request(:post, 'http://127.0.0.1:4723/session')
160160
.to_return(headers: HEADER, status: 200, body: response)
161161

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

166166
driver = core.start_driver
167167

168168
assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
169-
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
169+
assert_requested(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts',
170170
body: { implicit: 30_000 }.to_json, times: 1)
171171
driver
172172
end
@@ -179,7 +179,7 @@ def test_default_timeout_for_http_client_with_direct
179179
uri = driver.send(:bridge).http.send(:server_url)
180180
assert core.direct_connect
181181
assert_equal 'http', uri.scheme
182-
assert_equal 'localhost', uri.host
182+
assert_equal '1.1.1.1', uri.host
183183
assert_equal 8888, uri.port
184184
assert_equal '/wd/hub/', uri.path
185185
end
@@ -198,8 +198,8 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix
198198
appPackage: 'io.appium.android.apis',
199199
appActivity: 'io.appium.android.apis.ApiDemos',
200200
someCapability: 'some_capability',
201-
'appium:directConnectProtocol' => 'http',
202-
'appium:directConnectHost' => 'localhost',
201+
'appium:directConnectProtocol' => 'https',
202+
'appium:directConnectHost' => 'appium.io',
203203
'appium:directConnectPort' => '8888',
204204
'appium:directConnectPath' => '/wd/hub'
205205
}
@@ -209,14 +209,14 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix
209209
stub_request(:post, 'http://127.0.0.1:4723/session')
210210
.to_return(headers: HEADER, status: 200, body: response)
211211

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

216216
driver = core.start_driver
217217

218218
assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
219-
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
219+
assert_requested(:post, 'https://appium.io:8888/wd/hub/session/1234567890/timeouts',
220220
body: { implicit: 30_000 }.to_json, times: 1)
221221
driver
222222
end
@@ -228,8 +228,8 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix
228228
assert_equal 999_999, driver.send(:bridge).http.read_timeout
229229
uri = driver.send(:bridge).http.send(:server_url)
230230
assert core.direct_connect
231-
assert_equal 'http', uri.scheme
232-
assert_equal 'localhost', uri.host
231+
assert_equal 'https', uri.scheme
232+
assert_equal 'appium.io', uri.host
233233
assert_equal 8888, uri.port
234234
assert_equal '/wd/hub/', uri.path
235235
end
@@ -249,7 +249,7 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix_prior_than_no
249249
appActivity: 'io.appium.android.apis.ApiDemos',
250250
someCapability: 'some_capability',
251251
'appium:directConnectProtocol' => 'http',
252-
'appium:directConnectHost' => 'localhost',
252+
'appium:directConnectHost' => '1.1.1.1',
253253
'appium:directConnectPort' => '8888',
254254
'appium:directConnectPath' => '/wd/hub',
255255
directConnectProtocol: 'https',
@@ -263,14 +263,14 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix_prior_than_no
263263
stub_request(:post, 'http://127.0.0.1:4723/session')
264264
.to_return(headers: HEADER, status: 200, body: response)
265265

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

270270
driver = core.start_driver
271271

272272
assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
273-
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
273+
assert_requested(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts',
274274
body: { implicit: 30_000 }.to_json, times: 1)
275275
driver
276276
end
@@ -283,11 +283,54 @@ def test_default_timeout_for_http_client_with_direct_appium_prefix_prior_than_no
283283
uri = driver.send(:bridge).http.send(:server_url)
284284
assert core.direct_connect
285285
assert_equal 'http', uri.scheme
286-
assert_equal 'localhost', uri.host
286+
assert_equal '1.1.1.1', uri.host
287287
assert_equal 8888, uri.port
288288
assert_equal '/wd/hub/', uri.path
289289
end
290290

291+
def test_direct_connect_loopback_host_falls_back_to_original_url
292+
response = {
293+
value: {
294+
sessionId: '1234567890',
295+
capabilities: {
296+
platformName: :android,
297+
automationName: ENV['APPIUM_DRIVER'] || 'uiautomator2',
298+
app: 'test/functional/app/ApiDemos-debug.apk',
299+
platformVersion: '7.1.1',
300+
deviceName: 'Android Emulator',
301+
appPackage: 'io.appium.android.apis',
302+
appActivity: 'io.appium.android.apis.ApiDemos',
303+
someCapability: 'some_capability',
304+
directConnectProtocol: 'http',
305+
directConnectHost: 'localhost',
306+
directConnectPort: '8888',
307+
directConnectPath: '/wd/hub'
308+
}
309+
}
310+
}.to_json
311+
312+
stub_request(:post, 'http://127.0.0.1:4723/session')
313+
.to_return(headers: HEADER, status: 200, body: response)
314+
315+
stub_request(:post, 'http://127.0.0.1:4723/session/1234567890/timeouts')
316+
.with(body: { implicit: 30_000 }.to_json)
317+
.to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)
318+
319+
core = ::Appium::Core.for(Caps.android_direct)
320+
driver = core.start_driver
321+
322+
uri = driver.send(:bridge).http.send(:server_url)
323+
assert_equal 'http', uri.scheme
324+
assert_equal '127.0.0.1', uri.host
325+
assert_equal 4723, uri.port
326+
assert_equal '/', uri.path
327+
328+
assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
329+
assert_requested(:post, 'http://127.0.0.1:4723/session/1234567890/timeouts',
330+
body: { implicit: 30_000 }.to_json, times: 1)
331+
assert_not_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts')
332+
end
333+
291334
def test_default_timeout_for_http_client_with_direct_no_path
292335
android_mock_create_session_w3c_direct_no_path = lambda do |core|
293336
response = {
@@ -303,7 +346,7 @@ def test_default_timeout_for_http_client_with_direct_no_path
303346
appActivity: 'io.appium.android.apis.ApiDemos',
304347
someCapability: 'some_capability',
305348
directConnectProtocol: 'http',
306-
directConnectHost: 'localhost',
349+
directConnectHost: '1.1.1.1',
307350
directConnectPort: '8888'
308351
}
309352
}
@@ -352,7 +395,7 @@ def test_default_timeout_for_http_client_with_direct_no_supported_client
352395
appActivity: 'io.appium.android.apis.ApiDemos',
353396
someCapability: 'some_capability',
354397
directConnectProtocol: 'http',
355-
directConnectHost: 'localhost',
398+
directConnectHost: '1.1.1.1',
356399
directConnectPort: '8888',
357400
directConnectPath: '/wd/hub'
358401
}
@@ -517,7 +560,7 @@ def test_attach_to_an_existing_session
517560
appActivity: 'io.appium.android.apis.ApiDemos',
518561
someCapability: 'some_capability',
519562
directConnectProtocol: 'http',
520-
directConnectHost: 'localhost',
563+
directConnectHost: '1.1.1.1',
521564
directConnectPort: '8888',
522565
directConnectPath: '/wd/hub'
523566
}
@@ -527,14 +570,14 @@ def test_attach_to_an_existing_session
527570
stub_request(:post, 'http://127.0.0.1:4723/session')
528571
.to_return(headers: HEADER, status: 200, body: response)
529572

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

534577
driver = core.start_driver
535578

536579
assert_requested(:post, 'http://127.0.0.1:4723/session', times: 1)
537-
assert_requested(:post, 'http://localhost:8888/wd/hub/session/1234567890/timeouts',
580+
assert_requested(:post, 'http://1.1.1.1:8888/wd/hub/session/1234567890/timeouts',
538581
body: { implicit: 30_000 }.to_json, times: 1)
539582
driver
540583
end

0 commit comments

Comments
 (0)