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

Fix: YamlCreate using invalid filename for downloading #29454

Conversation

SpecterShell
Copy link
Contributor

@SpecterShell SpecterShell commented Oct 5, 2021

  • Have you signed the Contributor License Agreement?
  • Have you checked that there aren't other open pull requests for the same manifest update/change?
  • Have you validated your manifest locally with winget validate --manifest <path>?
  • Have you tested your manifest locally with winget install --manifest <path>?
  • Does your manifest conform to the 1.0 schema?

Note: <path> is the name of the directory containing the manifest you're submitting.


The script obtains the file name by slicing InstallerUrl variable and appends it to the destinaiton path, but sometimes the url doesn't contain a valid filename, which may cause errors.
For example, if InstallerUrl is set to "https://pinyin.thunisoft.com/webapi/v1/setup/downloadSetup?setupid=21f0e88683894ebcb440759c5aa25c47&filename=HuayuPY-V7.2.0.213.exe", the script will use "downloadSetup?setupid=21f0e88683894ebcb440759c5aa25c47&filename=HuayuPY-V7.2.0.213.exe" as file name, which contains an invalid file name character "?".
Many servers tell the client the real file name by adding a "ContentDisposition" field to the response header. If existed, the modified code will use it instead.

Microsoft Reviewers: Open in CodeFlow

@wingetbot
Copy link
Collaborator

Service Badge  Service Badge  

@Trenly
Copy link
Contributor

Trenly commented Oct 5, 2021

I'm not familiar with what this code is actually doing? Would you be able to help me understand how it works?

@SpecterShell
Copy link
Contributor Author

SpecterShell commented Oct 5, 2021

I'm not familiar with what this code is actually doing? Would you be able to help me understand how it works?

I updated the code so that it will not invoke web requests twice.
The script obtains the file name by slicing InstallerUrl variable and appends it to the destinaiton path, but sometimes the url doesn't contain a valid filename, which may cause errors.
For example, if InstallerUrl is set to "https://pinyin.thunisoft.com/webapi/v1/setup/downloadSetup?setupid=21f0e88683894ebcb440759c5aa25c47&filename=HuayuPY-V7.2.0.213.exe", the script will use "downloadSetup?setupid=21f0e88683894ebcb440759c5aa25c47&filename=HuayuPY-V7.2.0.213.exe" as file name, which contains an invalid file name character "?".
Many servers tell the client the real file name by adding a "ContentDisposition" field to the response header. If existed, the modified code will use it instead.

@jedieaston
Copy link
Contributor

Why not just name the file {PackageIdentifier}.{PackageVersion}.{extension} like winget does? You'll have to write in what the extension is for each installer type, but that way you don't have to rely on a header. (Or just name it "temp" or something, the extension doesn't really matter for this case I guess).

I'm fine with this, but I'm just afraid if someone didn't configure their web server correctly you could still get a yucky name.

@SpecterShell SpecterShell marked this pull request as draft October 5, 2021 14:31
@Trenly
Copy link
Contributor

Trenly commented Oct 6, 2021

For the purposes of the script, do we even need the file extension other than for determining the installer type?

@jedieaston
Copy link
Contributor

We shouldn't.

@vedantmgoyal9
Copy link
Contributor

I think 🤔 it can be closed (sorry if I am wrong) since a fix will be implemented in v2.0.0 soon.

@SpecterShell SpecterShell deleted the YamlCreate-invalid_filename-fix branch February 27, 2022 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants