Fix parsing of scheme that are invalid Ruby constant names

[byroot]

This fixes a regression from #26

Error: ActiveRecord::ConnectionAdapters::MergeAndResolveDefaultUrlConfigTest#test_url_with_hyphenated_scheme: NameError: wrong constant name IBM-DB if !uri_class && !const_name.empty? && Schemes.const_defined?(const_name, false) ^^^^^^^^^^^^^^^ /usr/local/lib/ruby/3.1.0/uri/common.rb:94:in `const_defined?' /usr/local/lib/ruby/3.1.0/uri/common.rb:94:in `for' /usr/local/lib/ruby/3.1.0/uri/rfc2396_parser.rb:210:in `parse' /rails/activerecord/lib/active_record/database_configurations/connection_url_resolver.rb:25:in `initialize' /rails/activerecord/lib/active_record/database_configurations/url_config.rb:48:in `new' /rails/activerecord/lib/active_record/database_configurations/url_config.rb:48:in `build_url_hash' /rails/activerecord/lib/active_record/database_configurations/url_config.rb:38:in `initialize' /rails/activerecord/lib/active_record/database_configurations.rb:262:in `new' /rails/activerecord/lib/active_record/database_configurations.rb:262:in `environment_url_config' /rails/activerecord/lib/active_record/database_configurations.rb:253:in `block in merge_db_environment_variables' /rails/activerecord/lib/active_record/database_configurations.rb:250:in `map' /rails/activerecord/lib/active_record/database_configurations.rb:250:in `merge_db_environment_variables' /rails/activerecord/lib/active_record/database_configurations.rb:180:in `build_configs' /rails/activerecord/lib/active_record/database_configurations.rb:19:in `initialize' /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:26:in `new' /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:26:in `resolve_db_config' /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:157:in `test_url_with_hyphenated_scheme' rails test rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:154 

https://buildkite.com/rails/rails/builds/79689#6a1b7cfc-fa17-4752-bc27-62158a228e79/1018-1029

Some symbols such as -, + or . are valid inside URI schemes.
See: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml

@eregon @hsbt

[eregon]

&& !const_name.empty? can be removed with this change.

/\A[A-Z][A-Z0-9_]+\z/.match?(const_name)

Should it be /\A[A-Z]\w*\z/.match?(const_name) rather (allow lowercase letters from the 2nd character, single upper character allowed)?

[byroot]

Definitely, I shouldn't have implemented this this late. Will fix.

[eregon]

It'd be interesting to add a test for register_scheme with a -, . or +.
Probably ends up in an exception, but better than e.g. allowing to register but then the class is not used when parsing.

[byroot]

Yeah, I wasn't sure if the intent was to have register_scheme be a public API or not. My priority was to fix the parsing of arbitrary schemes.

If register is public API, then I'd rather leave it to you to fix it.

[eregon]

I"d consider it public API because it is documented in

uri/lib/uri.rb

Line 33 in 86de79c

# register_scheme 'RSYNC', RSYNC

[byroot]

@eregon should be good to go now. I'd rather handle testing register_scheme in another PR.

[eregon]

Thanks for the fix!

[eregon]

BTW, it's interesting that const_defined? raises NameError when it could return false.
Or that constant names are pretty strict (/\A[A-Z]\w*\z/), when method names are not (can include spaces etc).
But that's how it is in Ruby currently.

[byroot]

Thanks for the merge.

BTW, it's interesting that const_defined? raises NameError when it could return false.

I had the exact same thought. Not sure if worth changing upstream.

On another note, may I request for this changed to be pulled in ruby soonish? It's much harder to spot new breaking changes when we have existing failing tests.

[eregon]

On another note, may I request for this changed to be pulled in ruby soonish? It's much harder to spot new breaking changes when we have existing failing tests.

Done: ruby/ruby@59a65f2