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

fix: /nar: handle nar-serve URLs #293

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

flokli
Copy link

@flokli flokli commented Mar 21, 2024

Old requests from nar-serve were never routed to the handler, as harmonia complained about being unable to serialize hash, as it failed to populate the NarRequest struct.

This means, the get function was never executed for the "/nar/outhash-narhash.nar" case.

Additionally, the logic was a bit hard to debug, as multiple "hash" variables meaning different things were present.

I refactored the URL handler to have two URL handlers, populating a new PathParams struct with narhash and outhash fields.

I also pushed length validations into the regex that's now used in the URL handler directly (and introduced validation for the strings to be nixbase32)

This simplifies the logic in the URL handler a bit, and most importantly, fixes support for URLs generated by nar-serve.

Both URLs were manually verified to now serve a NAR.

Old requests from nar-serve were never routed to the handler,
as harmonia complained about being unable to serialize `hash`,
as it failed to populate the NarRequest struct.

This means, the `get` function was never executed for the
"/nar/outhash-narhash.nar" case.

Additionally, the logic was a bit hard to debug, as multiple "hash"
variables meaning different things were present.

I refactored the URL handler to have two URL handlers, populating a new
`PathParams` struct with `narhash` and `outhash` fields.

I also pushed length validations into the regex that's now used in the
URL handler directly (and introduced validation for the strings to be
nixbase32)

This simplifies the logic in the URL handler a bit, and most
importantly, fixes support for URLs generated by nar-serve.

Both URLs were manually verified to now serve a NAR.
@flokli flokli marked this pull request as draft March 21, 2024 10:25
@flokli flokli marked this pull request as ready for review March 21, 2024 10:40
@Mic92 Mic92 merged commit 55880f1 into nix-community:master Mar 21, 2024
14 checks passed
@flokli flokli deleted the fix-nar-serve-paths branch March 22, 2024 08:46
Mic92 pushed a commit to NixOS/nixpkgs that referenced this pull request Mar 22, 2024
This cherry-picks nix-community/harmonia#293 into 0.7.3.
We can't bump to Harmonia 0.7.5 in release-23.11 as it brings a
libnixstore bump with backwards-incompatible changes (for a new Nix
version that's not available in 23.11).
Mic92 added a commit to NixOS/nixpkgs that referenced this pull request Mar 22, 2024
zeme-wana pushed a commit to input-output-hk/nixpkgs that referenced this pull request May 8, 2024
This cherry-picks nix-community/harmonia#293 into 0.7.3.
We can't bump to Harmonia 0.7.5 in release-23.11 as it brings a
libnixstore bump with backwards-incompatible changes (for a new Nix
version that's not available in 23.11).
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.

2 participants