Repository navigation
fix: send the API key in the query string, not the URL path - #3
Merged
Merged
Conversation
velddev
commented
Oct 23, 2026
Comment on lines
306
to
317
Member
Author
There was a problem hiding this comment.
can we improve this to be less error-prone?
Replaces the hand-assembled string of "&name=value" fragments with a list of parameters joined in one place, so the separator and the key's position can't drift again and every value is escaped.
The Push to Nuget step had no branch guard, unlike Pack and the GitHub Packages push, so on PR and branch runs it tried to push from a missing ./artifacts folder and failed the check. Guard it the same way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The request URL was built without a
?:So the whole parameter list, API key included, was part of the URL path. .NET's HTTP client tracing and OpenTelemetry only redact the query string, so the proxycheck.io API key ended up in plain text in exported spans (
url.full) and logs.Fix
Build a proper query string, which is proxycheck.io's documented form:
The URL now ends in
/?, the key (when set) is followed by&, and the other parameters are unchanged. With no key, the URL ishttps://proxycheck.io/v2/?vpn=0&.... The public API and behaviour are otherwise unchanged.Version
Bumped
Topgg.ProxyCheckfrom 1.0.1 to 1.0.2 (Version,AssemblyVersion,FileVersion). CI pushes with--skip-duplicate, so without a bump nothing new would be published. Consumers should move to 1.0.2 once it is published.Tests
RequestUriTestcaptures the outgoing request through a substitutedHttpMessageHandlerand checks thatAbsolutePathis exactly/v2/, that the path has no key, and thatQuerycontainskey=. It uses a dummy key.DeserializeTestURLs are updated to the/v2/?vpn=...form.dotnet testpasses 3 of 3 locally.CI workflow change
This PR also touches
.github/workflows/dotnet.yml. The "Push to Nuget" step had noif: github.ref == 'refs/heads/master'guard, unlike "Publish" (pack) and "Push to GitHub Packages". On PR and branch runs the pack step is skipped, so the NuGet push failed withCould not find a part of the path '.../artifacts'and turned the check red even though build and tests passed. The step now has the same guard, so master publishing is unchanged and PR runs no longer fail on it.