Remove a check that would break with pinned revision using ssh protocol with a user name in the url

Remove a check that would break with pinned revision using ssh protocol with a user name in the url

An example use case is github over ssh at a pinned revision.

R=petermayo@chromium.org
BUG=115202
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=123325

[Peter Mayo, 2012-02-23 20:32:56]

If you're sure, say you've check and I'll give you "the word".

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py#newcode125
gclient.py:125: return rev_tokens[1]
Is this check correct if we remove the restrictions on the urls?

The url library should handle if file location comes from the right section of
it, but I don't see that right away.

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py#newcode171
gclient.py:171: self._url = self._url.replace('/@', '@')
I think the RFC precludes using slashes in the user name in a URL, so this is
OK, but it has been too long since I last read it.

[Peter Mayo, 2012-02-23 20:49:57]

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py#newcode171
gclient.py:171: self._url = self._url.replace('/@', '@')
On 2012/02/23 20:32:56, Peter Mayo wrote:
> I think the RFC precludes using slashes in the user name in a URL, so this is
> OK, but it has been too long since I last read it.


1738 section 3.1 requires encoding of the @ and / within the user.  I don't see
it precluding an empty user tho.

ssh://@foo.com:2104/funky/@dean  now get mangled, but allowed.

[M-A Ruel, 2012-02-23 20:50:06]

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py#newcode125
gclient.py:125: return rev_tokens[1]
On 2012/02/23 20:32:56, Peter Mayo wrote:
> Is this check correct if we remove the restrictions on the urls?
> 
> The url library should handle if file location comes from the right section of
> it, but I don't see that right away.

In theory, yes, in practice, it's not a problem because this can only fetch from
svn repositories. And I don't care about adding svn over ssh support.

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py#newcode171
gclient.py:171: self._url = self._url.replace('/@', '@')
On 2012/02/23 20:32:56, Peter Mayo wrote:
> I think the RFC precludes using slashes in the user name in a URL, so this is
> OK, but it has been too long since I last read it.

As long as we make sure it doesn't work, it's fine. :)

[M-A Ruel, 2012-02-23 20:51:04]

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py
File gclient.py (right):

https://chromiumcodereview.appspot.com/9443025/diff/1/gclient.py#newcode171
gclient.py:171: self._url = self._url.replace('/@', '@')
On 2012/02/23 20:49:57, Peter Mayo wrote:
> On 2012/02/23 20:32:56, Peter Mayo wrote:
> > I think the RFC precludes using slashes in the user name in a URL, so this
is
> > OK, but it has been too long since I last read it.
> 
> 
> 1738 section 3.1 requires encoding of the @ and / within the user.  I don't
see
> it precluding an empty user tho.
> 
> ssh://@foo.com:2104/funky/@dean  now get mangled, but allowed.
> 

The important part is that it'll fail to sync. Which is fine with me. "Silent
corruption" would annoy me but plain failure is fine.

[Peter Mayo, 2012-02-23 20:51:52]

lgtm

[commit-bot: I haz the power, 2012-02-23 20:54:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/9443025/1

[commit-bot: I haz the power, 2012-02-23 21:01:21]

Change committed as 123325