Move to go-utils v1.1.8 - #78
Closed
juicycleff wants to merge 1 commit into
Closed
Conversation
v1.1.8 carries one fix, and it is the one this repository has been working around. bindQueryParam now fills a []string from every occurrence of a repeated query parameter, the way bindFormParam already did. A lone value still expands on commas, so scope=openid,profile keeps behaving as it always has. Nothing changes here yet. resourceParams still reads the RFC 8707 indicator off the raw request, the spec and all three SDKs regenerate byte-identical, and the suite passes. The bump is what makes the next step possible rather than the step itself. That next step is real, though. /authorize kept its raw read because the query side of the binder took the first resource and quietly dropped the rest, which was worse than the "unsupported field type" error the older binder raised: the request succeeded, and the token came back scoped to fewer audiences than the client asked for. With v1.1.8 that hazard is gone and the parameter can be a bound struct field.
Contributor
Author
|
Folding this into #76 instead. That branch is the one that actually needs v1.1.8: its commit is marked BLOCKED on xraph/go-utils#4, and three tests in plugins/oauth2provider fail without it. A standalone bump changes nothing on its own, so it belongs next to the code it unblocks rather than in a PR of its own. Same commit is now 7c3f53a on feat/oauth2-authorize-resource-param. |
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.
One line of
go.mod, and nothing in this repository behaves differently because of it. The reason to take it is what it makes possible next.What v1.1.8 fixes
bindQueryParamnow fills a[]stringfrom every occurrence of a repeated query parameter, the waybindFormParamalready did. A lone value still expands on commas, soscope=openid,profilekeeps working exactly as before, and header parameters are untouched.The bug it closes was the quiet kind. Before
setFieldValuegrew a slice case, a[]stringquery field failed the request outright with "unsupported field type". After that, the binder read one value and split it on commas, so the request succeeded carrying less than the caller sent. For an RFC 8707 resource indicator that means an access token scoped to fewer audiences than the client asked for, with nothing logged and nothing to notice.What changes here
Nothing yet, and that is deliberate.
resourceParamsstill reads the indicator off the raw request, the spec and all three SDKs regenerate byte-identical, and the full suite passes. Verified on this branch rather than assumed:go build ./...clean,go mod tidyproduces nothing beyond the bumpgo.modandgo.summodifiedgo test ./...exit 0What it unblocks
/authorizereads the query string by hand today, and #74 documented the parameter with aWithQuerySchemastruct that nothing binds to. That split existed because the query side of the binder would have dropped every resource after the first. With v1.1.8 the hazard is gone, so the parameter can become a real bound field andresourceParamscan go with it.It also matters for #77, which left a skipped spec test naming this exact go-utils change as its precondition. That test should be runnable once this lands and #77 is resolved against the current main.
Worth flagging for whoever picks that up: #74's commit message explains
resourceParamsas necessary because the binder cannot decode a[]string. As of v1.1.8 that reasoning is history rather than a live constraint.