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

Readd support for authentication and .netrc #9388

Closed
wants to merge 1 commit into from

Conversation

aehlig
Copy link
Contributor

@aehlig aehlig commented Sep 16, 2019

Now that the root cause of #9327 is fixed in e38d838.

This reverts commit a0e3bb2.

Now that the root cause of bazelbuild#9327 is fixed in e38d838.

This reverts commit a0e3bb2.
@aehlig
Copy link
Contributor Author

aehlig commented Sep 16, 2019

cc @keith

Potentially also relevant Bazel 1.0 (#8573)

@dslomov
Copy link
Contributor

dslomov commented Sep 16, 2019

cc @artem-zinnatullin

@keith
Copy link
Member

keith commented Sep 16, 2019

Works for me!

@artem-zinnatullin
Copy link
Contributor

Hm, I think there are issues with new implementation (I might be wrong tho), left few comments on the commit e38d838

We better not rush it as it can either break functionality or create another CVE :)

@aehlig
Copy link
Contributor Author

aehlig commented Sep 18, 2019

Hm, I think there are issues with new implementation[...]

As mentioned on the commit, I believe the mentioned issue can only lead to too few authentication headers being sent, which seems a safe change, and moreover, a same-host policy would interact strangely with the current ctx.download interface of expecting a map from URL to authentication information.

@aehlig
Copy link
Contributor Author

aehlig commented Sep 18, 2019

[...] moreover, a same-host policy would interact strangely with the current ctx.download interface of expecting a map from URL to authentication information.

Just to clarify the tragedy here: the problem is the slightly(!) generalized interface. Simply passing a .netrc file, or the fully generalized interface of passing a function (that could, e.g., be generated from a .netrc) would both allow (or in the first case even automatically force) a same-host policy. But a map from URLs to authentication credentials is on the one hand flexible enough to describe policies that use different credentials on the same host (which might even make sense in some cases), but on the other hand is not powerful enough to describe the full host-to-credential mapping that a .netrc file describes.

cc @dslomov

@aehlig
Copy link
Contributor Author

aehlig commented Sep 18, 2019

[...] I believe the mentioned issue can only lead to too few authentication headers being sent, which seems a safe change [...]

I wonder, if the amount of authentication sent is enough for the original use case.

cc @ittaiz @genrym

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

Successfully merging this pull request may close these issues.

5 participants