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.

Sign up for GitHub

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

Jump to bottom

Get-WebFileName Does Not Match on Invalid Characters #753

Closed
dtgm opened this issue May 27, 2016 · 17 comments
Closed

Get-WebFileName Does Not Match on Invalid Characters #753

dtgm opened this issue May 27, 2016 · 17 comments
Assignees
@ferventcoder
Labels
5 - Released Bug Priority_HIGH
Milestone
0.9.10

Comments

@dtgm
Copy link
Contributor

dtgm commented May 27, 2016

What You Are Seeing?

Install-Chocolatey(Zip)Package.ps1 sets switch -getOriginalFileName when calling Get-ChocolateyWebFile.ps1 so this code gets executed

https://github.com/chocolatey/choco/blob/master/src/chocolatey.resources/helpers/functions/Get-ChocolateyWebFile.ps1#128

And then Get-WebFileName which fails when trying to set filename for https://dl.dropboxusercontent.com/u/..../somefile.zip

I verified the issue is at least in that block by removing -getOriginalFileName parameter from https://github.com/chocolatey/choco/blob/master/src/chocolatey.resources/helpers/functions/Install-ChocolateyZipPackage.ps1#L103

How Did You Get This To Happen? (Steps to Reproduce)

choco install suwatchapp -ydv

Packages affected colordesker, deskpins, suwatchapp:

ColorDesker.2.2.611.1122/tools/chocolateyInstall.ps1:Install-ChocolateyPackage "ColorDesker" "exe" "/silent" "https://dl.dropboxusercontent.com/u/36711/ColorDeskerSetup.exe" -validExitCodes @(0)
ColorDesker.2.2.611.1122/tools/install.ps1:Install-ChocolateyPackage "ColorDesker" "exe" "/silent" "https://dl.dropboxusercontent.com/u/36711/ColorDeskerSetup.exe" -validExitCodes @(0)
deskpins.1.32/tools/chocolateyInstall.ps1:$url = 'https://dl.dropboxusercontent.com/u/3885603/releases/DeskPins-1.32-setup.exe'
suwatchapp.0.1.0.0/tools/chocolateyInstall.ps1:  Install-ChocolateyZipPackage 'suwatchapp' 'https://dl.dropboxusercontent.com/u/109652660/MiscTools.zip' "$(Split-Path -parent $MyInvocation.MyCommand.Definition)"

Output Log

Running licensed 'Get-WebFileName' with url:'https://dl.dropboxusercontent.com/u/109652660/MiscTools.zip', defaultName:'suwatchappInstall.zip' userAgent:'chocolatey command line'
Setting the UserAgent to 'chocolatey command line'
Using header 'Content-Disposition' (inline; filename="MiscTools.zip"; filename*=UTF-8''MiscTools.zip) to determine file name.
File name determined from url is 'MiscTools.zip; filename*=UTF-8''MiscTools.zip'
Attempt to use original download file name failed for 'https://dl.dropboxusercontent.com/u/109652660/MiscTools.zip'.
Running licensed 'Get-WebHeaders' with url:'https://dl.dropboxusercontent.com/u/109652660/MiscTools.zip', userAgent:'chocolatey command line'

https://gist.github.com/dtgm/a67a74d9c2a150dd1cc126ac8c71912c#file-cinst-suwatchapp-ydv-log-L261-L263
@dtgm
Copy link
Contributor Author

dtgm commented May 27, 2016

Error block from same gist as above https://gist.github.com/dtgm/a67a74d9c2a150dd1cc126ac8c71912c#file-cinst-suwatchapp-ydv-log-L287-L292

ERROR: Exception calling ".ctor" with "1" argument(s): "Illegal characters in path."
 at Get-ChocolateyWebFile, C:\ProgramData\chocolatey\helpers\functions\Get-ChocolateyWebFile.ps1: line 170
at Install-ChocolateyZipPackage, C:\ProgramData\chocolatey\helpers\functions\Install-ChocolateyZipPackage.ps1: line 103
at <ScriptBlock>, C:\ProgramData\chocolatey\lib\suwatchapp\tools\chocolateyInstall.ps1: line 9
at <ScriptBlock>, C:\ProgramData\chocolatey\helpers\chocolateyScriptRunner.ps1: line 48
at <ScriptBlock>, <No file>: line 1

@ferventcoder
Copy link
Member

@dtgm two things - I need to know what version of choco and what version of choco licensed

@ferventcoder
Copy link
Member

ferventcoder commented May 27, 2016 •
edited
Loading

It looks like this is similar if not the same as #727. Which was fixed in a recent beta, and then also applied in licensed 1.3.1.

@ferventcoder
Copy link
Member

Definitely a different bug

curl -I https://dl.dropboxusercontent.com/u/109652660/MiscTools.zip
HTTP/1.1 200 OK
Server: nginx
Date: Fri, 27 May 2016 23:40:36 GMT
Content-Type: application/zip
Content-Length: 1216986
Connection: keep-alive
x-robots-tag: noindex, nofollow, noimageindex
content-disposition: inline; filename="MiscTools.zip"; filename*=UTF-8''MiscTools.zip
set-cookie: uc_session=gYTBfVGa4p2OzgdPF4djdGNOCLxhSxS22nbT5Mu1b6EEN1VaI9mNMZUaWmklQXSK; Domain=dropboxusercontent.com; httponly; Path=/; secure
accept-ranges: bytes
content-security-policy: referrer no-referrer
etag: 46n
x-dropbox-request-id: 8f0880d28e08617b1d6df7c566465128
pragma: public
cache-control: max-age=0
x-content-security-policy: referrer no-referrer
x-webkit-csp: referrer no-referrer
X-Server-Response-Time: 175

@ferventcoder ferventcoder self-assigned this May 27, 2016
@ferventcoder ferventcoder added this to the 0.9.10 milestone May 27, 2016
@dtgm
Copy link
Contributor Author

dtgm commented May 27, 2016

PS C:\WINDOWS\system32> choco list -e chocolatey
Chocolatey v0.9.10-beta1-352-gc443f48 Professional
chocolatey 0.9.9.12 [Approved]

@ferventcoder
Copy link
Member

What I don't get yet is why filename*= is matching with filename=

@ferventcoder
Copy link
Member

Oh wait, nevermind - File name determined from url is 'MiscTools.zip; filename*=UTF-8''MiscTools.zip'

@ferventcoder
Copy link
Member

even went through the illegal characters check and nada. Hmmm

@dtgm
Copy link
Contributor Author

dtgm commented May 27, 2016

https://msdn.microsoft.com/en-us/library/system.io.path.getinvalidpathchars(v=vs.110).aspx doesn't look at all chars

@dtgm
Copy link
Contributor Author

dtgm commented May 27, 2016

It doesn't seem to be checking * which is the only invalid char in that string.

@ferventcoder
Copy link
Member

ferventcoder commented May 27, 2016 •
edited
Loading

We are looking for Path.GetInvalidFileNameChars() - which also doesn't have that. https://msdn.microsoft.com/en-us/library/system.io.path.getinvalidfilenamechars(v=vs.110).aspx

@dtgm
Copy link
Contributor Author

dtgm commented May 27, 2016

oops, right.

@ferventcoder
Copy link
Member

I stand corrected, it is there. It's not picking it up though....

@ferventcoder ferventcoder changed the title Get-WebFileName causes package to fail when using dropbox URLs Get-WebFileName Does Not Match on Invalid Characters May 28, 2016
@ferventcoder
Copy link
Member

expresso picks it up just fine. I've even set the options the same...still no match. Going to need to dive deeper on this one.

@dtgm
Copy link
Contributor Author

dtgm commented May 28, 2016

I think it's because you broke the regex engine. You can't IgnorePatternWhitespace within a character class so that's probably making things go 💥 Nothing will match. Omitting the option will work, but then spaces would also match as a bad char. So omit the option and remove the spaces before creating the regex char class.

replace
new-object regex("["+[System.Text.RegularExpressions.Regex]::Escape([System.IO.Path]::GetInvalidFileNameChars())+"]", [System.Text.RegularExpressions.RegexOptions]::IgnorePatternWhitespace)

with
new-object regex("["+[System.Text.RegularExpressions.Regex]::Escape([System.IO.Path]::GetInvalidFileNameChars() -join '')+"]")

@ferventcoder
Copy link
Member

That join is exactly what I was doing last night to remove spaces 👍

@ferventcoder
Copy link
Member

The fixes here are as you mentioned - remove the whitespace by doing a join and remove the option for IgnorePatternWhitespace. Additionally, at each step of attempting to determine the file name we determine if what we got back was valid or if the next check should be attempted.

ferventcoder added a commit that referenced this issue May 28, 2016
@ferventcoder
(GH-753) Get-WebFileName Matches on Invalid Chars
d50c812 
Previously, the regex used to determine whether invalid characters were
used or not used the option `IgnorePatternWhitespace` and included
spaces in the way that it returned the values. This actually caused it
to match on almost nothing.

Instead, remove the whitespace when joining
`Path.GetInvalidFileNameChars` and additionally add `=;`. While equals
and semi-colon are valid for file name, they are indicative of an issue
with the file name.

At every attempt to determine the file name, run the regex to determine
if what was determined was good to go or if the next check needs to
run. This allows as many of the checks to occur as necessary to
determine the file name from the remote location.

If any method of attempting to get the file name fails, fall back to
the default filename passed into the method.
ferventcoder added a commit that referenced this issue May 28, 2016
@ferventcoder
Merge branch 'stable'
fc27200 
* stable:
  (doc) update CHANGELOG/nuspec
  (GH-753) Get-WebFileName Matches on Invalid Chars
  (doc) update CHANGELOG/nuspec
  (GH-356) Resolve sources by name
  (GH-732) Request/Response Timeout configurable
  (maint) formatting
  (GH-751) Use package name/version from env vars
  (GH-752) Combine Push timeout and execution timeout
  (GH-622) Remove NuGet temp folders
  (GH-622) Allow silent retries
  (GH-584) License loading occurs in DLL
  (GH-584) update licensing docs
  (doc) update CHANGELOG/nuspec
  (GH-733) Don't pass some args to dependencies
  (doc) update CHANGELOG/nuspec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ferventcoder ferventcoder

Labels
5 - Released Bug Priority_HIGH
Projects
None yet
Milestone
0.9.10
Development

No branches or pull requests
3 participants
@ferventcoder @gep13 @dtgm