Skip to content

Conversation

@pascalj
Copy link

@pascalj pascalj commented Mar 29, 2025

Motivation

This fixes an issue where nix would try to check out invalid URLs, because it would pass 'dir' to the HTTP endpoint.

For later versions this was fixed in b2be6fe. This is a backport of just the relevant part.

Context

See #12417, where this is first discussed.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This fixes an issue where nix would try to check out invalid URLs,
because it would pass 'dir' to the HTTP endpoint.

For later versions this was fixed in
b2be6fe. This is a backport of just the
relevant part.

See NixOS#12417
@Mic92 Mic92 force-pushed the 2.24-maintenance branch from 61069a5 to 72bf563 Compare April 15, 2025 11:34
return std::nullopt;
}

const auto subdir = getOr(parsedURL.query, "dir", "");
Copy link
Member

Choose a reason for hiding this comment

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

What is missing here?

Copy link
Author

Choose a reason for hiding this comment

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

If you're asking about the code: there should be nothing missing (unless I am missing something). I left it as a draft since it seemed in some comments that there's no interest in a backport.

@Mic92
Copy link
Member

Mic92 commented May 12, 2025

I believe this is fixed by #13159 no?

@Mic92 Mic92 closed this May 12, 2025
@edolstra
Copy link
Member

@Mic92 That fixed it on master, this is a backport to 2.24.

@Mic92 Mic92 reopened this May 13, 2025
@pascalj pascalj marked this pull request as ready for review May 14, 2025 20:58
@pascalj pascalj requested a review from edolstra as a code owner May 14, 2025 20:58
@pascalj
Copy link
Author

pascalj commented May 14, 2025

Sorry for the delay, I thought it might not be of interest after #13159 but it indeed solves the problem for 2.24.

@Mic92 Mic92 enabled auto-merge May 18, 2025 21:13
@Mic92 Mic92 merged commit 7d15dbf into NixOS:2.24-maintenance May 18, 2025
14 checks passed
@pascalj pascalj deleted the 2.24-maintenance branch May 19, 2025 05:55
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