-
Notifications
You must be signed in to change notification settings - Fork 644
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 package version to query string for v2 CDN endpoint (#10082) #10083
Changes from 5 commits
0a1251c
15e626c
030504e
e5c703f
b3ccb52
7a6e130
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,212 @@ | ||
; EditorConfig to support per-solution formatting. | ||
; Use the EditorConfig VS add-in to make this work. | ||
; http://editorconfig.org/ | ||
|
||
; This is the default for the codeline. | ||
root = true | ||
|
||
[*] | ||
; Don't use tabs for indentation. | ||
indent_style = space | ||
; (Please don't specify an indent_size here; that has too many unintended consequences.) | ||
charset = utf-8 | ||
trim_trailing_whitespace = true | ||
insert_final_newline = true | ||
|
||
; Code files | ||
[*.{cs}] | ||
indent_size = 4 | ||
|
||
; All XML-based file formats | ||
[*.{config,csproj,nuspec,props,resx,ruleset,targets,vsct,vsixmanifest,xaml,xml,vsmanproj,swixproj}] | ||
indent_size = 2 | ||
|
||
; JSON files | ||
[*.json] | ||
indent_size = 2 | ||
|
||
; PowerShell scripts | ||
[*.{ps1}] | ||
indent_size = 4 | ||
|
||
[*.{sh}] | ||
indent_size = 4 | ||
|
||
; Dotnet code style settings | ||
[*.{cs,vb}] | ||
; Sort using and Import directives with System.* appearing first | ||
dotnet_sort_system_directives_first = true | ||
dotnet_separate_import_directive_groups = false | ||
|
||
; IDE0003 Avoid "this." and "Me." if not necessary | ||
dotnet_style_qualification_for_field = false:silent | ||
dotnet_style_qualification_for_property = false:silent | ||
dotnet_style_qualification_for_method = false:silent | ||
dotnet_style_qualification_for_event = false:silent | ||
|
||
; IDE0012 Use language keywords instead of framework type names for type references | ||
dotnet_style_predefined_type_for_locals_parameters_members = true:silent | ||
; IDE0013 | ||
dotnet_style_predefined_type_for_member_access = true:silent | ||
|
||
; Suggest more modern language features when available | ||
dotnet_style_object_initializer = true:suggestion | ||
dotnet_style_collection_initializer = true:suggestion | ||
dotnet_style_explicit_tuple_names = true:suggestion | ||
dotnet_style_coalesce_expression = true:suggestion | ||
dotnet_style_null_propagation = true:suggestion | ||
|
||
; Licence header | ||
file_header_template = Copyright (c) .NET Foundation. All rights reserved.\nLicensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
; CSharp code style settings | ||
dotnet_style_prefer_simplified_boolean_expressions = true:suggestion | ||
dotnet_style_prefer_conditional_expression_over_return = true:silent | ||
dotnet_style_prefer_conditional_expression_over_assignment = true:silent | ||
dotnet_style_prefer_inferred_tuple_names = true:suggestion | ||
dotnet_style_prefer_inferred_anonymous_type_member_names = true:suggestion | ||
dotnet_style_prefer_compound_assignment = true:suggestion | ||
dotnet_style_prefer_simplified_interpolation = true:suggestion | ||
dotnet_style_namespace_match_folder = true:suggestion | ||
dotnet_style_prefer_collection_expression = when_types_loosely_match:suggestion | ||
dotnet_style_readonly_field = true:suggestion | ||
dotnet_style_require_accessibility_modifiers = for_non_interface_members:silent | ||
dotnet_style_allow_statement_immediately_after_block_experimental = true:silent | ||
dotnet_style_allow_multiple_blank_lines_experimental = true:silent | ||
dotnet_code_quality_unused_parameters = all:suggestion | ||
dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity:silent | ||
dotnet_style_parentheses_in_other_binary_operators = always_for_clarity:silent | ||
dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity:silent | ||
dotnet_style_parentheses_in_other_operators = never_if_unnecessary:silent | ||
dotnet_style_prefer_auto_properties = true:silent | ||
dotnet_style_prefer_is_null_check_over_reference_equality_method = true:suggestion | ||
dotnet_style_operator_placement_when_wrapping = beginning_of_line | ||
end_of_line = crlf | ||
indent_size = 4 | ||
tab_width = 4 | ||
[*.cs] | ||
; IDE0007 'var' preferences | ||
csharp_style_var_for_built_in_types = true:silent | ||
csharp_style_var_when_type_is_apparent = true:silent | ||
csharp_style_var_elsewhere = false:silent | ||
|
||
; Helpful errors | ||
dotnet_diagnostic.CA2017.severity = error | ||
dotnet_diagnostic.CS0105.severity = error | ||
dotnet_diagnostic.IDE0005.severity = error | ||
dotnet_diagnostic.CA1304.severity = error | ||
dotnet_diagnostic.CA1305.severity = error | ||
dotnet_diagnostic.CA1307.severity = error | ||
dotnet_diagnostic.CA1309.severity = error | ||
dotnet_diagnostic.CA1310.severity = error | ||
dotnet_diagnostic.CA1311.severity = error | ||
|
||
; Prefer method-like constructs to have a block body | ||
csharp_style_expression_bodied_methods = when_on_single_line:suggestion | ||
csharp_style_expression_bodied_constructors = when_on_single_line:suggestion | ||
csharp_style_expression_bodied_operators = when_on_single_line:silent | ||
|
||
; Prefer property-like constructs to have an expression-body | ||
csharp_style_expression_bodied_properties = true:silent | ||
csharp_style_expression_bodied_indexers = true:silent | ||
csharp_style_expression_bodied_accessors = true:silent | ||
|
||
; Suggest more modern language features when available | ||
csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion | ||
csharp_style_pattern_matching_over_as_with_null_check = true:suggestion | ||
csharp_style_inlined_variable_declaration = true:suggestion | ||
csharp_style_throw_expression = true:suggestion | ||
csharp_style_conditional_delegate_call = true:suggestion | ||
|
||
; Newline settings | ||
csharp_new_line_before_open_brace = all | ||
csharp_new_line_before_else = true | ||
csharp_new_line_before_catch = true | ||
csharp_new_line_before_finally = true | ||
csharp_new_line_before_members_in_object_initializers = true | ||
csharp_new_line_before_members_in_anonymous_types = true | ||
|
||
; Naming styles | ||
dotnet_naming_style.pascal_case_style.capitalization = pascal_case | ||
dotnet_naming_style.camel_case_style.capitalization = camel_case | ||
|
||
; Naming rule: async methods end in Async | ||
dotnet_naming_style.async_method_style.capitalization = pascal_case | ||
dotnet_naming_style.async_method_style.required_suffix = Async | ||
dotnet_naming_symbols.async_method_symbols.applicable_kinds = method | ||
dotnet_naming_symbols.async_method_symbols.required_modifiers = async | ||
dotnet_naming_rule.async_methods_rule.severity = suggestion | ||
dotnet_naming_rule.async_methods_rule.symbols = async_method_symbols | ||
dotnet_naming_rule.async_methods_rule.style = async_method_style | ||
|
||
; Naming rule: Interfaces must be pascal-cased prefixed with I | ||
dotnet_naming_style.interface_style.capitalization = pascal_case | ||
dotnet_naming_style.interface_style.required_prefix = I | ||
dotnet_naming_symbols.interface_symbols.applicable_kinds = interface | ||
dotnet_naming_symbols.interface_symbols.applicable_accessibilities = * | ||
dotnet_naming_rule.interfaces_rule.severity = warning | ||
dotnet_naming_rule.interfaces_rule.symbols = interface_symbols | ||
dotnet_naming_rule.interfaces_rule.style = interface_style | ||
|
||
; Naming rule: All methods and properties must be pascal-cased | ||
dotnet_naming_symbols.method_and_property_symbols.applicable_kinds = method,property,class,struct,enum:property,namespace | ||
dotnet_naming_symbols.method_and_property_symbols.applicable_accessibilities = * | ||
dotnet_naming_rule.methods_and_properties_rule.severity = warning | ||
dotnet_naming_rule.methods_and_properties_rule.symbols = method_and_property_symbols | ||
dotnet_naming_rule.methods_and_properties_rule.style = pascal_case_style | ||
|
||
; Naming rule: Static fields must be pascal-cased | ||
dotnet_naming_symbols.static_member_symbols.applicable_kinds = field | ||
dotnet_naming_symbols.static_member_symbols.applicable_accessibilities = * | ||
dotnet_naming_symbols.static_member_symbols.required_modifiers = static | ||
dotnet_naming_symbols.const_member_symbols.applicable_kinds = field | ||
dotnet_naming_symbols.const_member_symbols.applicable_accessibilities = * | ||
dotnet_naming_symbols.const_member_symbols.required_modifiers = const | ||
dotnet_naming_rule.static_fields_rule.severity = warning | ||
dotnet_naming_rule.static_fields_rule.symbols = static_member_symbols | ||
dotnet_naming_rule.static_fields_rule.style = pascal_case_style | ||
|
||
; Naming rule: Private members must be camel-cased and prefixed with underscore | ||
dotnet_naming_style.private_member_style.capitalization = camel_case | ||
dotnet_naming_style.private_member_style.required_prefix = _ | ||
dotnet_naming_symbols.private_field_symbols.applicable_kinds = field,event | ||
dotnet_naming_symbols.private_field_symbols.applicable_accessibilities = private,protected,internal | ||
dotnet_naming_rule.private_field_rule.severity = warning | ||
dotnet_naming_rule.private_field_rule.symbols = private_field_symbols | ||
dotnet_naming_rule.private_field_rule.style = private_member_style | ||
csharp_style_prefer_null_check_over_type_check = true:suggestion | ||
csharp_prefer_simple_default_expression = true:suggestion | ||
csharp_style_prefer_local_over_anonymous_function = true:suggestion | ||
csharp_style_prefer_index_operator = true:suggestion | ||
csharp_style_prefer_range_operator = true:suggestion | ||
csharp_style_implicit_object_creation_when_type_is_apparent = true:suggestion | ||
csharp_style_prefer_tuple_swap = true:suggestion | ||
csharp_style_deconstructed_variable_declaration = true:suggestion | ||
csharp_style_prefer_utf8_string_literals = true:suggestion | ||
csharp_style_unused_value_expression_statement_preference = discard_variable:silent | ||
csharp_style_unused_value_assignment_preference = discard_variable:suggestion | ||
csharp_prefer_static_anonymous_function = true:suggestion | ||
csharp_prefer_static_local_function = true:suggestion | ||
csharp_style_prefer_readonly_struct = true:suggestion | ||
csharp_style_allow_embedded_statements_on_same_line_experimental = true:silent | ||
csharp_style_prefer_readonly_struct_member = true:suggestion | ||
csharp_style_allow_blank_line_after_token_in_arrow_expression_clause_experimental = true:silent | ||
csharp_style_allow_blank_line_after_token_in_conditional_expression_experimental = true:silent | ||
csharp_style_allow_blank_line_after_colon_in_constructor_initializer_experimental = true:silent | ||
csharp_style_allow_blank_lines_between_consecutive_braces_experimental = true:silent | ||
csharp_style_prefer_pattern_matching = true:silent | ||
csharp_style_prefer_switch_expression = true:suggestion | ||
csharp_style_prefer_extended_property_pattern = true:suggestion | ||
csharp_style_prefer_not_pattern = true:suggestion | ||
csharp_style_expression_bodied_local_functions = false:silent | ||
csharp_style_expression_bodied_lambdas = true:silent | ||
csharp_prefer_system_threading_lock = true:suggestion | ||
csharp_style_prefer_primary_constructors = true:suggestion | ||
csharp_style_prefer_top_level_statements = true:silent | ||
csharp_style_prefer_method_group_conversion = true:silent | ||
csharp_style_namespace_declarations = block_scoped:silent | ||
csharp_using_directive_placement = outside_namespace:silent | ||
csharp_prefer_braces = true:silent | ||
csharp_prefer_simple_using_statement = true:suggestion | ||
csharp_space_around_binary_operators = before_and_after | ||
csharp_indent_labels = no_change | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: ironically this file doesn't have a trailing newline. 😜 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Collections.Specialized; | ||
using System.Diagnostics; | ||
using System.Threading.Tasks; | ||
using System.Web; | ||
|
@@ -21,35 +22,36 @@ public CloudBlobFileStorageService( | |
IAppConfiguration configuration, | ||
ISourceDestinationRedirectPolicy redirectPolicy, | ||
IDiagnosticsService diagnosticsService, | ||
ICloudBlobContainerInformationProvider cloudBlobFolderInformationProvider) | ||
ICloudBlobContainerInformationProvider cloudBlobFolderInformationProvider) | ||
: base(client, diagnosticsService, cloudBlobFolderInformationProvider) | ||
{ | ||
_configuration = configuration; | ||
_redirectPolicy = redirectPolicy; | ||
} | ||
|
||
public async Task<ActionResult> CreateDownloadFileActionResultAsync(Uri requestUrl, string folderName, string fileName) | ||
public async Task<ActionResult> CreateDownloadFileActionResultAsync(Uri requestUrl, string folderName, string fileName, string versionParameter) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a low level method not specific to a package or version. What do you think about a simple There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would allow flexibility at the call site. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked a bit closer and the easiest approach hardcodes the parameter name lower down--this just passes the version. Making it more generic would also put the burden on a properly urlencoded query string on the caller. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems okay to me. A |
||
{ | ||
ICloudBlobContainer container = await GetContainerAsync(folderName); | ||
var blob = container.GetBlobReference(fileName); | ||
|
||
var redirectUri = GetRedirectUri(requestUrl, blob.Uri); | ||
var redirectUri = GetRedirectUri(requestUrl, blob.Uri, versionParameter); | ||
return new RedirectResult(redirectUri.AbsoluteUri, false); | ||
} | ||
|
||
internal async Task<ActionResult> CreateDownloadFileActionResult( | ||
HttpContextBase httpContext, | ||
string folderName, | ||
string fileName) | ||
string fileName, | ||
string versionParameter) | ||
{ | ||
var container = await GetContainerAsync(folderName); | ||
var blob = container.GetBlobReference(fileName); | ||
|
||
var redirectUri = GetRedirectUri(httpContext.Request.Url, blob.Uri); | ||
var redirectUri = GetRedirectUri(httpContext.Request.Url, blob.Uri, versionParameter); | ||
return new RedirectResult(redirectUri.AbsoluteUri, false); | ||
} | ||
|
||
internal Uri GetRedirectUri(Uri requestUrl, Uri blobUri) | ||
internal Uri GetRedirectUri(Uri requestUrl, Uri blobUri, string versionParameter) | ||
{ | ||
if (!_redirectPolicy.IsAllowed(requestUrl, blobUri)) | ||
{ | ||
|
@@ -79,19 +81,17 @@ internal Uri GetRedirectUri(Uri requestUrl, Uri blobUri) | |
// When no blob query string is passed, we forward the request | ||
// URI's query string to the CDN. See https://github.com/NuGet/NuGetGallery/issues/3168 | ||
// and related PR's. | ||
var queryString = !string.IsNullOrEmpty(blobUri.Query) | ||
? blobUri.Query | ||
: requestUrl.Query; | ||
var queryStringUri = !string.IsNullOrEmpty(blobUri.Query) | ||
? blobUri | ||
: requestUrl; | ||
|
||
if (!string.IsNullOrEmpty(queryString)) | ||
{ | ||
queryString = queryString.TrimStart('?'); | ||
} | ||
NameValueCollection queryValues = ParseQueryString(queryStringUri); | ||
queryValues.Add(CoreConstants.PackageVersionParameterName, versionParameter); | ||
|
||
var urlBuilder = new UriBuilder(scheme, host, port) | ||
{ | ||
Path = blobUri.LocalPath, | ||
Query = queryString | ||
Query = queryValues.ToString() | ||
}; | ||
|
||
return urlBuilder.Uri; | ||
|
@@ -102,5 +102,15 @@ public async Task<bool> IsAvailableAsync() | |
var container = await GetContainerAsync(CoreConstants.Folders.PackagesFolderName); | ||
return await container.ExistsAsync(cloudBlobLocationMode: null); | ||
} | ||
|
||
private static NameValueCollection ParseQueryString(Uri uri) | ||
{ | ||
if (string.IsNullOrEmpty(uri.Query)) return HttpUtility.ParseQueryString(string.Empty); | ||
|
||
string query = uri.Query.Substring(1); | ||
|
||
NameValueCollection nvcol = HttpUtility.ParseQueryString(query); | ||
return nvcol; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,15 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using NuGetGallery.Configuration; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this file and a few others modified in this PR don't adhere to the new style rules put in place by this PR (.editorconfig). |
||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Globalization; | ||
using System.IO; | ||
using System.Threading.Tasks; | ||
using System.Web.Hosting; | ||
using System.Web.Mvc; | ||
using NuGetGallery.Configuration; | ||
|
||
namespace NuGetGallery | ||
{ | ||
|
@@ -23,7 +24,7 @@ public FileSystemFileStorageService(IAppConfiguration configuration, IFileSystem | |
_fileSystemService = fileSystemService; | ||
} | ||
|
||
public Task<ActionResult> CreateDownloadFileActionResultAsync(Uri requestUrl, string folderName, string fileName) | ||
public Task<ActionResult> CreateDownloadFileActionResultAsync(Uri requestUrl, string folderName, string fileName, string versionParameter) | ||
{ | ||
if (string.IsNullOrWhiteSpace(folderName)) | ||
{ | ||
|
@@ -270,7 +271,7 @@ public Task SetMetadataAsync( | |
|
||
public Task SetPropertiesAsync( | ||
string folderName, | ||
string fileName, | ||
string fileName, | ||
Func<Lazy<Task<Stream>>, ICloudBlobProperties, Task<bool>> updatePropertiesAsync) | ||
{ | ||
return Task.CompletedTask; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: would be nice to keep the diagnostics alphabetically sorted by ID, as we will likely add more rules over time, and duplicates are tedious to find when the list is long.