Skip to content

Extend deepObject testing to include unicode#2381

Merged
mromaszewicz merged 1 commit into
oapi-codegen:mainfrom
mromaszewicz:fix/issue-2378
May 19, 2026
Merged

Extend deepObject testing to include unicode#2381
mromaszewicz merged 1 commit into
oapi-codegen:mainfrom
mromaszewicz:fix/issue-2378

Conversation

@mromaszewicz

Copy link
Copy Markdown
Member

Closes #2378

Since we test nearly everything in ASCII, we missed some failures in runtime around marshaling parameters. Extend the deepObject test to catch issues if they resurface.

@mromaszewicz mromaszewicz requested a review from a team as a code owner May 19, 2026 15:51
@greptile-apps

greptile-apps Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds unicode coverage for deepObject query parameter round-trips. The main changes are:

  • Bumps internal/test to github.com/oapi-codegen/runtime v1.4.1.
  • Adds a deepObject regression case with &, spaces, +, and Japanese text.
  • Updates internal/test/go.sum for the runtime dependency.

Confidence Score: 4/5

This is close, but one test path should be covered before merging.

  • The new shared test covers most router backends.

  • Echo v5 keeps a separate parameter round-trip suite and still only tests ASCII deepObject values.

  • The uncovered path uses the same generated query serialization pattern that this regression is meant to protect.

  • internal/test/parameters/echov5/echov5_param_test.go should get the same unicode and reserved-character deepObject case.

Important Files Changed

Filename Overview
internal/test/parameters/param_roundtrip_test.go Adds the shared unicode/reserved-character deepObject round-trip regression test.
internal/test/go.mod Bumps the internal test module to runtime v1.4.1.

Reviews (1): Last reviewed commit: "Extend deepObject testing to include uni..." | Re-trigger Greptile

Comment on lines +401 to +415
t.Run("deepObject with unicode and reserved chars", func(t *testing.T) {
adversarial := paramclient.ComplexObject{
Object: paramclient.Object{
FirstName: "filter&q=こんにちは",
Role: "admin role+with spaces",
},
Id: 12345,
IsAdmin: true,
}
params := paramclient.GetDeepObjectParams{DeepObj: adversarial}
req, err := paramclient.NewGetDeepObjectRequest(server, &params)
require.NoError(t, err)
var got paramclient.GetDeepObjectParams
doRoundTrip(t, req, &got)
assert.Equal(t, adversarial, got.DeepObj)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Echo v5 remains uncovered

This new regression only runs through the shared parameter round-trip suite. Echo v5 has its own separate internal/test/parameters/echov5/echov5_param_test.go suite with an ASCII-only deepObject case, while its generated client uses the same StyleParamWithOptions raw query fragment path. If unicode or reserved-character deepObject serialization regresses in that Echo v5 path, this test will still pass.

Closes oapi-codegen#2378

Since we test nearly everything in ASCII, we missed some failures in
runtime around marshaling parameters. Extend the deepObject test
to catch issues if they resurface.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@socket-security

socket-security Bot commented May 19, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedgithub.com/​oapi-codegen/​runtime@​v1.4.0 ⏵ v1.4.198100100100100

View full report

@mromaszewicz mromaszewicz added the bug Something isn't working label May 19, 2026
@mromaszewicz mromaszewicz merged commit 8c4029a into oapi-codegen:main May 19, 2026
15 checks passed
@mromaszewicz mromaszewicz deleted the fix/issue-2378 branch May 19, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update parameter binding/styling and round trip tests to catch URL breaking characters after decoding

1 participant