Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
09fa2ae
Upgrade uri gem from 0.13.3 to 1.1.1 and fix all incompatible call sites
madnialihussain Jul 24, 2026
361e3b9
Eliminate all RFC2396 references and migrate to RFC3986 for uri gem 1…
madnialihussain Jul 30, 2026
46b5aff
Fix qlty/rubocop issues: PerlBackrefs, ConstantRegexp, EmptyLinesArou…
madnialihussain Jul 30, 2026
47252e4
Fix validation errors after replacing make_regexp with uri: validator
madnialihussain Jul 31, 2026
4cb28f4
fix webhook scenario to test validation error for invalid URL
madnialihussain Aug 12, 2026
8641c06
remove underscore from HOST regex
madnialihussain Aug 12, 2026
4d98455
revert sub chain consolidation in messages_helper
madnialihussain Aug 12, 2026
9f32039
remove double-escaping test from messages_helper_test
madnialihussain Aug 12, 2026
b216d16
add test for + in path for redhat auth callback
madnialihussain Aug 12, 2026
5b5bc85
add service_discovery auth callback referrer URL tests
madnialihussain Aug 12, 2026
794a67c
remove dead i18n :invalid entries for uri-validated fields
madnialihussain Aug 12, 2026
493a7cf
revert inlined patterns to URI::RFC2396 references
madnialihussain Aug 14, 2026
3dd8111
fix rubocop RedundantRegexpEscape for colon in messages_helper
madnialihussain Aug 14, 2026
4b7f655
replace URI.regexp with balanced-bracket regex in linkifier
madnialihussain Aug 14, 2026
6400cf4
extract UriPatterns module for RFC2396 pattern constants
madnialihussain Aug 18, 2026
99b10e6
add whitespace validation for site URL on Custom and Github auth prov…
madnialihussain Aug 18, 2026
e47b127
fix misleading comment in UriPatterns module
madnialihussain Aug 19, 2026
f720960
Remove unnecessary allow_blank from whitespace validators
madnialihussain Aug 19, 2026
2ed9e4c
Add test for sub-delimiter characters in URI validation
madnialihussain Aug 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
8 changes: 1 addition & 7 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,7 @@ gem 'formtastic', '~> 5.0'
gem 'htmlentities', '~>4.3', '>= 4.3.4'
gem 'json', '~> 2.7', '>= 2.7.1'
gem 'responders', '~> 3.2' # For respond_with support
# uri >= 1.0.0 switched the default parser from RFC2396 to RFC3986 (ruby/uri#107),
# removed URI::DEFAULT_PARSER, dropped URI.decode, and the RFC3986 parser has no
# `registry` component. We rely on all of these across models (Proxy, ProxyRule,
# WebHook, AuthenticationProvider) and controllers. Upgrading is worthwhile but
# requires careful inspection of every call-site and should be deployed/tested
# separately from the Faraday update.
gem 'uri', '< 1.0.0'
gem 'uri', '~> 1.0'

gem 'mysql2', '~> 0.5.3'

Expand Down
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -975,7 +975,7 @@ GEM
rack
unicorn
uniform_notifier (1.18.0)
uri (0.13.3)
uri (1.1.1)
useragent (0.16.11)
version_gem (1.1.3)
webmock (3.24.0)
Expand Down Expand Up @@ -1170,7 +1170,7 @@ DEPENDENCIES
uglifier
unicorn
unicorn-rails
uri (< 1.0.0)
uri (~> 1.0)
webmock (~> 3.24.0)
webrick (~> 1.8.2)
will_paginate (~> 3.3)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/provider/admin/redhat/auth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def authentication_provider
def referrer_url
url = params.permit(:referrer)[:referrer]
if url
URI.decode(url)
URI::RFC2396_PARSER.unescape(url)
else
provider_admin_account_path
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ def show
protected

def referrer_url
url = params[:referrer]
url = params.permit(:referrer)[:referrer]
Comment thread
jlledom marked this conversation as resolved.
if url
URI.decode(url)
URI::RFC2396_PARSER.unescape(url)
else
new_admin_service_path
end
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/messages_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def message_subject(message)
def hyperlink_urls(text)
text = h(text)

text.scan(URI.regexp(%w(http https))) do
text.scan(URI::RFC2396_PARSER.make_regexp(%w(http https))) do
#$& contains the whole match of the regural expression
url = $&.sub(/\.$/, '').sub(/\:$/,'')
text = text.sub(url, link_to(url, url))
Expand Down
2 changes: 1 addition & 1 deletion app/lib/three_scale/oauth2/service_discovery_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def query_options
def call
url = super
# OpenShift builtin OAuth has a serious bug, it does not store correctly the redirect_uri
ServiceDiscovery::Config.rh_sso? ? url : URI.decode(url)
ServiceDiscovery::Config.rh_sso? ? url : URI::RFC2396_PARSER.unescape(url)
rescue => e
# Do better error management
Rails.logger.debug("[Openshift OAuth] Error decoding callback URL for builtin <%s>. Error: %s\n%s" % [url.inspect, e.message, e.backtrace.join("\n")])
Expand Down
2 changes: 1 addition & 1 deletion app/models/authentication_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class AuthenticationProvider < ApplicationRecord

validates :client_id, :client_secret, presence: true, if: :oauth_config_required?

with_options format: { with: URI::DEFAULT_PARSER.make_regexp(%w[http https]), allow_blank: true, message: :invalid_url } do |ops|
with_options format: { with: URI::RFC2396_PARSER.make_regexp(%w[http https]), allow_blank: true, message: :invalid_url } do |ops|
ops.validates :site
ops.validates :token_url
ops.validates :authorize_url
Expand Down
4 changes: 2 additions & 2 deletions app/models/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Proxy < ApplicationRecord # rubocop:disable Metrics/ClassLength

validates :error_status_no_match, :error_status_auth_missing, :error_status_auth_failed, :error_status_limits_exceeded, presence: true

uri_pattern = URI::DEFAULT_PARSER.pattern
uri_pattern = URI::RFC2396_PARSER.pattern

URI_OR_LOCALHOST = /\A(https?:\/\/([a-zA-Z0-9._:\/?-])+|.*localhost.*)\Z/
OPTIONAL_QUERY_FORMAT = "(?:\\?(#{uri_pattern.fetch(:QUERY)}))?"
Expand Down Expand Up @@ -675,7 +675,7 @@ def call(attribute)

begin
uri = URI.parse(attribute_value)
value = URI::Generic.new(uri.scheme, uri.userinfo, uri.host, uri.port, uri.registry, uri.path, uri.opaque, uri.query, uri.fragment).to_s
value = URI::Generic.new(uri.scheme, uri.userinfo, uri.host, uri.port, nil, uri.path, uri.opaque, uri.query, uri.fragment).to_s
Comment thread
qltysh[bot] marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 8 issues:

1. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]


2. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]


3. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]


4. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]


5. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]


6. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]


7. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]


8. Proxy::PortGenerator#call refers to 'uri' more than self (maybe move it to another class?) [reek:FeatureEnvy]

@model[attribute] = value unless @model[attribute] == value
rescue URI::InvalidURIError
@model.errors.add(attribute, 'Invalid domain')
Expand Down
10 changes: 5 additions & 5 deletions app/models/proxy_rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ class PatternParser
# ":" | "@" | "&" | "=" | "+" | "$" | ","
param = /
(?:
[#{URI::REGEXP::PATTERN::UNRESERVED}:@&=+,] # note that $ is in the RFC but is removed for our purpposes
[#{URI::RFC2396_REGEXP::PATTERN::UNRESERVED}:@&=+,] # note that $ is in the RFC but is removed for our purpposes
|
#{URI::REGEXP::PATTERN::ESCAPED}
#{URI::RFC2396_REGEXP::PATTERN::ESCAPED}
|
#{REGEX_VARIABLE}
)*
Comment thread
qltysh[bot] marked this conversation as resolved.
Expand All @@ -53,9 +53,9 @@ class PatternParser

query = /
(?:
[#{URI::REGEXP::PATTERN::UNRESERVED}#{URI::REGEXP::PATTERN::RESERVED}]
[#{URI::RFC2396_REGEXP::PATTERN::UNRESERVED}#{URI::RFC2396_REGEXP::PATTERN::RESERVED}]
|
#{URI::REGEXP::PATTERN::ESCAPED}
#{URI::RFC2396_REGEXP::PATTERN::ESCAPED}
|
#{REGEX_LITERAL}=#{REGEX_VARIABLE}
)*
Comment thread
qltysh[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -89,7 +89,7 @@ def call(_)
validates :http_method, inclusion: { in: ALLOWED_HTTP_METHODS }
validate :non_repeated_parameters
validate :no_vars_in_keys
validates :redirect_url, format: URI::DEFAULT_PARSER.make_regexp(%w[http https]), allow_blank: true, length: { maximum: 10000 }
validates :redirect_url, format: URI::RFC2396_PARSER.make_regexp(%w[http https]), allow_blank: true, length: { maximum: 10000 }

def parameters
Addressable::Template.new(path_pattern).variables
Expand Down
2 changes: 1 addition & 1 deletion app/models/web_hook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class WebHook < ApplicationRecord
alias provider account

validates :account_id, presence: true
validates :url, format: { :with => URI::DEFAULT_PARSER.make_regexp(%w[http https]), :if => :active }, length: { maximum: 255 }
validates :url, format: { :with => URI::RFC2396_PARSER.make_regexp(%w[http https]), :if => :active }, length: { maximum: 255 }

#TODO: limit association only to providers?
#TODO validate url as url?
Expand Down