Escape path delimiters during reserved parameter expansion

MIGRATION.md

@@ -1,3 +1,16 @@
+# Path escaping in 1.1.x
+
+Path parameters with `allowReserved: true` now percent-encode `/`, `?`, `#`,
+`[`, and `]`. Versions 1.1.1 through 1.1.3 sent these characters unescaped.
+For example, `opa/examples` now becomes `opa%2Fexamples` in the request path.
+This follows the OpenAPI path rules and lets generated servers recover the
+original value within one route segment.
+
+Applications that used the raw-slash behavior for Open Policy Agent or other
+multi-segment endpoints must verify that the server accepts encoded slashes.
+There is no standard `allowReserved` option for sending raw path separators.
+Other valid reserved characters and existing percent-escapes are preserved.
+
 # Migrating from OpenAPI.jl 0.2.x to 1.0
 
 OpenAPI.jl 1.0 replaces the 0.2.x model — a runtime library consumed by code
@@ -46,7 +59,7 @@ OpenAPI.server("openapi.json"; name = "MyServer", path = "MyServer.jl")
 | `pre_request_hook`, `get_return_type` | `request_headers` / `request_options` keywords; typed responses come from the document |
 | Chunk readers (`LineChunkReader`, …) for streaming | `stream_to::Channel` keyword; framing follows the response media type, customizable with `codec!` |
 | `httplib = Downloads` or `HTTP` backends | HTTP.jl only |
-| `Client(url; escape_path_params = false)` | Declare `allowReserved: true` on the path parameter in the document; the generated client then leaves reserved characters such as `/` unescaped for that parameter. Requires a 3.0 or 3.2 document — 3.1 scopes `allowReserved` to query parameters and rejects it on a path parameter at load time |
+| `Client(url; escape_path_params = false)` | There is no equivalent for unescaped `/`, `?`, or `#` in path values: OpenAPI requires them to be percent-encoded. `allowReserved: true` preserves other valid reserved path characters and existing percent-escapes. The loader accepts this on 3.0 and 3.2 path parameters; 3.1 rejects it |
 | `pre_request_hook` rewriting the path to send `%2E` for `.` | `Client(url; escape_path_chars = ".")` percent-encodes the listed characters in every path parameter value on top of the standard RFC 3986 escaping |
 | Constructor/`setproperty!` validation, `val_format` overloads | Full JSON Schema validation at encode/decode time; disable per client with `validate_requests` / `validate_responses` |
 | `mutable struct` models, `haspropertyat` / `getpropertyat` | Immutable keyword-constructed structs; optional absent fields are `ABSENT` |

docs/src/clients.md

@@ -114,12 +114,13 @@ Generated clients support:
   plus the bracket-path `deepObject` extension for arrays and nested values
   (see [deepObject bracket paths](@ref));
 - `allowReserved`, `allowEmptyValue`, explode defaults, and parameter `content`.
-  `allowReserved: true` is honoured on path parameters too, so a
-  slash-delimited value such as an OPA document path is sent as-is instead of
-  with every `/` percent-encoded. OAS 3.2 documents this for path parameters
-  and 3.0 tolerates it, but 3.1 allows `allowReserved` only on query
-  parameters, so a 3.1 document that declares it on a path parameter fails
-  validation when the document is loaded;
+  OAS 3.2 permits `allowReserved: true` on path parameters, but path values
+  must still escape `/`, `?`, and `#`; `[` and `]` are also not valid path
+  characters. Other reserved characters, such as `:` and `@`, pass through.
+  Existing percent-escapes pass through too: `a%2Fb` decodes to `a/b`, so use
+  `a%252Fb` to send the literal text `a%2Fb`. The loader also accepts the field
+  on 3.0 path parameters; 3.1 restricts it to query parameters and rejects it
+  on a path parameter. See the [OAS 3.2 path templating rules](https://spec.openapis.org/oas/v3.2.0.html#path-templating).
 - JSON and structured-suffix JSON media types;
 - text and binary bodies;
 - `application/x-www-form-urlencoded` bodies;

docs/src/servers.md

@@ -121,12 +121,12 @@ the selected JSON schema accepts null. A full `HTTP.Response` bypasses
 generated status, header, and body validation. The handler owns that
 validation.
 
-One client capability has no server counterpart yet: a path parameter declared
-`allowReserved: true`. Generated clients send such a value with its reserved
-characters intact, so a slash-delimited value spans several path segments, but
-generated servers register the path template as written and `HTTP.Router`
-matches `{name}` against a single segment. Those requests reach the router as
-`404`s rather than the handler.
+Path parameters occupy one route segment, including those declared with
+`allowReserved: true`. Generated clients percent-encode `/`, `?`, and `#`
+inside path values, so `opa/examples/public servers` is sent as
+`opa%2Fexamples%2Fpublic%20servers` and decoded back to its original value by
+its generated server. This also works when the parameter precedes another
+path segment, such as `/documents/{path}/versions`.
 
 ### deepObject bracket paths
 

src/runtime.jl

@@ -1316,18 +1316,10 @@ function _join_object(value, pair_delimiter, key_delimiter)
     )
 end
 
-# `allow_reserved` is honoured for path parameters as well as query parameters.
-# OAS 3.2 lists `allowReserved` under the path-parameter branch of the Parameter
-# Object (`styles-for-path` in `schemas/oas-3.2.json`), so this is conformant,
-# not an extension: reserved characters go on the wire as-is. It matters for
-# APIs whose path parameters are themselves slash-delimited paths (OPA data
-# documents, proxied object paths) — without it every `/` becomes `%2F` and the
-# server sees a single segment.
-#
-# Version caveat: 3.0 tolerates the field on a path parameter and 3.2 blesses
-# it, but 3.1 scopes it to `in: query` under `unevaluatedProperties: false`, so
-# a 3.1 document that declares it fails document validation outright (even with
-# `strict = false`) rather than reaching this code.
+# OAS 3.2 allows reserved expansion in path values, but still forbids raw
+# `/`, `?`, and `#` there. RFC 3986 also excludes `[` and `]` from path
+# segments. Preserve other reserved characters and existing percent-escapes.
+# A literal percent-escape must itself be encoded (e.g. `%252F` for `%2F`).
 #
 # `escape_chars` names characters to percent-encode on top of RFC 3986 escaping
 # (`Client(escape_path_chars = ...)`). Unreserved characters such as `.` are
@@ -1338,7 +1330,10 @@ end
 # style delimiters (`.` for `label`, `;` and `=` for `matrix`) and the
 # parameter name from the path template stay literal.
 _path_scalar(value; allow_reserved::Bool = false, escape_chars::AbstractString = "") =
-    _percent_encode_chars(_escape(_scalar(value); allow_reserved), escape_chars)
+    _percent_encode_chars(
+        _escape(_scalar(value); allow_reserved),
+        allow_reserved ? "/?#[]" * escape_chars : escape_chars,
+    )
 
 function _percent_encode_chars(text::String, chars::AbstractString)
     isempty(chars) && return text
@@ -1620,9 +1615,10 @@ function _append_parameter!(client, path, query, headers, cookies, descriptor, v
                 path,
                 "{" * descriptor.name * "}" => (
                     preencoded ? serialized :
-                    _percent_encode_chars(
-                        _escape(serialized; allow_reserved = descriptor.allow_reserved),
-                        client.escape_path_chars,
+                    _path_scalar(
+                        serialized;
+                        allow_reserved = descriptor.allow_reserved,
+                        escape_chars = client.escape_path_chars,
                     )
                 ),
             )

test/runtime.jl

@@ -434,17 +434,31 @@
                        string(char) : "%" * uppercase(string(byte; base = 16, pad = 2))
             @test invoke(:_escape, String([byte])) == expected
         end
-        # allowReserved on a path parameter keeps slash-delimited values intact
+        # Reserved expansion must still escape forbidden path characters
         @test invoke(:_path_parameter, "path", "opa/examples/public servers", :simple, false) ==
               "opa%2Fexamples%2Fpublic%20servers"
         @test invoke(:_path_parameter, "path", "opa/examples/public servers", :simple, false;
-                     allow_reserved = true) == "opa/examples/public%20servers"
+                     allow_reserved = true) == "opa%2Fexamples%2Fpublic%20servers"
         @test invoke(:_path_parameter, "path", ["a/b", "c d"], :simple, false;
-                     allow_reserved = true) == "a/b,c%20d"
+                     allow_reserved = true) == "a%2Fb,c%20d"
         @test invoke(:_path_parameter, "path", "a/b", :label, false; allow_reserved = true) ==
-              ".a/b"
+              ".a%2Fb"
         @test invoke(:_path_parameter, "path", "a/b", :matrix, false; allow_reserved = true) ==
-              ";path=a/b"
+              ";path=a%2Fb"
+        @test invoke(:_path_parameter, "path", Dict("a/b" => "c?d#e[f]"), :simple, true;
+                     allow_reserved = true) == "a%2Fb=c%3Fd%23e%5Bf%5D"
+        @test invoke(:_path_parameter, "path", ":@!\$&'()*+,;=", :simple, false;
+                     allow_reserved = true) == ":@!\$&'()*+,;="
+        content_path = (
+            location = :path, style = :none, explode = false,
+            name = "path", allow_reserved = true,
+            content = ((media_type = "text/plain",),),
+        )
+        @test invoke(
+            :_append_parameter!, C.DEFAULT_CLIENT, "/documents/{path}",
+            Tuple{String,String,Bool,Bool}[], Pair{String,String}[],
+            Tuple{String,String,Bool,Bool}[], content_path, "a/b?c#d",
+        ) == "/documents/a%2Fb%3Fc%23d"
         # escape_path_chars percent-encodes extra characters after standard
         # escaping (Rails routes treat a literal `.` as a format suffix)
         @test invoke(:_path_parameter, "id", "acme.example.com-42", :simple, false) ==
@@ -464,9 +478,9 @@
               ";v.1=a%2Eb"
         @test invoke(:_path_parameter, "id", Dict("k.1" => "v.2"), :matrix, true;
                      escape_chars = ".") == ";k%2E1=v%2E2"
-        # composes with allowReserved: reserved characters pass, listed ones are encoded
+        # Extra escaping composes with required path escaping
         @test invoke(:_path_parameter, "path", "a/b.c", :simple, false;
-                     allow_reserved = true, escape_chars = ".") == "a/b%2Ec"
+                     allow_reserved = true, escape_chars = ".") == "a%2Fb%2Ec"
         # a listed unreserved character that is already reserved-escaped is unaffected
         @test invoke(:_path_parameter, "id", "a b", :simple, false; escape_chars = " ") ==
               "a%20b"

test/runtime_integration.jl

@@ -856,17 +856,15 @@ end
         client = C.Client()
 
         @testset "allowReserved on a path parameter" begin
-            # A slash-delimited document path (OPA style) must reach the server
-            # as path segments, not as one %2F-joined segment; other unsafe
-            # characters are still percent-encoded.
+            # A path value stays in one segment even with reserved expansion.
             result = call(
                 :getdocument,
                 "opa/examples/public servers";
                 client,
                 with_http_info = true,
             )
             request = take_request()
-            @test request.target == "/documents/opa/examples/public%20servers"
+            @test request.target == "/documents/opa%2Fexamples%2Fpublic%20servers"
             @test result.status == 200
             @test result.body["ok"] === true
         end
@@ -891,9 +889,9 @@ end
             @test result.status == 200
             @test result.body["ok"] === true
 
-            # composes with allowReserved: `/` still passes, `.` is encoded
+            # Extra escaping composes with required path escaping
             call(:getdocument, "opa/v1.2/data"; client = dotted_client)
-            @test take_request().target == "/documents/opa/v1%2E2/data"
+            @test take_request().target == "/documents/opa%2Fv1%2E2%2Fdata"
         end
 
         @testset "parameters, servers, and request overrides" begin

test/servergen.jl

@@ -398,6 +398,74 @@ empty200(req) = nothing
 nullout(req) = nothing
 """
 
+@testset "reserved path parameter round trips" begin
+    for version in ("3.0.3", "3.2.0")
+        paths = OpenAPI.obj()
+        implementations = String[]
+        for style in ("simple", "label", "matrix"), suffix in ("", "/versions")
+            id = style * (isempty(suffix) ? "tail" : "middle")
+            paths["/$id/{path}$suffix"] = OpenAPI.obj(
+                "get" => OpenAPI.obj(
+                    "operationId" => id,
+                    "parameters" => Any[server_parameter(
+                        "path", "path", OpenAPI.obj("type" => "string");
+                        required = true, style, allowReserved = true,
+                    )],
+                    "responses" => OpenAPI.obj("200" => OpenAPI.obj(
+                        "description" => "decoded path",
+                        "content" => OpenAPI.obj("text/plain" => OpenAPI.obj(
+                            "schema" => OpenAPI.obj("type" => "string"),
+                        )),
+                    )),
+                ),
+            )
+            push!(implementations, "$id(request, path) = path")
+        end
+        document = OpenAPI.obj(
+            "openapi" => version,
+            "info" => OpenAPI.obj("title" => "Reserved paths", "version" => "1"),
+            "paths" => paths,
+        )
+        host = Module(:ReservedPathHost)
+        Base.include_string(host, OpenAPI.server(document; name = "ReservedServer"))
+        Base.include_string(host, OpenAPI.client(document; name = "ReservedClient"))
+        impl = Module(:ReservedPathImpl)
+        Base.include_string(impl, join(implementations, "\n"))
+        router = HTTP.Router()
+        target = Ref("")
+        middleware = handler -> request -> begin
+            target[] = request.target
+            handler(request)
+        end
+        Base.invokelatest(host.ReservedServer.register!, router, impl; middleware)
+        server = HTTP.serve!(router, "127.0.0.1", 0; verbose = false)
+        try
+            Base.invokelatest(host.ReservedClient.server!, "http://127.0.0.1:$(HTTP.port(server))")
+            for style in ("simple", "label", "matrix"), suffix in ("", "/versions")
+                id = style * (isempty(suffix) ? "tail" : "middle")
+                prefix = style == "label" ? "." : style == "matrix" ? ";path=" : ""
+                for (value, wire, decoded) in (
+                    ("opa/examples/public servers", "opa%2Fexamples%2Fpublic%20servers", "opa/examples/public servers"),
+                    ("a?b#c[d]", "a%3Fb%23c%5Bd%5D", "a?b#c[d]"),
+                    ("a:b@c", "a:b@c", "a:b@c"),
+                    ("a%2Fb", "a%2Fb", "a/b"),
+                    ("a%252Fb", "a%252Fb", "a%2Fb"),
+                )
+                    response = Base.invokelatest(
+                        getfield(host.ReservedClient, Symbol(id)), value;
+                        with_http_info = true,
+                    )
+                    @test response.status == 200
+                    @test response.body == decoded
+                    @test target[] == "/$id/$prefix$wire$suffix"
+                end
+            end
+        finally
+            close(server)
+        end
+    end
+end
+
 @testset "server generation" begin
     server_source = OpenAPI.server(SERVER_ROUNDTRIP_DOCUMENT; name = "RoundTripServer")
     @test server_source == OpenAPI.server(SERVER_ROUNDTRIP_DOCUMENT; name = "RoundTripServer")