Url checking #148

Trenly · 2022-02-05T05:32:43Z

I've been seeing a few PRs where the URL is redirected. This adds a prompt and a setting to replace URLs which are redirected.
@vedantmgoyal2009 - This might help with your automation, if there is a vanity URL to the latest but it just redirects to a package specific URL

Example if you need one to test with -

OfficialEsco

🤔

Selection: 2

[Required] Enter the Package Identifier, in the following format <Publisher shortname.Application shortname>. For example: Microsoft.Excel
PackageIdentifier: Paessler.PRTGDesktop

[Required] Enter the version. for example: 1.33.7
Version: 22.1.0
Found Existing Version: 21.12.0
Installer Entry #1:

        Architecture: x64
        InstallerType: wix
        Scope: machine


[Required] Enter the download url to the installer.
Url: https://downloads.paessler.com/prtg_desktop/22.1.0/64bit/PRTG_Desktop_Full_Installer.msi


The URL provided appears to be redirected. Would you like to use the destination URL instead?
Discovered URL:
  [Y] Use detected URL
  [N] Use original URL

Enter Choice (default is 'Y'): y
[Warning] URL Changed - The URL was changed during processing and will be re-validated

Test-Url: C:\Users\Esco\GitHub\winget-pkgs\Tools\YamlCreate.ps1:328
Line |
 328 |          if ((Test-Url $NewInstallerUrl) -ne 200) {
     |                        ~~~~~~~~~~~~~~~~
     | Cannot bind argument to parameter 'URL' because it is an empty string.

[Error] Internal Error - Value was not able to be saved successfully

Trenly · 2022-02-05T16:08:36Z

Wut. Thats weird. I'll dig deeper

vedantmgoyal9 · 2022-02-05T16:10:16Z

I can't reproduce @OfficialEsco's issue.

whereas it works as expected...

I'm using:

🚀  $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.22538.1000
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.22538.1000
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Trenly · 2022-02-05T16:12:53Z

I can't reproduce @OfficialEsco's issue.

I'm not able to either. @OfficialEsco - What version of PowerShell are you using?

There may already be a PR for this change. Would you like to continue anyways?
New package: Trenly.Test version test3 - https://github.com/microsoft/winget-pkgs/pull/24579
  [Y] Yes
  [N] No

Enter Choice (default is 'N'): y

[Required] Enter the download url to the installer.
Url: https://downloads.paessler.com/prtg_desktop/22.1.0/64bit/PRTG_Desktop_Full_Installer.msi


Downloading URL. This will take a while...
Time taken: 4 second(s)

Using quick update -

There may already be a PR for this change. Would you like to continue anyways?
Updated Google.Chrome to version 92.0.4515.159 - https://github.com/microsoft/winget-pkgs/pull/25223
  [Y] Yes
  [N] No

Enter Choice (default is 'N'): y
Found Existing Version: 97.0.4692.99
Installer Entry #1:

        Architecture: x64 
        InstallerType: wix
        Scope: machine    


[Required] Enter the download url to the installer.
Url: https://downloads.paessler.com/prtg_desktop/22.1.0/64bit/PRTG_Desktop_Full_Installer.msi
Installer Entry #2:

        Architecture: x86
        InstallerType: wix
        Scope: machine


[Required] Enter the download url to the installer.
Url: https://downloads.paessler.com/prtg_desktop/22.1.0/64bit/PRTG_Desktop_Full_Installer.msi

Yaml file created: D:\Git\winget-pkgs\manifests\g\Google\Chrome\1.0\Google.Chrome.locale.en-US.yaml

Yaml file created: D:\Git\winget-pkgs\manifests\g\Google\Chrome\1.0\Google.Chrome.installer.yaml

Yaml file created: D:\Git\winget-pkgs\manifests\g\Google\Chrome\1.0\Google.Chrome.yaml
Manifest validation succeeded.

OfficialEsco · 2022-02-05T17:17:22Z

[Required] Enter the download url to the installer.
Url: https://downloads.paessler.com/prtg_desktop/22.1.0/64bit/PRTG_Desktop_Full_Installer.msi


The URL provided appears to be redirected. Would you like to use the destination URL instead?
Discovered URL:
  [Y] Use detected URL
  [N] Use original URL

Enter Choice (default is 'Y'): y
   ~\GitHub\winget-pkgs  a725214df5                                                                  18:16:45 
❯ $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.2.1
PSEdition                      Core
GitCommitId                    7.2.1
OS                             Microsoft Windows 10.0.22000
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[Required] Enter the download url to the installer.
Url: https://downloads.paessler.com/prtg_desktop/22.1.0/64bit/PRTG_Desktop_Full_Installer.msi
Installer Entry #2:

        Architecture: x86
        InstallerType: wix
        Scope: machine


[Required] Enter the download url to the installer.
Url:
   ~\GitHub\winget-pkgs  a725214df5                                                                  18:16:41 
❯ $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.22000.282
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.22000.282
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

Trenly · 2022-02-05T18:04:29Z

Aha. Powershell improved their resource handling when they went to v7. In v5 it doesn't dispose of the resource immediately when you close it but it does in v7. Should be fixed in latest commit

OfficialEsco · 2022-02-05T19:57:02Z

There needs to be a space between these lines

The URL provided appears to be redirected. Would you like to use the destination URL instead?
Discovered URL: https://objects.githubusercontent.com/github-production-release-asset-2e65be/248290531/eacfacc2-8493-4c65-b621-3f5ff0eb08c5?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20220205%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220205T195441Z&X-Amz-Expires=300&X-Amz-Signature=87e9ca98230511651dcf483793e4837a5068b2766754a7a952dc7f818a0b5256&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=248290531&response-content-disposition=attachment%3B%20filename%3D64Gram-setup-x64-1.0.13.exe&response-content-type=application%2Foctet-stream
  [Y] Use detected URL
  [N] Use original URL

Enter Choice (default is 'Y'): nInstaller Entry #2:

And there should not be a delay after you press y/n, if you for example paste a url it will throw a error

OfficialEsco · 2022-02-05T20:16:15Z

And there should not be a delay after you press y/n, if you for example paste a url it will throw a error

A easy workaround for this would probably be to implement the Downloading... message/module(for later update)

Also Sourceforge redirects are a problem

[Required] Enter the download url to the installer.
Url: https://sourceforge.net/projects/scidavis/files/SciDAVis/2/2.7/scidavis.2.7-win-dist.msi/download


The URL provided appears to be redirected. Would you like to use the destination URL instead?
Discovered URL: https://netix.dl.sourceforge.net/project/scidavis/SciDAVis/2/2.7/scidavis.2.7-win-dist.msi
  [Y] Use detected URL
  [N] Use original URL

HOWEVER, we cannot detect the ProductCode when its a redirect 🤔

Edit2:
CrystalDiskInfo is also a niece issue but understandable

[Required] Enter the download url to the installer.
Url: https://osdn.net/frs/redir.php?m=auto&f=crystaldiskinfo%2F76657%2FCrystalDiskInfo8_15_0.exe


The URL provided appears to be redirected. Would you like to use the destination URL instead?
Discovered URL: https://osdn.dl.osdn.net/crystaldiskinfo/76657/CrystalDiskInfo8_15_0.exe
  [Y] Use detected URL
  [N] Use original URL

Trenly · 2022-02-05T20:21:40Z

And there should not be a delay after you press y/n, if you for example paste a url it will throw a error

I don't think I can fix the delay here, because that is where it is checking if the URL is valid, and depending on the mode you're running in, downloading and checking the application productcode etc.. What exactly do you mean by pasting a url throwing an error?

A easy workaround for this would probably be to implement the Downloading... message/module(for later update)

Possibly, I'm not sure what the exact error you're seeing is though

Also Sourceforge redirects are a problem

I would argue these aren't a problem, since the second url is more accurate for installer type and will ultimately be faster because it skips all the redirects

OfficialEsco · 2022-02-05T20:27:05Z

Also Sourceforge redirects are a problem

I would argue these aren't a problem, since the second url is more accurate for installer type and will ultimately be faster because it skips all the redirects

We want it to choose the best location for the person downloading so its fast for everyone

And there should not be a delay after you press y/n, if you for example paste a url it will throw a error

I don't think I can fix the delay here, because that is where it is checking if the URL is valid, and depending on the mode you're running in, downloading and checking the application productcode etc.. What exactly do you mean by pasting a url throwing an error?

Actually, i remember the issue now, it was because i pressed Enter, it then threw a error saying the NewInstallerUrl is empty which is understandable

Trenly · 2022-02-05T20:35:54Z

We want it to choose the best location for the person downloading so its fast for everyone

That is a good point; I didn't realize they redirected dynamically. I don't know which is more valuable, the product code or the dynamic cdn. I would think that almost all of their network url's would be fast enough for most places in the world. It isn't the most optimal to use a fixed url for sourceforge, but I think it is probably the better route

OfficialEsco

Ok so, this is approvable, and @vedantmgoyal2009 needs to have a json switch for this UseRedirectedURL: yay/nay
However i'm still not 100% satisfied with how this feels to use #148 (comment)
When you press Y its okay because you have a Write-Host, but when you press N you're left wondering if it does anything

Trenly · 2022-02-11T17:18:36Z

When you press Y its okay because you have a Write-Host, but when you press N you're left wondering if it does anything

I'll take a look at updating the user experience flow here; Hope to have a new commit in ~20 mins

Trenly · 2022-02-11T17:33:46Z

@OfficialEsco Updated with some messaging to be more clear

OfficialEsco

Ok the feedback feels goood

* Prompt user to use direct URL * Formatting * Fix: PS7 Resource Delegation * Ignore github for url checking * Add messaging for User Experience * Fix bug where URI could contain invalid characters

* Url checking (Trenly#148) * Prompt user to use direct URL * Formatting * Fix: PS7 Resource Delegation * Ignore github for url checking * Add messaging for User Experience * Fix bug where URI could contain invalid characters * Match WIX logic to wingetcreate (Trenly#152) * Fix: Don't retain SignatureSha256 between installers * Culture sorting (Trenly#157) * Set culture of current thread while running * Fix Null Value

jedieaston approved these changes Feb 5, 2022

View reviewed changes

OfficialEsco requested changes Feb 5, 2022

View reviewed changes

vedantmgoyal9 approved these changes Feb 5, 2022

View reviewed changes

Trenly requested review from OfficialEsco and jedieaston February 5, 2022 18:08

This comment was marked as off-topic.

Sign in to view

OfficialEsco mentioned this pull request Feb 7, 2022

Check for ProductCode if url does not end with .msi #150

Open

Trenly force-pushed the v2.1.0 branch 2 times, most recently from 708dd1c to e550cef Compare February 11, 2022 00:18

OfficialEsco reviewed Feb 11, 2022

View reviewed changes

Trenly added 5 commits February 11, 2022 11:19

Prompt user to use direct URL

876be06

Formatting

a4044e0

Fix: PS7 Resource Delegation

1430049

Ignore github for url checking

1e49463

Add messaging for User Experience

7f34316

Trenly force-pushed the UrlChecking branch from 2b83879 to 7f34316 Compare February 11, 2022 17:29

Fix bug where URI could contain invalid characters

88fcff7

Trenly force-pushed the v2.1.0 branch from e550cef to f707aec Compare February 11, 2022 17:32

OfficialEsco approved these changes Feb 12, 2022

View reviewed changes

Trenly merged this pull request into v2.1.0 Feb 13, 2022

Trenly deleted the UrlChecking branch February 13, 2022 04:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Url checking #148

Url checking #148

Trenly commented Feb 5, 2022 •

edited

Loading

OfficialEsco left a comment

Trenly commented Feb 5, 2022

vedantmgoyal9 commented Feb 5, 2022 •

edited

Loading

Trenly commented Feb 5, 2022 •

edited

Loading

OfficialEsco commented Feb 5, 2022

Trenly commented Feb 5, 2022

OfficialEsco commented Feb 5, 2022

OfficialEsco commented Feb 5, 2022 •

edited

Loading

Trenly commented Feb 5, 2022

OfficialEsco commented Feb 5, 2022

Trenly commented Feb 5, 2022

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

OfficialEsco left a comment

Trenly commented Feb 11, 2022

Trenly commented Feb 11, 2022

OfficialEsco left a comment

Url checking #148

Url checking #148

Conversation

Trenly commented Feb 5, 2022 • edited Loading

OfficialEsco left a comment

Choose a reason for hiding this comment

Trenly commented Feb 5, 2022

vedantmgoyal9 commented Feb 5, 2022 • edited Loading

whereas it works as expected...

Trenly commented Feb 5, 2022 • edited Loading

OfficialEsco commented Feb 5, 2022

Trenly commented Feb 5, 2022

OfficialEsco commented Feb 5, 2022

OfficialEsco commented Feb 5, 2022 • edited Loading

Trenly commented Feb 5, 2022

OfficialEsco commented Feb 5, 2022

Trenly commented Feb 5, 2022

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

OfficialEsco left a comment

Choose a reason for hiding this comment

Trenly commented Feb 11, 2022

Trenly commented Feb 11, 2022

OfficialEsco left a comment

Choose a reason for hiding this comment

Trenly commented Feb 5, 2022 •

edited

Loading

vedantmgoyal9 commented Feb 5, 2022 •

edited

Loading

Trenly commented Feb 5, 2022 •

edited

Loading

OfficialEsco commented Feb 5, 2022 •

edited

Loading