Skip to content

Commit

Permalink
Fix getting authentication for URLs in http repo rules
Browse files Browse the repository at this point in the history
- Fixed the leak of `remote_patches` URLs for downloaded the source archive.
- Compute auth for required URLs only

Fixes bazelbuild#22201

Closes bazelbuild#22517.

PiperOrigin-RevId: 638300996
Change-Id: Ib76e3284f209d2314844cfd662ac8eadba785fae
  • Loading branch information
meteorcloudy authored and bazel-io committed May 29, 2024
1 parent bc346fb commit 38edbde
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 40 deletions.
8 changes: 4 additions & 4 deletions MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions src/test/tools/bzlmod/MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 20 additions & 29 deletions tools/build_defs/repo/http.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,19 @@ Authentication is not supported.
URLs are tried in order until one succeeds, so you should list local mirrors first.
If all downloads fail, the rule will fail."""

def _get_all_urls(ctx):
"""Returns all urls provided via the url, urls and remote_patches attributes.
def _get_source_urls(ctx):
"""Returns source urls provided via the url, urls attributes.
Also checks that at least one url is provided."""
if not ctx.attr.url and not ctx.attr.urls:
fail("At least one of url and urls must be provided")

all_urls = []
source_urls = []
if ctx.attr.urls:
all_urls = ctx.attr.urls
source_urls = ctx.attr.urls
if ctx.attr.url:
all_urls = [ctx.attr.url] + all_urls
if hasattr(ctx.attr, "remote_patches") and ctx.attr.remote_patches:
all_urls = all_urls + ctx.attr.remote_patches.keys()

return all_urls
source_urls = [ctx.attr.url] + source_urls
return source_urls

_AUTH_PATTERN_DOC = """An optional dict mapping host names to custom authorization patterns.
Expand Down Expand Up @@ -130,25 +127,21 @@ def _http_archive_impl(ctx):
if ctx.attr.build_file and ctx.attr.build_file_content:
fail("Only one of build_file and build_file_content can be provided.")

all_urls = _get_all_urls(ctx)
auth = get_auth(ctx, all_urls)

source_urls = _get_source_urls(ctx)
download_info = ctx.download_and_extract(
# TODO(fzakaria): all_urls here has the remote_patch URL which is incorrect
# I believe this to be a file
all_urls,
source_urls,
ctx.attr.add_prefix,
ctx.attr.sha256,
ctx.attr.type,
ctx.attr.strip_prefix,
canonical_id = ctx.attr.canonical_id or get_default_canonical_id(ctx, all_urls),
auth = auth,
canonical_id = ctx.attr.canonical_id or get_default_canonical_id(ctx, source_urls),
auth = get_auth(ctx, source_urls),
integrity = ctx.attr.integrity,
)
workspace_and_buildfile(ctx)

download_remote_files(ctx, auth = auth)
patch(ctx, auth = auth)
download_remote_files(ctx)
patch(ctx)

return _update_integrity_attr(ctx, _http_archive_attrs, download_info)

Expand Down Expand Up @@ -176,15 +169,14 @@ def _http_file_impl(ctx):
download_path = ctx.path("file/" + downloaded_file_path)
if download_path in forbidden_files or not str(download_path).startswith(str(repo_root)):
fail("'%s' cannot be used as downloaded_file_path in http_file" % ctx.attr.downloaded_file_path)
all_urls = _get_all_urls(ctx)
auth = get_auth(ctx, all_urls)
source_urls = _get_source_urls(ctx)
download_info = ctx.download(
all_urls,
source_urls,
"file/" + downloaded_file_path,
ctx.attr.sha256,
ctx.attr.executable,
canonical_id = ctx.attr.canonical_id or get_default_canonical_id(ctx, all_urls),
auth = auth,
canonical_id = ctx.attr.canonical_id or get_default_canonical_id(ctx, source_urls),
auth = get_auth(ctx, source_urls),
integrity = ctx.attr.integrity,
)
ctx.file("WORKSPACE", "workspace(name = \"{name}\")".format(name = ctx.name))
Expand All @@ -211,15 +203,14 @@ filegroup(

def _http_jar_impl(ctx):
"""Implementation of the http_jar rule."""
all_urls = _get_all_urls(ctx)
auth = get_auth(ctx, all_urls)
source_urls = _get_source_urls(ctx)
downloaded_file_name = ctx.attr.downloaded_file_name
download_info = ctx.download(
all_urls,
source_urls,
"jar/" + downloaded_file_name,
ctx.attr.sha256,
canonical_id = ctx.attr.canonical_id or get_default_canonical_id(ctx, all_urls),
auth = auth,
canonical_id = ctx.attr.canonical_id or get_default_canonical_id(ctx, source_urls),
auth = get_auth(ctx, source_urls),
integrity = ctx.attr.integrity,
)
ctx.file("WORKSPACE", "workspace(name = \"{name}\")".format(name = ctx.name))
Expand Down
6 changes: 3 additions & 3 deletions tools/build_defs/repo/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ def _use_native_patch(patch_args):
return False
return True

def _download_patch(ctx, patch_url, integrity, auth):
def _download_patch(ctx, patch_url, integrity, auth = None):
name = patch_url.split("/")[-1]
patch_path = ctx.path(_REMOTE_PATCH_DIR).get_child(name)
ctx.download(
patch_url,
patch_path,
canonical_id = ctx.attr.canonical_id,
auth = auth,
auth = get_auth(ctx, [patch_url]) if auth == None else auth,
integrity = integrity,
)
return patch_path
Expand All @@ -108,7 +108,7 @@ def download_remote_files(ctx, auth = None):
remote_file_urls,
path,
canonical_id = ctx.attr.canonical_id,
auth = auth,
auth = get_auth(ctx, remote_file_urls) if auth == None else auth,
integrity = ctx.attr.remote_file_integrity.get(path, ""),
block = False,
)
Expand Down

0 comments on commit 38edbde

Please sign in to comment.