URL encoding 500 error using pull remote mirror

Error occurs entering invalid URLs. We should show a nice message - probably the same as with Push to a remote repository:

mentioned in issue #834 (closed)

added customer label

added reproduced on GitLab.com label

A customer reported experiencing this => https://gitlab.zendesk.com/agent/tickets/82531

Reproduce:

This can be reproduced by entering a bad formatted url to pull mirror from. The logs below where produced from setting the pull mirror url as http://192.168.10.1:1op/test.git

Reproduced this on EE 9.5.4 and 9.5.5 as well as on GitLab.com

Started PATCH "/root/pull-url-test/mirror" for 192.168.10.1 at 2017-09-19 13:23:44 +0000
Processing by Projects::MirrorsController#update as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"[FILTERED]", "project"=>{"mirror"=>"0", "username_only_import_url"=>"[FILTERED]", "import_data_attributes"=>{"ssh_known_hosts"=>"", "auth_method"=>"password", "password"=>"[FILTERED]", "id"=>"2"}, "mirror_user_id"=>"1", "mirror_trigger_builds"=>"0"}, "namespace_id"=>"root", "project_id"=>"pull-url-test"}
Completed 500 Internal Server Error in 218ms (ActiveRecord: 21.3ms | Elasticsearch: 0.0ms)

Addressable::URI::InvalidURIError (Invalid port number: "1op"):
  lib/gitlab/url_sanitizer.rb:63:in `parse_url'
  lib/gitlab/url_sanitizer.rb:27:in `initialize'
  app/models/project.rb:581:in `new'
  app/models/project.rb:581:in `import_url'
  app/models/project.rb:615:in `external_import?'
  app/controllers/projects/mirrors_controller.rb:33:in `update'
  lib/gitlab/i18n.rb:46:in `with_locale'
  lib/gitlab/i18n.rb:52:in `with_user_locale'
  app/controllers/application_controller.rb:334:in `set_locale'
  lib/gitlab/middleware/multipart.rb:93:in `call'
  lib/gitlab/request_profiler/middleware.rb:14:in `call'
  lib/gitlab/middleware/go.rb:17:in `call'
  lib/gitlab/etag_caching/middleware.rb:11:in `call'
  lib/gitlab/middleware/rails_queue_duration.rb:20:in `call'
  lib/gitlab/metrics/rack_middleware.rb:29:in `block in call'
  lib/gitlab/metrics/transaction.rb:49:in `run'
  lib/gitlab/metrics/rack_middleware.rb:29:in `call'
  lib/gitlab/middleware/readonly_geo.rb:30:in `call'
  lib/gitlab/request_context.rb:18:in `call'
  lib/gitlab/metrics/requests_rack_middleware.rb:27:in `call'

added SP2 label

This seems super simple. @DouweM Is this your jurisdiction? This is a support scheduling request. :)

@lbot It should be super simple!

@nick.thomas Didn't you create an issue related to this recently?

@DouweM https://gitlab.com/gitlab-org/gitlab-ce/issues/37448 ?

marked this issue as related to #1715

@nick.thomas I think I was thinking of https://gitlab.com/gitlab-org/gitlab-ee/issues/1715.

I think you mentioned that it's not as easy as it looks because this happens in the import_url setter, not a validation.

/cc @mydigitalself

Hmm. This particular error is actually raised in the getter, import_url

It's currently:

  def import_url
    if import_data && super.present?
      import_url = Gitlab::UrlSanitizer.new(super, credentials: import_data.credentials)
      import_url.full_url
    else
      super
    end
  end

We could change it to be:

  def import_url
    if import_data && super.present?
      import_url = Gitlab::UrlSanitizer.new(super, credentials: import_data.credentials)
      import_url.full_url
    else
      super
    end
  rescue
    super
  end

?

added Platform repository labels

@jramsay 10.3 Deliverable ?

changed milestone to %10.3

added Deliverable label