Skip to content

Commit

Permalink
Finally fix token-in-URI deprecation notice.
Browse files Browse the repository at this point in the history
The original fix had to be reverted because it broke release asset
downloads (see #976). Reimplement the original fix and add a workaround
for release asset downloads which was suggested by GH support.

Fixes #972
  • Loading branch information
maniac103 committed Dec 15, 2020
1 parent 97cabff commit b5ec77e
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
return true;
case R.id.download:
UiUtils.enqueueDownloadWithPermissionCheck(this, buildRawFileUrl(),
FileUtils.getMimeTypeFor(mPath), FileUtils.getFileName(mPath), null, null,
UiUtils.DownloadTokenHandling.UseAuthHeader);
FileUtils.getMimeTypeFor(mPath), FileUtils.getFileName(mPath), null);
return true;
case R.id.share:
IntentUtils.share(this, getString(R.string.share_file_subject,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
return true;
case R.id.download:
UiUtils.enqueueDownloadWithPermissionCheck(this, mGistFile.rawUrl(),
mGistFile.type(), mGistFile.filename(), null, null,
UiUtils.DownloadTokenHandling.None);
mGistFile.type(), mGistFile.filename(), null);
}
return super.onOptionsItemSelected(item);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,7 @@ private void fillNotes(Optional<String> bodyHtmlOpt) {

@Override
public void onItemClick(ReleaseAsset item) {
UiUtils.enqueueDownloadWithPermissionCheck(this, item.url(),
item.contentType(), item.name(), item.label(),
"application/octet-stream", UiUtils.DownloadTokenHandling.AppendToUri);
UiUtils.enqueueDownloadWithPermissionCheck(this, item);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
.appendPath(getCurrentRef())
.toString();
UiUtils.enqueueDownloadWithPermissionCheck(this, zipUrl, "application/zip",
mRepoName + "-" + getCurrentRef() + ".zip", null, null,
UiUtils.DownloadTokenHandling.UseAuthHeader);
mRepoName + "-" + getCurrentRef() + ".zip", null);
return true;
}
}
Expand Down
3 changes: 1 addition & 2 deletions app/src/main/java/com/gh4a/fragment/ContentListFragment.java
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ public boolean onContextItemSelected(MenuItem item) {
mRepository.name(), mRef, contents.path());
UiUtils.enqueueDownloadWithPermissionCheck(getBaseActivity(),
url, FileUtils.getMimeTypeFor(contents.name()),
contents.name(), null, null,
UiUtils.DownloadTokenHandling.UseAuthHeader);
contents.name(), null);
return true;
}

Expand Down
17 changes: 4 additions & 13 deletions app/src/main/java/com/gh4a/fragment/EventListFragment.java
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,7 @@ public void onItemClick(GitHubEvent event) {
case DownloadEvent: {
DownloadPayload payload = (DownloadPayload) event.payload();
Download download = payload.download();
UiUtils.enqueueDownloadWithPermissionCheck((BaseActivity) getActivity(),
download.htmlUrl(), download.contentType(),
download.name(), download.description(), null,
UiUtils.DownloadTokenHandling.None);
UiUtils.enqueueDownloadWithPermissionCheck((BaseActivity) getActivity(), download);
break;
}

Expand Down Expand Up @@ -484,18 +481,12 @@ public boolean onContextItemSelected(MenuItem item) {
if (event.type() == GitHubEventType.ReleaseEvent) {
ReleasePayload payload = (ReleasePayload) event.payload();
ReleaseAsset asset = payload.release().assets().get(id - MENU_DOWNLOAD_START);
UiUtils.enqueueDownloadWithPermissionCheck((BaseActivity) getActivity(),
asset.url(), asset.contentType(),
asset.name(), asset.label(), "application/octet-stream",
UiUtils.DownloadTokenHandling.AppendToUri);
UiUtils.enqueueDownloadWithPermissionCheck((BaseActivity) getActivity(), asset);
} else if (event.type() == GitHubEventType.DownloadEvent) {
DownloadPayload payload = (DownloadPayload) event.payload();
Download download = payload.download();
UiUtils.enqueueDownloadWithPermissionCheck((BaseActivity) getActivity(),
download.htmlUrl(), download.contentType(),
download.name(), download.description(), null,
UiUtils.DownloadTokenHandling.None);
}
UiUtils.enqueueDownloadWithPermissionCheck((BaseActivity) getActivity(), download);
}
return true;
}

Expand Down
102 changes: 75 additions & 27 deletions app/src/main/java/com/gh4a/utils/UiUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,24 @@
import com.gh4a.BaseActivity;
import com.gh4a.Gh4Application;
import com.gh4a.R;
import com.gh4a.ServiceFactory;
import com.gh4a.widget.IssueLabelSpan;
import com.meisolsson.githubsdk.model.Download;
import com.meisolsson.githubsdk.model.Label;
import com.meisolsson.githubsdk.model.ReleaseAsset;

