lucidsoftware / apt-boto-s3

The fast and simple S3 transport for apt.
Apache License 2.0
56 stars 13 forks source link

Fails to download when secret access key contains slash #9

Open yannh opened 8 years ago

yannh commented 8 years ago

APT update fails when the secret access key contains a slash "/" I believe the issue might be due to parts = self.uri.netloc.split('@', 1)

Not handling slashes properly? Thanks!

yannh commented 8 years ago

Hi @pauldraper ! It seems that urlparse ( https://github.com/lucidsoftware/apt-boto-s3/blob/master/s3.py#L182 ) fails if the secret key contains special characters, such as / or + . Following the recommendation from https://github.com/celery/kombu/issues/221, I tried urlencoding the access key and the secret key - without success. It seems that the characters, even if urlencoded in the list file, are passed non urlencoded to the application :disappointed:

Urlencoding twice, however, works :sunglasses:

A fix would be to urlencode the accesskey and secret key before building the S3Uri object - that might however be quite ugly and duplicate some functionality of the S3Uri class. I can work on a patch, do you have any opinion on this?

pauldraper commented 8 years ago

Yes, it seems that apt percent unescapes the URL before passing it the method..

This doesn't just happen for the user-info, but it happens everywhere else, e.g. http://example.com/foo/a%2Fb/ is passed to the method as http://example.com/foo/a/b/.

I would opt for updating the documentation to urlencode the twice. Kind of dumb, but otherwise I think the URL parsing becomes ambiguous. And it's no different than if someone has special characters in a path prefix (i.e. if I prefix everything the bucket with a%2Fb, I write that as a%25Fb in the apt list file).

pauldraper commented 8 years ago

In fact, apt percent decodes the entire file.

%64eb s3://s3.amazonaws.com/my-bucket jessie main contrib

is deb s3://...

So one level of escaping for putting it in user info; another level for putting it in an apt list file.