add GET /openrtb2/auction endpoint with profiles by przemkaczmarek · Pull Request #4820 · prebid/prebid-server

przemkaczmarek · 2026-06-09T17:31:55Z

… and exitpoint support

New GET endpoint: GET /openrtb2/auction registered alongside existing POST
parseGETRequest() builds OpenRTB BidRequest from query params:
- srid (required) -> ext.prebid.storedrequest.id
- slot -> imp[0].tagid
- mtype (1=banner, 2=video, 3=audio) with full param sets
- rprof/req_profiles -> ext.prebid.profiles (request-level)
- iprof/imp_profiles -> imp[0].ext.prebid.profiles (imp-level)
- of/om -> output format and module signaling
- privacy: gdpr, gdpr_consent, gppc, gpps, coppa, usp
- content: cgenre, clang, ctitle, cseries, curl
- video: mindur, maxdur, w, h, skip, proto, api, delivery, etc.
- audio: mindur, maxdur, feed, nvol, stitched, etc.
Profiles: ProfileFetcher interface + NoopProfileFetcher + MergeProfiles (RFC 7396)
ExtRequestPrebid: new fields Profiles, OutputFormat (of), OutputModule (om)
ExtImpPrebid: new field Profiles
ExtRequestPrebidServer: new field RequestMethod (set to GET for GET requests)
sendAuctionResponse: exitpoint stage skipped on error path (hasErrors guard)
Tests: 25 new tests covering parseGETRequest, MergeProfiles, serialization

… and exitpoint support - New GET endpoint: GET /openrtb2/auction registered alongside existing POST - parseGETRequest() builds OpenRTB BidRequest from query params: - srid (required) -> ext.prebid.storedrequest.id - slot -> imp[0].tagid - mtype (1=banner, 2=video, 3=audio) with full param sets - rprof/req_profiles -> ext.prebid.profiles (request-level) - iprof/imp_profiles -> imp[0].ext.prebid.profiles (imp-level) - of/om -> output format and module signaling - privacy: gdpr, gdpr_consent, gppc, gpps, coppa, usp - content: cgenre, clang, ctitle, cseries, curl - video: mindur, maxdur, w, h, skip, proto, api, delivery, etc. - audio: mindur, maxdur, feed, nvol, stitched, etc. - Profiles: ProfileFetcher interface + NoopProfileFetcher + MergeProfiles (RFC 7396) - ExtRequestPrebid: new fields Profiles, OutputFormat (of), OutputModule (om) - ExtImpPrebid: new field Profiles - ExtRequestPrebidServer: new field RequestMethod (set to GET for GET requests) - sendAuctionResponse: exitpoint stage skipped on error path (hasErrors guard) - Tests: 25 new tests covering parseGETRequest, MergeProfiles, serialization

bsardo · 2026-07-23T12:31:42Z

 	}

 	r.POST("/openrtb2/auction", openrtbEndpoint)
+	r.GET("/openrtb2/auction", openrtbEndpoint)


👍 same handler for both POST and GET

bsardo · 2026-07-23T12:58:57Z

+
+	// For GET requests, build the request body JSON from query parameters.
+	// The resulting JSON is fed into the normal parseRequest flow as if it were a POST body.
+	if httpRequest.Method == http.MethodGet {
+		getBody, getErr := parseGETRequest(httpRequest)
+		if getErr != nil {
+			errs = []error{getErr}
+			return
+		}
+		httpRequest.Body = io.NopCloser(strings.NewReader(string(getBody)))
+	}
+


The current approach in parseRequest synthesizes a JSON body from GET query params and then stuffs it back into httpRequest.Body so the downstream body-reading code (compression check, LimitedReader, size enforcement) processes it as if it were a real POST payload. This works, but it's a side-effect-heavy trick that:

Couples GET handling to POST-only mechanics (compression negotiation, request size limits) that don't logically apply to query-param-constructed JSON.

Makes control flow harder to follow — a reader has to notice the body was swapped upstream.

Is fragile if future changes add middleware that also inspects httpRequest.Body.

Consider replacing the body-mutation block with a method-switch that produces requestJson []byte directly:

var requestJson []byte switch httpRequest.Method { case http.MethodGet: requestJson, err = parseGETRequest(httpRequest) if err != nil { errs = []error{err} return } case http.MethodPost: requestJson, err = readRequestBody(httpRequest, deps.cfg) if err != nil { errs = []error{err} return } default: errs = []error{fmt.Errorf("unsupported HTTP method: %s", httpRequest.Method)} return } labels.RequestSize = len(requestJson)

Where readRequestBody is a small extracted function encapsulating the existing compression-check + LimitedReader + size-exceeded logic. This way:

GET requests skip compression/size checks that are irrelevant for query-param-constructed JSON.

No mutation of httpRequest.Body — the intent is explicit at the call site.

parseGETRequest stays exactly as-is (it's already a clean pure function).

The rest of parseRequest operates on requestJson without caring how it was obtained.

bsardo · 2026-07-23T13:03:57Z

Change this file name to get_request_parser.go as it is really just building a raw OpenRTB request from query parameters.

bsardo · 2026-07-23T13:05:07Z

+		}
+		httpRequest.Body = io.NopCloser(strings.NewReader(string(getBody)))
+	}
+


I would move my suggested readRequestBody function for POST into a new file called post_request_parser.go for symmetry with my suggested name of get_request_parser.go for get_auction.go.

bsardo · 2026-07-23T13:21:36Z

+// qInt parses the first matching param as an integer. Returns -1 if absent or invalid.
+func qInt(q url.Values, names ...string) int {
+	return qParseInt(qFirst(q, names...))
+}
+
+// qParseInt parses a string as int, returning -1 on error or empty string.
+func qParseInt(s string) int {
+	if s == "" {
+		return -1
+	}
+	i, err := strconv.Atoi(s)
+	if err != nil {
+		return -1
+	}
+	return i
+}


These functions are treating values that cannot be converted to int as though they were not provided by returning -1. Is this the behavior we want? Do the requirements indicate that is ok? If it is not specified, we should see what the Java team did. We might want to warn or fail so the caller knows that they provided an incorrect value. It is possible though that this is acceptable assuming any required value that we are attempting to set via query param is already on the stored request meaning there is a default value to use.

The behaviour is intentional. The GET interface requirements (Tech Response, query truncation handling) specify that invalid parameter values are dropped and unknown parameters are silently ignored, rather than rejecting the request. The reasoning is exactly the case you raised at the end of your comment: GET query strings are prone to truncation and mangling by intermediate proxies/CDNs, and any field settable via query param is expected to have a default available from the stored request or profile. Failing a whole auction because one parameter arrived malformed would make the endpoint unnecessarily brittle in exactly the environments it targets.

Java takes the same approach. AmpRequestFactory.parseIntOrZero() catches NumberFormatException and returns 0; the timeout parser returns null on the same exception; canonicalUrl() swallows IllegalArgumentException. In every case a malformed query param is silently treated as absent, with no warning or rejection. Our -1 is the same policy

bsardo · 2026-07-23T13:27:44Z

+	// stored auction response
+	if sarid := qFirst(q, "sarid"); sarid != "" {
+		imp.Ext = setGETImpExtField(imp.Ext, "prebid", "storedauctionresponse", map[string]string{"id": sarid})
+	}
+
+	// Imp-level profiles
+	if iprof := qCSV(q, "iprof", "imp_profiles"); len(iprof) > 0 {
+		impPrebid := openrtb_ext.ExtImpPrebid{Profiles: iprof}
+		if extBytes, merr := json.Marshal(map[string]interface{}{"prebid": impPrebid}); merr == nil {
+			imp.Ext = extBytes
+		}
+	}
+


There is a bug here. The first if block uses a helper function to set imp.Ext. The next if block potentially overwrites imp.Ext instead of merging the newly created impPrebid object into imp.Ext.

bsardo · 2026-07-23T13:29:58Z

