fix(request): decode Query and ContentFields values exactly once by freitasjca · Pull Request #570 · HashLoad/horse

freitasjca · 2026-09-14T10:12:11Z

Problem

Req.Query[...] and Req.ContentFields[...] decode a value that was already URL-decoded, and do it again on every read. With the default Indy provider:

Request	`Req.Query['v']` today	Expected
`?v=100%25`	`EConvertError: Error decoding URL style (%XX) encoded string at position 4` → HTTP 500	`100%`
`?v=50%25off`	`EConvertError: Invalid URL encoded character (%of) at position 3` → HTTP 500	`50%off`
`?v=a%2B%2541`	`a A` (no error, wrong value)	`a+%41`

It was reported to us as an intermittent HTTP 500. It's intermittent because DecodeParam skips values without a %, so it only fails when the data contains one.

Cause

Every path that fills these collections already stores decoded values:

InitializeQuery calls DecodeParam on each key and value before storing them.
WebBroker's ContentFields are decoded by ExtractHTTPFields.
Providers that fill the dictionaries themselves store decoded values.

Since 3.3.0, THorseCoreParam.GetItem / TryGetValue also run DecodeParam on read, then write the result back, so each repeated read decodes again. Field(...).AsString reads the dictionary directly and always returned the right value, so two accessors disagree on the same request.

Fix

InitializeQuery and InitializeContentFields now create THorseCoreParam with ADecodeValues = False, which InitializeParams already does. Two lines in src/Horse.Request.pas, no other behaviour change.

Tests

tests/src/tests/Tests.Horse.Request.DecodeOnce.pas (unit): stores decoded values in Query, ContentFields and Params and checks that Field, [v], a repeated [v] and TryGetValue return them unchanged.
tests/src/tests/Tests.Integration.QueryDecode.pas (integration, port 9126): the table above through the default provider, plus a form-urlencoded PUT.
Both registered in Console.dpr.

Validated on Windows / Delphi 12 (Indy):

On 3.3.5 without the fix, the 10 expected new tests fail.
With the fix, all 13 new tests pass (202 → 212 of 221). The remaining failures in the suite are identical with and without this change.

Not run on FPC. The change is the same constructor call InitializeParams already makes.

Fixes #569

InitializeQuery and InitializeContentFields now create THorseCoreParam with ADecodeValues=False, as InitializeParams already does. Every store path already holds DECODED values: InitializeQuery runs DecodeParam on each key and value, WebBroker ContentFields are decoded by ExtractHTTPFields, and non-WebBroker providers store decoded values. But GetItem/TryGetValue decoded AGAIN on every read and wrote the result back: ?v=100%25 -> EConvertError "Error decoding URL style (%XX) encoded string" ?v=50%25off -> EConvertError "Invalid URL encoded character (%of)" ?v=a%2B%2541 -> silently "a A" instead of "a+%41" Field(...).AsString read the stored value and was always correct. This affects the default Indy provider too. User-reported as an intermittent HTTP 500. Validated on Windows/Delphi with a three-way read test (Field, [v], [v] again): 131 passed / 8 failed before, 139/139 after, on top of 3.3.5.

Tests.Horse.Request.DecodeOnce (unit, no network): stores already-decoded values in THorseRequest.Query / ContentFields / Params and asserts that Field, the first indexed read, a repeated indexed read and TryGetValue all return them unchanged. Cases: '100%', '50%off', 'a+%41', plus a no-percent control. 'a+%41' fails without the fix on FPC too, where HTTPDecode may not raise. Tests.Integration.QueryDecode (port 9126): GET ?v=100%25 / 50%25off / a%2B%2541 and a form-urlencoded PUT v=100%25 through the default provider; the handler reads each value twice and then via Field. Validated on Windows/Delphi: 10 new failures on 3.3.5 (EConvertError "Error decoding URL style (%XX)", "Invalid URL encoded character", silent "a A", HTTP 500s), all 13 new tests pass with the fix. The remaining suite failures are identical with and without it.

… pairs FIX-DECODE-ONCE-1 companion to HashLoad/horse#570. Query and application/x-www-form-urlencoded keys and values are now URL-decoded when stored, like every other Horse provider. The decoder is tolerant: malformed %xx is kept, + is a space, and invalid UTF-8 falls back to the raw text, because it runs outside any handler's try/except. Also fixes form-urlencoded parsing: the WHOLE "name=value" text was stored as the key with an empty value, so Req.ContentFields['name'] always returned ''. Tests 41-45 (same as horse-provider-crosssocket), plus the HARNESS-CB-1 test client fix (SetEvent in finally, callback exceptions reported as status -1). Validated on Windows/Delphi: 15 new failures before the fix, 124/124 after, with the #570 Horse fix. Against an unpatched Horse, Req.Query[...] still double-decodes, but Field() and form fields are now correct.

FIX-DECODE-ONCE-1 companion to HashLoad/horse#570 (Horse.Provider.ICS.Request): query and form-urlencoded keys and values are decoded once at store time with a tolerant decoder. Form parsing stored the whole "name=value" as the key, so Req.ContentFields['name'] was always ''. FIX-ICS-UTF8-BODY (Horse.Provider.ICS): ICS's AnswerString encodes the body with TEncoding.Default, which is ANSI on Windows, so non-ASCII response characters went out as ANSI under charset=utf-8. All three call sites now use AnswerBodyTB(TEncoding.UTF8.GetBytes(Body)), public since ICS 9.2 and on the same AnswerStream path. POSIX was unaffected. Tests: 41-45 added. HARNESS-CB-1 test client fix (SetEvent in finally, callback exceptions reported as status -1). Test 15 now sends Content-Length: 0: TCrossHttpClient omits the header for an empty body, and ICS rejects a POST without it with its own HTML 400 before the provider runs (curl: 10/10), which left the test's body check unreachable. Validated on Windows/Delphi: 117/117 with the #570 Horse fix.

Check 7 read Req.Query.Content. The stored query values are already URL-decoded, and on Horse 3.3.0-3.3.5 Content decodes every value again. A value holding a literal percent sign (?k=100%25, stored as "100%") made that second decode raise EConvertError, "Error decoding URL style (%XX) encoded string", so a valid request got a 500 before any route ran. The check is on by default (2048/2048), so every app using the guard was affected, and reading values with Field() in the handler could not help. The check now iterates Req.Query.Dictionary, which returns the stored pairs as they are. Dictionary exists since Horse 3.0.0, so the >=3.0.0 floor holds, and the result is the same with or without HashLoad/horse#570. Splitting key from value is no longer needed, so a key containing '=' is measured correctly too. Adds the integration suite under samples/tests (server on 127.0.0.1:9200 plus a CrossSocket client), with test 13: GET /ping?k=100%25 -> 200. Also fixes the server's header comment, where a compiler directive written inside a brace comment closed it early and kept the server from compiling. Validated on Windows/Delphi with a control matrix: Horse 3.3.5 Horse.Request.pas + old guard -> 16 passed, 2 failed (test 13) Horse 3.3.5 Horse.Request.pas + new guard -> 18 passed Horse with #570 + new guard -> 18 passed

regyssilveira · 2026-09-15T18:45:40Z

Obrigado pela PR e pelos testes. Complementei a correção na sua branch com a decodificação dos campos de formulário no provedor IOCP e com a aceitação de Content-Type contendo charset no Delphi 10. Também ampliei os testes de formulário para cobrir percentuais literais e sequências que poderiam ser decodificadas novamente. Os 14 testes direcionados passaram em Delphi 10 e Delphi 12 (IOCP); o núcleo compilou em FPC/Linux via Docker. A suíte completa ainda apresenta falhas em testes de integração não relacionados, portanto a validação aqui é dos casos direcionados e das compilações. A PR agora referencia a issue #569 para fechá-la no merge.

freitasjca added 3 commits September 14, 2026 10:35

test(request): bind the decode-once integration server to 127.0.0.1

166afe6

freitasjca mentioned this pull request Sep 14, 2026

fix(request): decode query/form values once at store time; parse form pairs freitasjca/horse-provider-mormot#2

Merged

fix(request): decode IOCP form fields once across Delphi versions

20080ea

regyssilveira merged commit 017dd72 into HashLoad:master Sep 15, 2026

regyssilveira mentioned this pull request Sep 15, 2026

chore: prepare Horse 3.3.6 release #571

Merged

Navigation Menu

Uh oh!

fix(request): decode Query and ContentFields values exactly once - #570

fix(request): decode Query and ContentFields values exactly once#570
regyssilveira merged 4 commits into
HashLoad:masterfrom
freitasjca:fix/decode-once

freitasjca commented Sep 14, 2026 •

edited by regyssilveira

Loading

Uh oh!

regyssilveira commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

freitasjca commented Sep 14, 2026 • edited by regyssilveira Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Cause

Fix

Tests

Uh oh!

regyssilveira commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

freitasjca commented Sep 14, 2026 •

edited by regyssilveira

Loading