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

Some uri escape to %2520 instead of %20 #17

Closed
RedBaron2 opened this issue May 4, 2018 · 7 comments
Closed

Some uri escape to %2520 instead of %20 #17

RedBaron2 opened this issue May 4, 2018 · 7 comments
Labels
Bug Priority: High The issue have the highest priority of all issues
Milestone

Comments

@RedBaron2
Copy link
Contributor

RedBaron2 commented May 4, 2018

Get-RedirectedUrl "https://www.dropbox.com/download?build=48.4.58&plat=win&type=full"
Get-RedirectedUrl "https://electron.authy.com/download?channel=stable&arch=x64&platform=win32&version=latest&product=authy"
both of these urls should return the same with spaces escaped to %20, and they don't right now

Expected Behavior

If you're describing a bug, tell us what should happen
When using the powershell command
[uri]::EscapeUriString($url)
in Get-RedirectedUrl helper
the result is
https://clientupdates.dropboxstatic.com/dbx-releng/client/Dropbox%252048.4.58%2520Offline%2520Installer.exe
when it should be
https://clientupdates.dropboxstatic.com/dbx-releng/client/Dropbox%252048.4.58%20Offline%20Installer.exe

Current Behavior

If describing a bug, tell us what happens instead of the expected behavior
The current behavior results in a 403 forbidden error

Possible Solution

The easiest fix is to PR the Get-RedirectedUrl helper to have a switch for EscapeUriString usage
PR submitted with automated check also with regex

Steps to Reproduce (for bugs)

  1. Get-RedirectedUrl "https://www.dropbox.com/download?build=48.4.58&plat=win&type=full"
  2. Get-RedirectedUrl "https://electron.authy.com/download?channel=stable&arch=x64&platform=win32&version=latest&product=authy"

Context

Your Environment

  • Package Version used: Latest
  • Operating System and version: 10.0.17661.1001 build 1803
  • Chocolatey version: N/A
  • Install/uninstall gist: Chocobot
@AdmiringWorm
Copy link
Member

Just some clarification (and a reminder for myself) on what the issue actually is.

The URL from dropbox is already correctly escaped when we get it, as such the % symbol is escaped with (%25).
The Get-RedirectedURL helper should handle such scenarios, and not escape % symbols when followed by numbers (or perhaps unescape the url before escaping again, just in case).

@AdmiringWorm AdmiringWorm added the Priority: High The issue have the highest priority of all issues label May 4, 2018
@RedBaron2
Copy link
Contributor Author

OK I have an idea that might solve the bug
I'll update the PR then

@AdmiringWorm
Copy link
Member

hmm, just playing with a thought here and wanted your input.

Could perhaps add a check to see if the url matches the following \%20 and in those cases we'll consider the url to be already escaped and skip the escaping of the url. Thoughts?

@AdmiringWorm
Copy link
Member

I believe that would be the easiest right now, and could perhaps improve the check over time when needed.

@RedBaron2
Copy link
Contributor Author

Yeah that would be the easiest too, 😄

@AdmiringWorm
Copy link
Member

wait, I was a little quick. I actually meant \%\d+

@AdmiringWorm AdmiringWorm added this to the 0.2.2 milestone May 5, 2018
@AdmiringWorm
Copy link
Member

a new version was pushed to chocolatey now, I'll update the core team repo when it has been approved.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Priority: High The issue have the highest priority of all issues
Projects
None yet
Development

No branches or pull requests

2 participants