Skip to content
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

Don't double encode GitHub download URLs #2402

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

HebaruSan
Copy link
Member

Problem

Netkan currently is double-encoding GitHub download URLs, which causes any URL that has an encoded character to fail. See #2401 for details.

Cause

The GitHub API returns the URL already encoded:

        "browser_download_url": "https://github.com/jrodrigv/DestructionEffects/releases/download/v1.8%2C0/DestructionEffects.1.8.0_0412018.zip"

We then encode it again, which changes any % characters to %25, which makes the URL fail with a 404 status:

json.SafeAdd("download", Uri.EscapeUriString(ghRelease.Download.ToString()));

Changes

The Uri.EscapeUriString call is removed. The DestructionEffects netkan succeeds with this change.

Fixes #2401.

@HebaruSan HebaruSan added Bug Something is not working as intended Pull request Netkan Issues affecting the netkan data Network Issues affecting internet connections of CKAN and removed Bug Something is not working as intended Network Issues affecting internet connections of CKAN labels Apr 9, 2018
@HebaruSan
Copy link
Member Author

HebaruSan commented Apr 9, 2018

The DestructionEffects author has fixed the problem upstream, but I did test this fix before that:

$ ls -gG
-rw-r--r-- 1 1039 Apr  9 18:15 DestructionEffects-2-v1.8,0.ckan

@HebaruSan HebaruSan added Bug Something is not working as intended Network Issues affecting internet connections of CKAN labels Apr 9, 2018
@politas politas merged commit 7894581 into KSP-CKAN:master Apr 11, 2018
politas added a commit that referenced this pull request Apr 11, 2018
@politas politas removed Bug Something is not working as intended Network Issues affecting internet connections of CKAN Pull request labels Apr 11, 2018
@HebaruSan HebaruSan deleted the fix/github-url-double-encoding branch April 11, 2018 15:51
@HebaruSan HebaruSan mentioned this pull request Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Netkan Issues affecting the netkan data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants