Repository navigation
[dart-dio] fix: URI-encode path parameters - #25544
Conversation
The query_param helpers pass strings through unchanged (built_value) or interpolate raw values (freezed, json_serializable), so path parameters containing '/', spaces or other reserved characters broke the request path. Substitute path params through a new api/path_param partial that wraps the value in Uri.encodeComponent for all three serialization libraries. Fixes OpenAPITools#25527
There was a problem hiding this comment.
1 issue found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/dart/dio/DartDioClientCodegenTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/dart/dio/DartDioClientCodegenTest.java:499">
P2: These independent substring assertions do not prove that path parameters are encoded. Assert the complete placeholder replacement expression, or exercise a generated client with a value containing `/` or spaces and verify the resulting request path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
|
||
| Path petApi = output.toPath().resolve("lib/src/api/pet_api.dart"); | ||
| String content = Files.readString(petApi, StandardCharsets.UTF_8); | ||
| Assert.assertTrue(content.contains(".replaceAll("), content); |
There was a problem hiding this comment.
P2: These independent substring assertions do not prove that path parameters are encoded. Assert the complete placeholder replacement expression, or exercise a generated client with a value containing / or spaces and verify the resulting request path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/test/java/org/openapitools/codegen/dart/dio/DartDioClientCodegenTest.java, line 499:
<comment>These independent substring assertions do not prove that path parameters are encoded. Assert the complete placeholder replacement expression, or exercise a generated client with a value containing `/` or spaces and verify the resulting request path.</comment>
<file context>
@@ -471,6 +471,37 @@ public void testFreezedIgnoresUseOptional() throws IOException {
+
+ Path petApi = output.toPath().resolve("lib/src/api/pet_api.dart");
+ String content = Files.readString(petApi, StandardCharsets.UTF_8);
+ Assert.assertTrue(content.contains(".replaceAll("), content);
+ Assert.assertTrue(content.contains("Uri.encodeComponent("),
+ "path parameter should be URI-encoded in " + serializationLibrary + ": " + content);
</file context>
There was a problem hiding this comment.
Strengthened in 5301a77: testPathParamsAreUriEncoded now iterates every generated *_api.dart file and asserts the count of Uri.encodeComponent( calls equals the count of .replaceAll( path substitutions — so any future path substitution added without encoding fails the test. 20 tests, 0 failures.
source_helper 1.4.0 (published 2026-10-19) added extension members
that clash with json_serializable 6.x's internal copies, breaking
compilation of the build_runner build script ('toStringNonNullable is
defined in multiple extensions'). The json_serializable variant is
unaffected because copy_with_extension_gen's source_gen constraint
holds source_helper below 1.4.0; the freezed variant has no such
guard and floated to 1.4.0.
Verified with Dart 3.9: pub get resolves source_helper 1.3.13,
build_runner builds, dart test passes (190 tests).
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
- render Uri.encodeComponent on one line so generated paths keep the previous single-line style - assert in testPathParamsAreUriEncoded that the number of Uri.encodeComponent calls equals the number of path substitutions in every generated api class, not just that one occurrence exists Addresses review feedback in OpenAPITools#25544.
- render Uri.encodeComponent on one line so generated paths keep the previous single-line style - assert in testPathParamsAreUriEncoded that the number of Uri.encodeComponent calls equals the number of path substitutions in every generated api class, not just that one occurrence exists Addresses review feedback in OpenAPITools#25544.
source_helper 1.3.9+ requires Dart 3.9, but the freezed variant advertises sdk >=3.8.0 - 1.3.8 (source_gen >=3.1.0 <5.0.0, Dart ^3.7.0) keeps 3.8 users able to resolve while 3.9+ users still get 1.3.13. Addresses review feedback in OpenAPITools#25544.
source_helper 1.3.9+ requires Dart 3.9, but the freezed variant advertises sdk >=3.8.0 - 1.3.8 (source_gen >=3.1.0 <5.0.0, Dart ^3.7.0) keeps 3.8 users able to resolve while 3.9+ users still get 1.3.13. Addresses review feedback in OpenAPITools#25544.
|
thanks for the PR cc @jaumard (2018/09) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12) @ahmednfwela (2021/08) |
|
lgtm. let's give it a try |
Fixes #25527.
The
dart-diogenerator built request paths by substituting path parameters through theapi/query_paramhelper — which (forbuilt_value) passes strings through unchanged and (forfreezed/json_serializable) interpolates the raw value. A path parameter containing/, spaces, or other reserved characters was therefore sent unencoded and broke the request path (e.g. a scanned QR-code URL as{bin}→ 404).What changed
serialization/<library>/api/path_param.mustachefor all three serialization libraries (built_value,freezed,json_serializable) wrapping the existing query_param rendering inUri.encodeComponent(...)— suggested in the issue and credited to the reporter.api.mustachenow substitutes path parameters viaapi/path_paraminstead ofapi/query_param.Uri.encodeComponentencodes the value as a single path component (including/→%2F), so values containing slashes stay within one segment — exactly the failing scenario from the issue.Verification
testPathParamsAreUriEncodedgenerates the petstore client for all three serialization libraries and assertsUri.encodeComponent(appears in the generated API classes.DartDioClientCodegenTest: 20 tests, 0 failures.dart-diosample configs; every path substitution now goes throughUri.encodeComponent.dart analyzeon the regeneratedbuilt_valuepetstore sample: no issues (one pre-existing unrelatedunused_importwarning).PR checklist
./bin/generate-samples.sh(alldart-dioconfigs); no files deleted.masterbranch.Summary by cubic
Fixes #25527. Generated
dart-dioclients now URI-encode path parameters, so values containing/or spaces no longer break the request path.api/path_parampartial for all three serialization libraries (built_value,freezed,json_serializable), wrapping the value inUri.encodeComponenton a single line to keep the generated path style unchanged.api.mustachenow uses this partial for path parameters instead of the query-param rendering.source_helperto>=1.3.8 <1.4.0in thefreezedvariant'spubspec.mustache; version 1.4.0 added extension members that clash withjson_serializable6.x and break thebuild_runnerbuild script, while 1.3.9+ requires Dart 3.9 (above the variant's advertised >=3.8.0).Written for commit 594a4aa. Summary will update on new commits.