ENV['RSPEC_ALLOW_INVALID_URLS'] makes unit tests of Gitlab::UrlBlocker difficult
About
Gitlab::UrlBlocker
has a special ENV['RSPEC_ALLOW_INVALID_URLS']
variable for the test environment to silently consider invalid URLs to be valid. It is true
for tests by default.
The comment above the line explains:
# In the test suite we use a lot of mocked urls that are either invalid or
# don't exist. In order to avoid modifying a ton of tests and factories
# we allow invalid urls unless the environment variable RSPEC_ALLOW_INVALID_URLS
# is not true
Problem
When testing Gitlab::UrlBlocker
itself this variable becomes a bit of a difficulty, because by default invalid URIs in the tests always pass .validate!
and .blocked_url?
unless the developer is very careful to set that ENV
var to false
when needed.
This is leading to some false positives in spec/lib/gitlab/url_blocker_spec.rb
.
For example, this test passes because the URI is invalid instead of because the domain resolves to an IP on the allow list. For the test to pass legitimately the URI needs to first not be considered invalid:
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 0d0379847992..a1da2f2f0fb3 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -677,7 +677,7 @@
end
it 'allows domain with port when resolved ip has port allowed' do
- stub_domain_resolv("www.resolve-domain.com", '127.0.0.1') do
+ stub_domain_resolv("www.resolve-domain.com", '127.0.0.1', 2000) do
expect(described_class).not_to be_blocked_url("http://www.resolve-domain.com:2000", **url_blocker_attributes)
end
end
We can flush a lot of these false positives out by setting the ENV var to be false
for all tests in spec/lib/gitlab/url_blocker_spec.rb
:
# In spec/lib/gitlab/url_blocker_spec.rb
before do
stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
end
But now we have the reverse problem, which is that we can have false positives when tests assert the URI should be blocked. It may be being blocked just because the URI is considered invalid, rather than for the reason intended in the test.