Skip to content

fix: warn on malformed URI query parameter#11489

Merged
tomberek merged 2 commits intoNixOS:masterfrom
bryanhonof:bryanhonof.warn-on-malformed-uri-query
Sep 30, 2024
Merged

fix: warn on malformed URI query parameter#11489
tomberek merged 2 commits intoNixOS:masterfrom
bryanhonof:bryanhonof.warn-on-malformed-uri-query

Conversation

@bryanhonof
Copy link
Member

@bryanhonof bryanhonof commented Sep 11, 2024

cc: @tomberek @roberth @Mic92

Motivation

Retry of #11349

Context

Priorities and Process

Add 👍 to pull requests you find important.

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

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Could you add a test in functional that checks for the warning? Here is an example: d22a2fe
You can use grepQuiet for filtering.

@bryanhonof bryanhonof force-pushed the bryanhonof.warn-on-malformed-uri-query branch from f53b144 to a798569 Compare September 18, 2024 13:40
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Sep 18, 2024
@bryanhonof
Copy link
Member Author

bryanhonof commented Sep 18, 2024

Could you add a test in functional that checks for the warning? Here is an example: d22a2fe You can use grepQuiet for filtering.

@Mic92 I added some tests in tests/functional/flakes, and used the assertStderr function to test the output I'm expecting. Please let me know if this isn't what you were expecting.

@bryanhonof bryanhonof force-pushed the bryanhonof.warn-on-malformed-uri-query branch 2 times, most recently from f9cead0 to b04005a Compare September 18, 2024 14:37
@bryanhonof bryanhonof requested a review from Mic92 September 18, 2024 14:40
@bryanhonof bryanhonof force-pushed the bryanhonof.warn-on-malformed-uri-query branch 3 times, most recently from 196c65e to 33b8aa6 Compare September 18, 2024 16:01
@bryanhonof
Copy link
Member Author

CI on macOS seems to been canceled whilst it was running.

@Mic92
Copy link
Member

Mic92 commented Sep 18, 2024

tests run locally on macOS:

Ok: 151
Expected Fail: 1
Fail: 2
Unexpected Pass: 0
Skipped: 29
Timeout: 0

@bryanhonof
Copy link
Member Author

I also ran a nix flake check on my aarch64-darwin machine, all seems to have passed just fine.

@bryanhonof bryanhonof force-pushed the bryanhonof.warn-on-malformed-uri-query branch from 33b8aa6 to 6c93bf1 Compare September 18, 2024 23:36
Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

CI is green now.

infinisil added a commit to NixOS/SC-election-2024 that referenced this pull request Sep 20, 2024
Some of these are a bit after the specified time period, but I also believe some of these items are difficult to gauge how much "impact" they had.

Helped organize, host, and run, the Summer of Nix Lecture Series 2022

- https://www.youtube.com/playlist?list=PLt4-_lkyRrOMWyp5G-m_d1wtTcbBaOxZk
- NixOS/infra#213

Helped organize, and host infra for, the Summer of Nix Lecture Series 2023

- https://www.youtube.com/playlist?list=PLt4-_lkyRrOPcBuz_tjm6ZQb-6rJjU3cf
- NixOS/infra#240

Helped organize, host, and was responsible for livestreaming infra during, NixCon Paris 2022

- https://www.youtube.com/playlist?list=PLgknCdxP89ReD6gxl755B6G_CI65z4J2e

Maintenance of some nixpkgs packages

- NixOS/nixpkgs#340223 (contribution after 2024-05-01)
- NixOS/nixpkgs#290084
- NixOS/nixpkgs#170089

Organized, and assembled a team for the FOSDEM 2023 Nix/NixOS Devroom

- https://discourse.nixos.org/t/fosdem-2023-nix-and-nixos-devroom/23133

Organizer & sole maintainer of the Config Management Camp Nix track

- https://discourse.nixos.org/t/config-management-camp-2023-ghent/23455
- https://discourse.nixos.org/t/config-management-camp-2024-ghent/33852
- https://discourse.nixos.org/t/cfgmgmtcamp-2025-is-looking-for-nix-presentations/51658 (contribution after 2024-05-01)

Public speaking & spreading awareness of Nix/NixOS

- https://youtu.be/gUjvnZ9ZwMs?si=nDiZTCpQj53wwq8P
- https://www.youtube.com/watch?v=hNcYPH5Q_pA&t=862s

The occasional dabble into the Nix C++ code base

- NixOS/nix#11494 (contribution after 2024-05-01)
- NixOS/nix#11490 (contribution after 2024-05-01)
- NixOS/nix#11489 (contribution after 2024-05-01)
- NixOS/nix#11349 (contribution after 2024-05-01)
- NixOS/nix#11241 (contribution after 2024-05-01)
- NixOS/nix#9557
- NixOS/nix#8788
- NixOS/nix#8212
- NixOS/nix#5147

General evangelism

Pretty much every event I attend, I'm talking about Nix, showing off Nix/NixOS, and just trying to get people to see how awesome this tool is.
@bryanhonof bryanhonof force-pushed the bryanhonof.warn-on-malformed-uri-query branch 2 times, most recently from 2d8ee84 to ebc80fa Compare September 26, 2024 22:08
@tomberek tomberek self-assigned this Sep 30, 2024
@bryanhonof bryanhonof force-pushed the bryanhonof.warn-on-malformed-uri-query branch from d6f8e03 to 5150a96 Compare September 30, 2024 12:44
Copy link
Contributor

@tomberek tomberek left a comment

Choose a reason for hiding this comment

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

  • reviewed in meeting
  • built and tested on x86_64-linux and aarch64-darwin

@tomberek tomberek merged commit 14f029d into NixOS:master Sep 30, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-09-30-nix-team-meeting-minutes-182/53814/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants