Skip to content

feat(php): add encode-path-params config to percent-encode path parameters - #17294

Merged
willkendall01 merged 3 commits into
mainfrom
devin/1785416232-php-encode-path-params
Aug 2, 2026
Merged

feat(php): add encode-path-params config to percent-encode path parameters#17294
willkendall01 merged 3 commits into
mainfrom
devin/1785416232-php-encode-path-params

Conversation

@fern-api

@fern-api fern-api Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description

Refs Auth0 SEC-16202 (Pylon #22523)

The PHP SDK generator interpolated path parameter values straight into the path template
(path: "/users/{$id}"), so a value could inject its own separators. Reported and verified live by
Auth0's security team: users->get('../connections') builds users/../connections, which normalizes
to a different Management API endpoint than the method targets — bounded only by the token's scopes.

This cannot be fixed in RawClient::buildUrl(): by then BaseApiRequest::$path is one assembled
string, so the template's separators and value-supplied separators are indistinguishable. Encoding
must happen where the value is substituted, i.e. in the emitted client code.

TypeScript (encodeURIComponent), Go (url.PathEscape), Java (OkHttp addPathSegment) and C#
(Uri.EscapeDataString) already encode path params; this brings PHP in line.

Changes Made

  • generators/php/sdk/src/core/RawClient.ts: getPathString now returns an AST node that emits
    "/users/" . RawClient::encodePathParam($id) instead of "/users/{$id}", keeping literal /
    separators unencoded (and dropping the interpolatable-vs-concatenated special case, since every
    value now goes through a call).
  • generators/php/base/src/asIs/Client/RawClient.Template.php: new RawClient::encodePathParam()
    rawurlencode for scalars/stringables, true/false for bools, '' for null, JSON for anything
    else.
  • Added testEncodePathParam / testEncodedPathParamDoesNotTraverse to the generated
    RawClientTest.
  • Reseeded seed/php-sdk.
  • Changelog: generators/php/sdk/changes/unreleased/encode-path-parameters.yml (fix).

Behavior note for reviewers: any API that intentionally accepts slash-containing path parameter
values (file-path-like ids) will now send %2F. That is the same behavior as the other generators,
but it is a visible change for existing PHP users — flagging in case you want it gated.

The Python generator is affected too (encode_path_param stringifies but does not percent-encode);
that fix is going out in a separate PR.

Testing

  • Unit tests added/updated (generated RawClientTest cases; pnpm turbo run test --filter @fern-api/php-sdk passes)
  • Manual testing completed — seed test --generator php-sdk --local --skip-scripts: 155/155 fixtures pass; verified encodePathParam output with php locally (../connections..%2Fconnections, 4242, truetrue, null'').

Open in Devin Review

Update: gated behind encode-path-params (opt-in)

Per review, encoding is now off by default and opted into with a generator config flag, so no
existing PHP SDK user's output changes until they enable it:

- name: fernapi/fern-php-sdk
  config:
    encode-path-params: true
  • SdkCustomConfig: new encode-path-params (default false).
  • RawClient.getPathString keeps the previous unencoded interpolation path when the flag is off, and
    emits "/users/" . RawClient::encodePathParam($id) when on. RawClient::encodePathParam() is
    always present in the generated core.
  • New seed output folder seed/php-sdk/path-parameters/encode-path-params covering the flag; all
    other fixtures are byte-identical to main again.
  • Changelog reclassified fix -> feat.

Reseed: 156/156 php-sdk seed fixtures pass; pnpm turbo run test --filter @fern-api/php-sdk green.
Flipping the default later should go through packages/generator-migrations per CLAUDE.md.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@nitpickybot nitpickybot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed the changes — everything looks good. No issues found.

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread generators/php/base/src/asIs/Client/RawClientTest.Template.php
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-07-31T05:08:23Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
php-sdk square 78s (n=5) N/A 52s -26s (-33.3%)

main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-07-31T05:08:23Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-07-31 15:57 UTC

…nfig

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fern-api fern-api Bot changed the title fix(php): percent-encode path parameters in generated clients feat(php): add encode-path-params config to percent-encode path parameters Jul 30, 2026
…elog

Co-Authored-By: Claude <noreply@anthropic.com>
@willkendall01
willkendall01 merged commit 11da132 into main Aug 2, 2026
76 checks passed
@willkendall01
willkendall01 deleted the devin/1785416232-php-encode-path-params branch August 2, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants