Fix sync_uri parsing for paths beginning with file:// #28
Conversation
|
@gentoo/portage, wouldn't it be better to actually use |
|
I don't know if gentoo-portage-dev ML is premoderated, but I've sent this patch to ML a couple of days ago and I still cannot see it in the archive. Is there smth else I should do to get this moving forward? |
|
You need to be subscribed to the mail list or it will be rejected. So if you have multiple email id's/accounts, you need to make sure you send it from the correct id/email account. But I initially agree with mgorny that it is likely better to use urllib to parse the uri than hand parsing and offsets. What are the dis-advantages/cons to using that lib? |
|
I see. Thank you for the info. Should I post this patch to ML then or here is enough? |
|
The mail list is preffered, but here can work also, it just sometimes takes longer for us to get around to it/notice it. |
Portage allows file URLs, i.e. paths beginning with 'file://',
in sync_uri. According to RFC-1738 [1] a file URL must take the form
'file://<host>/foo/bar' or 'file:///foo/bar', when <host> is omitted
(in this case localhost is assumed).
Portage incorrectly parses file URLs because it leaves the second slash
from the 'file://' prefix as a part of the URL. Additionally test suite
incorrectly uses file URLs beginning with 'file:/' instead of 'file://'.
This patch adjusts string offset so that file URLs are parsed correctly:
>>> sync_uri='/foo/bar/baz'
>>> ('file://' + sync_uri)[6:]
'//foo/bar/baz'
>>> ('file://' + sync_uri)[6:] == sync_uri
False
>>> ('file://' + sync_uri)[7:]
'/foo/bar/baz'
>>> ('file://' + sync_uri)[7:] == sync_uri
True
Additionally test suite is updated to use file URLs of the form
'file:///foo/bar' as required by the aforementioned RFC.
[1]: https://tools.ietf.org/html/rfc1738#section-3.10
|
Rebased and repushed to fix merge conflict. |
|
Why not use urlparse, like mgorny suggested? |
|
Sorry, we've been so caught up with things, we've neglected the PR's in here... Is there any chance you can rebase the patch on urlparse? |
I'm not interested. |
Portage allows file URLs, i.e. paths beginning with 'file://',
in sync_uri. According to RFC-1738 1 a file URL must take the form
'file:///foo/bar' or 'file:///foo/bar', when 'host' is omitted
(in this case localhost is assumed).
Portage incorrectly parses file URLs because it leaves the second slash
from the 'file://' prefix as a part of the URL. Additionally test suite
incorrectly uses file URLs beginning with 'file:/' instead of 'file://'.
This patch adjusts string offset so that file URLs are parsed correctly:
Additionally test suite is updated to use file URLs of the form
'file:///foo/bar' as required by the aforementioned RFC.