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

Add ability to skip aggressive vscode.Uri encoding #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

heroldev
Copy link

@heroldev heroldev commented Jun 24, 2024

This is related to a bug present in mindaro-dev.file-downloader: microsoft/vscode-file-downloader#56, and the relevant pull request to fix it: microsoft/vscode-file-downloader#57

When trying to use this extension to download any file through a URL containing query parameters, the Uri is too aggressively encoded resulting in a 400 Bad Request (and no file downloaded).

A good example:
Original URL: http://localhost:8081/service/rest/v1/search/assets/download?group=org.osgi&name=org.osgi.core&version=4.3.1&maven.extension=jar&maven.classifier
when encoded, becomes
http://localhost:8081/service/rest/v1/search/assets/download?group%3Dorg.osgi%26name%3Dorg.osgi.core%26version%3D4.3.1%26maven.extension%3Djar%26maven.classifier, resulting in a bad request since the query parameters were incorrectly encoded.

This is due to a change in how vscode.Uri.toString() works, where an optional parameter was added that allowed inputted URLs to bypass aggressive encoding (since too many extensions depended on the over-aggressive encoding functionality). However, this breaks query parameters as & and = are encoded, causing URLs to break.

vscode.Uri.toString(true) bypasses this aggressive encoding and allows URLs with query parameters to work.

The issue in specific lies in FileDownloader.ts on line 118:

const downloadStream: Readable = await this._requestHandler.get(
                url.toString(), // right here is the bug, this should be url.toString(true)
                timeoutInMs,
                retries,
                retryDelayInMs,
                headers,
                cancellationToken,
                onDownloadProgressChange
            );

There is already a PR out for fixing this bug: microsoft/vscode-file-downloader#57, however the changes also need to be mirrored to the API so that Typescript programs can take advantage of the new optional parameter. This PR mirrors the necessary changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant