Skip to content

Revert "Gitlab::UrlBlocker: Ensure absence of URI scheme with `schemes: :none`"

Peter Leitzen requested to merge pl-revert-url-blocker-none-scheme into master

What does this MR do and why?

This reverts commit 77870fca added in !115189 (merged).

According to https://www.ietf.org/rfc/rfc3986.txt URIs without a scheme are invalid. Weirdly, they are accepted by Addressable::URI.parse.

For example:

uri = Addressable::URI.parse("192.168.1.2")
uri.scheme   # => nil
uri.hostname # => nil
uri.path     # => "192.168.1.2

With this it doesn't make sense to enforce URI without a scheme when passing schemes: :none to UrlBlocker as it won't be able to resolve those URI because hostname is nil:

Gitlab::UrlBlocker.validate!("192.168.1.2", schemes: :none)
# Gitlab::UrlBlocker::BlockedUrlError: Host cannot be resolved or invali
# Caused by SocketError: getaddrinfo: Name or service not known

Note that the option schemes: :none isn't used in production and was only added to support !114917 (comment 1320128257) which isn't merged yet.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Peter Leitzen

Merge request reports