Skip to content

Conversation

@surayya-MS
Copy link
Member

Fixes #

Context

Changes Made

Testing

Notes

Mariana Dematte and others added 3 commits April 10, 2025 21:59
…nse headers

DownloadFile should not rely on the remote server response headers. Unless the DestinationFileName task parameter is specified - let's just fallback to the request URI - which is as well the publicly documented behavior

----
#### AI description  (iteration 1)
#### PR Classification
Bug fix

#### PR Summary
This pull request updates the `DownloadFile` task to avoid relying on response headers for determining the file name, instead using the request URI.
- Changes in `src/Tasks/DownloadFile.cs` to use `requestUri` instead of `response` for file name determination.
- Modified method signature and logic in `TryGetFileName` to use `requestUri`.
- Updated call to `TryGetFileName` to pass `uri` instead of `response`.
<!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->
We're doing a version bump so all branches have up-to-date opt-prof runs.

[OptProf data](https://dev.azure.com/devdiv/_apps/hub/ms-vscs-artifact.build-tasks.drop-hub-group-explorer-hub?name=OptimizationData/DotNet-msbuild-Trusted/vs17.12/20250414.8/11397433/1)

----
#### AI description  (iteration 1)
#### PR Classification
Version bump.

#### PR Summary
This pull request updates the version number in the project configuration.
- `eng/Versions.props`: Bumped `VersionPrefix` from `17.12.35` to `17.12.36`.
<!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->
Copilot AI review requested due to automatic review settings May 19, 2025 17:14
@surayya-MS surayya-MS requested a review from a team as a code owner May 19, 2025 17:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR merges tag v17.12.36, bumps the version prefix, and refactors the file-name extraction in DownloadFile to take a URI instead of the full HTTP response.

  • Changed TryGetFileName to accept a Uri and updated its call site.
  • Removed fallback logic for Content-Disposition header when determining file names.
  • Bumped <VersionPrefix> to 17.12.36 in eng/Versions.props.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Tasks/DownloadFile.cs Refactored TryGetFileName signature and logic; removed header-based filename fallback
eng/Versions.props Updated <VersionPrefix> to 17.12.36
Comments suppressed due to low confidence (2)

src/Tasks/DownloadFile.cs:314

  • [nitpick] Add unit tests for TryGetFileName to cover cases with a server-supplied Content-Disposition filename, user-specified DestinationFileName, and URI-only filenames.
private bool TryGetFileName(Uri requestUri, out string filename)

src/Tasks/DownloadFile.cs:326

  • Removing the Content-Disposition fallback means server-provided filenames will no longer be used. Consider restoring response.Content.Headers.ContentDisposition?.FileName before falling back to the URI path.
: Path.GetFileName(requestUri.LocalPath);

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.

3 participants