New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpUrl encodes `+` to `%20` which makes OkHttp unusable with base64 URLs such as Cloudfront signed URLs #1682

Closed
SalomonBrys opened this Issue Jun 2, 2015 · 8 comments

Comments

2 participants
@SalomonBrys

SalomonBrys commented Jun 2, 2015

We are using Cloudfront to store our images. These images are displayed on our Android app via Picasso which uses OkHTTP.

The problem is that OkHTTP re-encode the URLs, changing + to %20.

HttpUrl.parse("http://test.org/res?signature=a+b=").toString() == "http://test.org/res?signature=a%20b="

As cloudfront URLs contains base64 encoded signatures and because + is one of the 64 characters used by Base64, the fact that OkHttpUrl replaces those + by %20 breaks the signature. Cloudfront therefore shows a "Could not unencode Signature" error.

@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Jun 2, 2015

Member

Yikes.

Member

swankjesse commented Jun 2, 2015

Yikes.

@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Jun 2, 2015

Member

Seems like you probably want base64url. http://en.wikipedia.org/wiki/Base64#URL_applications

Member

swankjesse commented Jun 2, 2015

Seems like you probably want base64url. http://en.wikipedia.org/wiki/Base64#URL_applications

@SalomonBrys SalomonBrys changed the title from OkHttpUrl encodes `+` to `%20` which makes OkHttp unusable with base64 URLs such as Cloudfront signed URLs to HttpUrl encodes `+` to `%20` which makes OkHttp unusable with base64 URLs such as Cloudfront signed URLs Jun 2, 2015

@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Jun 2, 2015

Member

Just curious: does switching to Base64url work in your use case?

Member

swankjesse commented Jun 2, 2015

Just curious: does switching to Base64url work in your use case?

@SalomonBrys

This comment has been minimized.

Show comment
Hide comment
@SalomonBrys

SalomonBrys Jun 3, 2015

This does work:

url = url.replaceAll("\\+", "%2B")

SalomonBrys commented Jun 3, 2015

This does work:

url = url.replaceAll("\\+", "%2B")
@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Jun 6, 2015

Member

The alternative is that in OkHttp 2.5 we don't canonicalize pre-encoded foo+bar to foo%20bar. I'm open to that.

Member

swankjesse commented Jun 6, 2015

The alternative is that in OkHttp 2.5 we don't canonicalize pre-encoded foo+bar to foo%20bar. I'm open to that.

@swankjesse swankjesse added the bug label Jun 6, 2015

@swankjesse swankjesse added this to the 2.5 milestone Jun 6, 2015

@SalomonBrys

This comment has been minimized.

Show comment
Hide comment
@SalomonBrys

SalomonBrys Jun 7, 2015

While I understand the need to encode spaces to %20, what is the reason to encode pluses ?

SalomonBrys commented Jun 7, 2015

While I understand the need to encode spaces to %20, what is the reason to encode pluses ?

@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Aug 25, 2015

Member

The spec requires we encode plus.

Member

swankjesse commented Aug 25, 2015

The spec requires we encode plus.

@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Aug 25, 2015

Member

I'm not going to take action on this; I think the best fix is for you to use base64url.

Member

swankjesse commented Aug 25, 2015

I'm not going to take action on this; I think the best fix is for you to use base64url.

@swankjesse swankjesse closed this Aug 25, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment