jakubgarfield/Bonobo-Git-Server — pull request #61 by DavidAnson (2013-12-19 05:55:24+00:00)
Title: Fix for issue #55 where viewing repository files with a '+' in the path ...

...fails with an IIS error

Problem:
By default, IIS 7 (and later) throws a "HTTP Error 404.11 URL Double Escaped" error if the request URL contains characters that represent an escape sequence (http://support.microsoft.com/kb/942076). This is a site/application-level setting and security measure, so it is preferable not to require a change from the default (more secure) value. The '+' character is a legal path character for files in a GIT repository and triggers the above error when used with Bonobo's Tree/Blob/Raw controllers as part of a path reference.

Discarded approach:
Encode the '+' character in URLs with a custom encoding scheme. This idea was rejected because it is non-standard and the encoding/escape character may conflict with valid path names or fail to include other problematic characters (see below for an example).

Proposed approach:
Encode/decode path fragments with the standard "modified base 64 for URL" encoding (http://en.wikipedia.org/wiki/Base_64). This solves the reported problem with '+' as well as avoids similar URL/encoding problems for other troublesome GIT paths. Specifically, this change solves an unreported problem where viewing a file with a '&' character in the path results in "HttpException (0x80004005): A potentially dangerous Request.Path value was detected from the client (&)." (an exception which is not affected by the requestFiltering/allowDoubleEscaping setting for '+'). The downside of the proposed approach is that the path fragment in the URL is no longer human-readable.

Discarded approach:
A hybrid solution of the previous two, because of the additional complexity and risk of overlooking another problem character.

Implementation:
A new PathEncoder class is added to the Helpers namespace and exposes Encode/Decode methods. The implementation uses .NET's Convert.To/FromBase64String methods plus substitutions for '+' and '/' (per the base 64 convention for URLs), and can be easily changed to another suitable encoding without requiring other changes (because the relevant scenarios all now go through PathEncoder). A corresponding set of unit tests for PathEncoder is added to the test project to verify encode/decode correctness and the absence of double-escape characters in the encoded text.

Routes for the Tree/Blob/Raw actions are updated to use "encodedPath" instead of "path", the corresponding controllers decode the encoded path before using it, and the corresponding links encode the path in their URLs.

changelog.md is updated accordingly.

Note: A similar change to encode the "name" route parameter should fix issue #54; I will look into that if this change gets accepted.