Apply the global query param allow-list to static badges - #12177
Open
BurakErdemci wants to merge 2 commits into
Open
BurakErdemci wants to merge 2 commits into
BurakErdemci wants to merge 2 commits into
Conversation
BaseStaticService passed the raw query string to coalesceBadge, while every other badge goes through handleRequest, which only forwards the keys in globalQueryParams plus the ones the service declares. A key read by coalesceBadge but missing from the list therefore worked on static badges only (badges#9191 / badges#10110). Move the list and the filter into core/base-service/query-params.js and use it on both paths. Legacy path behaviour is unchanged. badges#10111
The test compared flattenQueryParams() against the exported Set, so a key dropped from the list would have passed. Assert the literal list instead.
Contributor
|
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
1 participant
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.
#10111
BaseStaticService.registerhanded the raw query string tocoalesceBadge. Every other badge goes throughhandleRequest, which forwards only the keys inglobalQueryParamsplus the keys the service declares. That gap let #9191 addlogoSizeand have it work on/badge/...alone until #10110 put it on the list.I moved the list and the filter into
core/base-service/query-params.jsand use it on both paths.legacy-request-handler.js/badge/...,/static/v1, gitter)coalesceBadgequeryParamSchemakeysRendered output stays the same today, because
coalesceBadgereads exactly the ten keys on the global list. A key added to one side without the other now fails the same way on every badge.Left alone on purpose:
cacheSecondsandmaxAgestill come from the raw object on the normal path, and static badges still ignore them (they set their own cache headers).'link'twice. TheSetalready ignored the duplicate, so the new list names it once.Tests:
query-params.spec.jspins the list and the filter shape;base-static.spec.jsis the first spec for that class and fails without the filter. I rannpm run lint,npm run prettier:check,npm run test:core(1585 passing),npm run test:entrypointandnpm run test:services -- --only=staticbadge,querystringstatic(19 passing) on Windows with Node 24.21.One question: I export
globalQueryParamsonly so the first spec case can check membership. Say the word and I will drop the export.