+func setGETImpExtField(ext json.RawMessage, outerKey, innerKey string, value interface{}) json.RawMessage {
+	m := map[string]interface{}{}
+	if len(ext) > 0 {
+		_ = json.Unmarshal(ext, &m)


We shouldn't swallow errors.

bsardo · 2026-07-23T13:30:05Z

+	}
+	outer[innerKey] = value
+	m[outerKey] = outer
+	b, _ := json.Marshal(m)


We shouldn't swallow errors.

bsardo · 2026-07-23T13:56:12Z

This file needs to handle the following requirements:

Prebid Server must support the following fields being passed in the GET request header:
The IP address must be specified as X-Device-IP and must be mapped to the ip attribute in the device object in the oRTB standard
The User Agent must be specified as X-Device-User-Agent and must be mapped to the ua attribute in the device object in the oRTB standard

Prebid Server should support the following optional fields being passed in the GET request header:
The device make can be specified as X-Device-Make and must be mapped to the make attributed in the device object in the oRTB standard
The device model can be specified as X-Device-Model and must be mapped to the model attributed in the device object in the oRTB standard
The device os can be specified as X-Device-Os and must be mapped to the os attributed in the device object in the oRTB standard
The audio player can be specified as X-Device-Player and must be mapped to the displaymanager attributed in the imp object in the oRTB standard

Some of these values may be passed in as query parameters as well as via headers. I assume the query parameters take precedence but let's check with the Java team to see what they did.

Implemented — applyGETHeaderParams now maps all six headers, and I've added test coverage for each.

One important correction on precedence. You assumed query params win, but the requirements specify the opposite. From the Tech Response §3.1 rule 4: "HTTP headers should take precedence over conflicting query string values." Our internal spec notes make the same point explicitly, and flag it as an override rather than a fallback.

The rationale makes sense: GET query strings are prone to truncation and rewriting by intermediate proxies/CDNs, whereas the X-Device-* headers are set by the player closest to the device. So the header is the more trustworthy source. I've implemented it that way — query params are mapped first, then applyGETHeaderParams overwrites. TestParseGETRequest_HeadersOverrideQueryParams locks the behaviour in.

przemkaczmarek · 2026-07-25T13:10:48Z

 	// Mark request method so exit-point modules can detect GET channel.
 	prebid.Server = &openrtb_ext.ExtRequestPrebidServer{
-		RequestMethod: http.MethodGet,
+		HTTPMethod: http.MethodGet,


Aligned ext.prebid.server with prebid-server-java. Java added the same capability in PR #4378 (ExtRequestPrebidServer.httpMethod), which serializes as http_method via Jackson's SNAKE_CASE strategy. We had it as requestmethod, so the two implementations would have exposed the same information under different ORTB names — a problem for any module reading it. Renamed the Go field to HTTPMethod with json:"http_method,omitempty" to match, and added a round-trip test plus a comment recording why the name is what it is.

bsardo · 2026-07-28T13:40:58Z

+	//
+	// The JSON name matches prebid-server-java's `ExtRequestPrebidServer.httpMethod`
+	// (added in PR #4378), which serializes as `http_method` under Jackson's
+	// SNAKE_CASE strategy. Keeping the two implementations in sync matters because
+	// modules read this field off the ORTB request.


Remove this comment

bsardo · 2026-07-28T13:43:01Z

Delete this file. Going forward, if you want to provide this, just attach it as a file in a comment for reviewers instead of making it part of a commit.

bsardo · 2026-07-28T13:51:42Z

-	// Exitpoint will modify the response and set response headers according to hook implementation.
-	finalResponse := hookExecutor.ExecuteExitpointStage(response, w)
+	// Exitpoint modifies the response and sets response headers according to hook implementation.
+	// Per spec: exitpoint is only triggered when there are no errors during auction processing.


Delete this second "Per spec" comment

bsardo · 2026-07-28T13:54:27Z

+	if !hasErrors {
+		finalResponse = hookExecutor.ExecuteExitpointStage(response, w)
+	}


If this path is not taken because hasErrors is false, is the expected behavior that finalResponse is empty? Please double check this.

finalResponse is initialized to response before the branch, so when hasErrors is true we simply skip the exitpoint stage and encode the standard BidResponse — same behavior as before this change. The variable is never left empty.

bsardo · 2026-07-28T14:01:36Z

Please create post_request_parser_test.go with unit tests for this file.

bsardo · 2026-07-28T14:07:27Z

The following requirements appear in the Prebid Server Technical Response to Audio and CTV Requirements and may still need to be implemented:

The main assumption of GET interface is that there is only 1 impression object per request. In case when for any reason the resulting auction request ends up with more than 1 impression, Prebid Server should discard other impressions after the first one and emit a sampled log entry with the referrer and account information.

Query string length is limited. Length limit varies greatly by client and by variables like the presence of the proxy server. Current limit on the Prebid Server side is 8192 to guard against malicious resource exhaustion attacks. The value can be tweaked through the configuration: server.max-initial-line-length.

bsardo · 2026-07-28T14:33:09Z

As discussed, this PR will not implement any profile logic. Let's remove all profile logic which means removing this file, its corresponding test file, all references to profiles in openrtb_ext directory and any parsing of the profiles GET params. We can add all of that in a separate PR along with the logic that fetches the profiles and merges them into the stored request.

That ends up limiting this PRs scope to parsing all non-profile query params with proper validations constructing a JSON body from them.

Two requirements from the Audio/CTV technical response were missing. Request line length limit: - Add MaxInitialLineLength to config with a default of 8192 bytes and validation that it is not negative. A value of 0 disables the check. - parseGETRequest now rejects a request whose method, URI and protocol exceed the limit, guarding against resource exhaustion via oversized query strings. Single impression per GET request: - The GET interface assumes exactly one impression. enforceSingleImp truncates imp to the first element and emits a sampled warning that records the discarded count, the referrer and the account, so operators can find misbehaving callers without flooding the logs. - Runs after stored requests are merged, since that is where extra impressions can appear. Also adds unit coverage for readRequestBody, which had none: plain and gzip bodies, unsupported encodings, malformed gzip headers, and the size limit boundaries.

Profiles are out of scope for this PR. Drop the data model, the query parameter parsing and the merge helper so the change is limited to parsing the non-profile query params, validating them and building a JSON body from the result. - Delete stored_requests/profiles.go and its test. It held the ProfileFetcher interface, a noop implementation and MergeProfiles, none of which was ever called from the auction flow. - Drop Profiles from ExtRequestPrebid and ExtImpPrebid. - Stop parsing rprof / req_profiles and iprof / imp_profiles, and renumber the parameter precedence list in the parseGETRequest doc. - Rename request_profiles_test.go to request_get_ext_test.go, since what remains covers of, om and http_method rather than profiles. - Repoint the setGETImpExtField tests at storedauctionresponse, which is now the only caller of that helper. Profiles will come back in a separate PR together with the fetching backend and the logic that merges them into the stored request.

przemkaczmarek added 2 commits June 9, 2026 19:33

docs(get-interface): add reviewer guide README

3273ad7

przemkaczmarek force-pushed the GetInterface branch from 4127e62 to 3273ad7 Compare June 9, 2026 17:33

style: run gofmt on modified files

afb4209

bsardo self-assigned this Jul 21, 2026

bsardo reviewed Jul 23, 2026

View reviewed changes

przemkaczmarek added 2 commits July 25, 2026 09:55

fix comments

589dfea

fix comments

f017cf4

przemkaczmarek commented Jul 25, 2026

View reviewed changes

bsardo reviewed Jul 30, 2026

View reviewed changes

przemkaczmarek added 4 commits July 31, 2026 15:08

docs: remove GET interface reviewer README from repo

ea1e5a0

Merge remote-tracking branch 'origin/master' into GetInterface

86f4fa5

Uh oh!

Conversation

przemkaczmarek commented Jun 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

przemkaczmarek Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

przemkaczmarek Jul 31, 2026 •

edited

Loading