-
Notifications
You must be signed in to change notification settings - Fork 356
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
GR: Parse paths in npmrc auth fields correctly #901
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #901 +/- ##
==========================================
- Coverage 63.37% 63.34% -0.04%
==========================================
Files 145 145
Lines 11883 11873 -10
==========================================
- Hits 7531 7521 -10
Misses 3885 3885
Partials 467 467 ☔ View full report in Codecov by Sentry. |
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.
nice! just some minor comments.
func (info npmRegistryInfo) buildRequest(ctx context.Context, urlComponents ...string) (*http.Request, error) { | ||
for i := range urlComponents { | ||
urlComponents[i] = url.PathEscape(urlComponents[i]) | ||
var npmAuthFields = [...]string{":_authToken", ":_auth", ":username", ":_password"} // reference of the relevant config key suffixes |
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.
What does the ...
mean in the [...]
?
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.
It sets the length of the array based on the number of dots (so in this case 3); my understanding is it's a shortcut make
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.
hmm... but there's 4 elements in the slice?
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.
It means an array with length equal to the number of elements defined:
From the go spec:
The length of an array literal is the length specified in the literal type. If fewer elements than the length are provided in the literal, the missing elements are set to the zero value for the array element type. It is an error to provide elements with index values outside the index range of the array. The notation
...
specifies an array length equal to the maximum element index plus one.buffer := [10]string{} // len(buffer) == 10 intSet := [6]int{1, 2, 3, 5} // len(intSet) == 6 days := [...]string{"Sat", "Sun"} // len(days) == 2
Should fix #899
Changed the npmrc parsing logic when using
--data-source=native
so that specifying registries with paths (e.g.//my.registry/package/path:_authToken
) will now correctly add the authorization headers.Used npm-registry-fetch as reference.