import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.List;

import okhttp3.Call;
import okhttp3.Callback;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;

public class UiUtils {
public static void hideImeForView(View view) {
if (view == null) {
Expand Down Expand Up @@ -238,55 +248,93 @@ private static void enqueueDownload(Context context, Uri uri, String fileName,
dm.enqueue(request);
}

public enum DownloadTokenHandling {
None,
AppendToUri,
UseAuthHeader
public static void enqueueDownloadWithPermissionCheck(final BaseActivity activity,
final Download download) {
enqueueDownloadWithPermissionCheck(activity, download.htmlUrl(), download.contentType(),
download.name(), download.description());
}

public static void enqueueDownloadWithPermissionCheck(final BaseActivity activity,
final String url, final String mimeType, final String fileName,
final String description, final String mediaType,
final DownloadTokenHandling tokenHandling) {
final ReleaseAsset asset) {
final ActivityCompat.OnRequestPermissionsResultCallback cb =
(requestCode, permissions, grantResults) -> {
if (grantResults[0] == PackageManager.PERMISSION_GRANTED) {
enqueueDownload(activity, url, mimeType, fileName,
description, mediaType, tokenHandling);
enqueueDownload(activity, asset);
}
};
activity.requestPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE, cb,
R.string.download_permission_rationale);
}

private static void enqueueDownload(final Context context, String url, final String mimeType,
final String fileName, final String description,
final String mediaType, final DownloadTokenHandling tokenHandling) {
if (url == null) {
return;
public static void enqueueDownloadWithPermissionCheck(final BaseActivity activity,
final String url, final String mimeType, final String fileName, final String description) {
final ActivityCompat.OnRequestPermissionsResultCallback cb =
(requestCode, permissions, grantResults) -> {
if (grantResults[0] == PackageManager.PERMISSION_GRANTED) {
enqueueDownload(activity, url, fileName, description, mimeType, null, false);
}
};
activity.requestPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE, cb,
R.string.download_permission_rationale);
}

private static void enqueueDownload(final Context context, final ReleaseAsset asset) {
// Ugly workaround for #972 (see #976 for analysis), suggested by GH support:
// "First, you make an API call to the endpoint for fetching an asset and you pass in the
// token via the Authorization header. You make this call using an HTTP library (not via
// the Android Download Manager) and you disable automatic following of redirects when you
// make that call (in case that's enabled by default). The result of that call will be a
// redirect response with a Location header.
// Second, you use the Android Download Manager to download the asset from the URL that's
// provided in the Location header from the response of the first step. You would not
// provide an Authorization header here since the required authorization is already a
// part of the URL."
final OkHttpClient client = ServiceFactory.getHttpClientBuilder()
.followRedirects(false)
.build();
final Request.Builder requestBuilder = new Request.Builder()
.url(asset.url())
.header("Accept", "application/octet-stream");
final String token = Gh4Application.get().getAuthToken();
if (token != null) {
requestBuilder.addHeader("Authorization", "Token " + token);
}

final Uri baseUri = Uri.parse(url);
final Uri uri;
if (tokenHandling == DownloadTokenHandling.AppendToUri) {
uri = baseUri.buildUpon()
.appendQueryParameter("access_token", Gh4Application.get().getAuthToken())
.build();
} else {
uri = baseUri;
client.newCall(requestBuilder.build()).enqueue(new Callback() {
private void completeDownload(final String url) {
enqueueDownload(context, url, asset.name(), asset.label(), asset.contentType(),
"application/octet-stream", false);
}
@Override
public void onFailure(Call call, IOException e) {
completeDownload(asset.url());
}

@Override
public void onResponse(Call call, Response response) {
completeDownload(response.isRedirect()
? response.header("Location")
: call.request().url().toString());
}
});
}

private static void enqueueDownload(final Context context, String url, final String fileName,
final String description, final String mimeType,
final String mediaType, final boolean addAuthHeader) {
if (url == null) {
return;
}
final boolean addAuthHeader = tokenHandling == DownloadTokenHandling.UseAuthHeader;

final Uri uri = Uri.parse(url);
if (!downloadNeedsWarning(context)) {
enqueueDownload(context, uri, fileName, description, mimeType,
mediaType, false, addAuthHeader);
enqueueDownload(context, uri, fileName, description, mimeType, mediaType, false, addAuthHeader);
return;
}

DialogInterface.OnClickListener buttonListener = (dialog, which) -> {
boolean wifiOnly = which == DialogInterface.BUTTON_NEUTRAL;
enqueueDownload(context, uri, fileName, description,
mimeType, mediaType, wifiOnly, addAuthHeader);
enqueueDownload(context, uri, fileName, description, mimeType, mediaType, wifiOnly, addAuthHeader);
};

new AlertDialog.Builder(context)
Expand Down

0 comments on commit b5ec77e

Please # to comment.