Fix routing for encoded slashes

It seems URI.Path gets unescaped. URI.EscapePath() contains the %2F we want.

Also see https://gitlab.com/gitlab-org/gitlab-ce/issues/4124

cc @ayufan @stanhu

mentioned in merge request !21 (closed)

gitlab-org/gitlab-ce#4124

If the Web server decodes %2F, does this still work?

https://github.com/gitlabhq/gitlabhq/issues/8290#issuecomment-127040461

No. But I think the web server should not do that.

@stanhu what happens in GitLab itself if the %2F is decoded already?

The GitLab API doesn't work when %2F is decoded, but the GitLab Rails routes generally work fine since slashes are not allowed in project or namespace IDs. Therefore, I think this change should be fine for now.

In nginx, the decoding is disabled via the proxy_pass configuration option. In Apache, the AllowEncodedSlashes NoDecode option is used.

However, as this bug report mentions:

AllowEncodedSlashes should really be "on" by default and probably even deprecated. From what I can tell the only thing it protects against is poor application writers and does it in a less-than-graceful way by slapping up a 404.

Unfortunately, we can't do anything about this now unless we deprecate the use of stuffing namespace%2Fproject method of accessing the API (i.e. those identifiers should get their own path so this %2F hackiness is not needed).

@stanhu are you saying that Apache recommends AllowEncodedSlashes on and that GitLab would keep working with that change? If so you could maybe bring this up in gitlab-recipes.

@jacobvosmaer AllowEncodedSlashes on breaks API access using the namespace%2Fproject form, so it's not recommended at this time. I am saying that GitLab should strive to work in either mode (AllowEncodedSlashes on and AllowEncodedSlashes NoDecode), and I've made a number of patches to the main GitLab routes to work this way by decoding the %2F if it is present.

In any case, I think this MR should go in since it's preventing a number of users from being able to use the archive API.

@stanhu OK.

I am still having some doubts about what is the best way to handle URL encoding in workhorse, but this MR adds tests so even if I change my mind we at least know that the archive download thing will keep working.

I am working on another routing fix that should go out in the same version as this.

Status changed to merged

mentioned in commit e2ba5727