Change to tilde escaping causes test failures

On 06/09/11 01:19, Martin [gz] wrote:
> Public bug reported:
>
> In r6079 url handling was changed to start escaping tildes to %7E, which
> was noted as suboptimal in review:
>
> <https://code.launchpad.net/~jelmer/bzr/transport-segments/+merge/71583>
Please note that the issue that was pointed out by the reviewer is
already fixed in bzr.dev, and is different from what you're pointing out
here. That was about displaying "locations", which is different from the
representation we use internally anyway.

Cheers,

Jelmer

> There is no reason to escape tildes, and not being consistent about escaping is dangerous.
rfc1738 considers tilde one of the unsafe characters and says that all unsafe characters must always be encoded within a URL.
(page 30)

> Please note that the issue that was pointed out by the reviewer is
> already fixed in bzr.dev, and is different from what you're pointing out
> here. That was about displaying "locations", which is different from the
> representation we use internally anyway.

I put that in the bug report not because I thought anyone had dropped the ball, just that it was relevant that the change was deliberate and not an oversight.

> rfc1738 considers tilde one of the unsafe characters and says that all
> unsafe characters must always be encoded within a URL.
> (page 30)

And in rfc3986 it's an unreserved character, hence the endless confusion over tildes in urls. :)

It also calls out the problem with comparing equivalent but unequal values, and makes a recommendation:

<http://www.ietf.org/rfc/rfc3986.txt>

   URIs that differ in the replacement of an unreserved character with
   its corresponding percent-encoded US-ASCII octet are equivalent: they
   identify the same resource. However, URI comparison implementations
   do not always perform normalization prior to comparison (see Section
   6). For consistency, percent-encoded octets in the ranges of ALPHA
   (%41-%5A and %61-%7A), DIGIT (%30-%39), hyphen (%2D), period (%2E),
   underscore (%5F), or tilde (%7E) should not be created by URI
   producers and, when found in a URI, should be decoded to their
   corresponding unreserved characters by URI normalizers.

On 06/09/11 02:15, Martin [gz] wrote:
>> Please note that the issue that was pointed out by the reviewer is
>> already fixed in bzr.dev, and is different from what you're pointing out
>> here. That was about displaying "locations", which is different from the
>> representation we use internally anyway.
> I put that in the bug report not because I thought anyone had dropped
> the ball, just that it was relevant that the change was deliberate and
> not an oversight.
But it wasn't a deliberate decision to start escaping tildes. The
decision was about normalizing URLs. Various bits of the code (in
particular Transport._unsplit_url) have been quoting tildes forever.

>> rfc1738 considers tilde one of the unsafe characters and says that all
>> unsafe characters must always be encoded within a URL.
>> (page 30)
> And in rfc3986 it's an unreserved character, hence the endless confusion
> over tildes in urls. :)
Ah, thanks for that pointer. I guess I should get with the times and
stop pretending it's still 1994 :)

Cheers,

Jelmer

Two Gio tests also started failing if there are tildes involved:

 bzrlib.tests.per_transport.TransportTests.test_abspath(GioTransport,GioLocalURLServer)
 bzrlib.tests.per_transport.TransportTests.test_clone(GioTransport,GioLocalURLServer)

Traceback (most recent call last):
  ...
  File ".../bzrlib/tests/per_transport.py", line 1399, in test_abspath
    transport.abspath('relpath'))
AssertionError: not equal:
a = 'gio+file:/tmp/cuddle~/testbzr-4Jw5xY.tmp/bzrlib.tests.per_transport.TransportTests.test_abspath%28GioTransport%2CGioLocalURLServer%29/work/relpath'
b = 'gio+file:/tmp/cuddle%7E/testbzr-4Jw5xY.tmp/bzrlib.tests.per_transport.TransportTests.test_abspath%28GioTransport%2CGioLocalURLServer%29/work/relpath'