Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/HttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,12 @@ private function buildRequestOptions(
}

if ($body !== null) {
$requestOptions['json'] = $body;
// An empty array serializes to the JSON array `[]`, but JSON-object
// endpoints expect an object `{}`. This happens whenever a body is
// entirely optional and every field is omitted (e.g. challengeFactor
// on a TOTP factor). Cast to an object so the empty case encodes as
// `{}`; non-empty associative arrays already encode as objects.
$requestOptions['json'] = $body === [] ? (object) $body : $body;

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 Preserve array payloads This branch rewrites every top-level empty array body to an empty object. HttpClient::request() is public and can be used with arbitrary paths or custom base URLs, so a caller that intentionally sends an empty JSON list now sends {} instead of []. That changes the wire payload and can make endpoints with a top-level array schema reject the request or apply different semantics.

Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/HttpClient.php
Line: 232

Comment:
**Preserve array payloads** This branch rewrites every top-level empty array body to an empty object. `HttpClient::request()` is public and can be used with arbitrary paths or custom base URLs, so a caller that intentionally sends an empty JSON list now sends `{}` instead of `[]`. That changes the wire payload and can make endpoints with a top-level array schema reject the request or apply different semantics.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Devin Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is intentional. The WorkOS API has no endpoints that accept a top-level JSON array [] as a request body — all endpoints expect objects. The $body parameter in internal SDK methods (e.g. challengeFactor) is always built from associative arrays representing object fields, so an empty $body always means "no fields provided" (should be {}), never "send an empty list". The theoretical concern about arbitrary callers doesn't apply here since this SDK is purpose-built for the WorkOS API.

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.

Fair enough — since this SDK is purpose-built for the WorkOS API and all endpoints expect JSON objects, the concern about array payloads doesn't apply here. The fix is appropriate as-is.

}

return $requestOptions;
Expand Down
52 changes: 52 additions & 0 deletions tests/HttpClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,58 @@ public function testResolveUrlPreservesEncodedIdAsSingleSegment(): void
$this->assertSame('/organizations/om_xyz/foo', $rawSlashRequest->getUri()->getPath());
}

public function testEmptyBodySerializesAsJsonObject(): void
{
// Issue #400: an all-optional body with every field omitted reduces to
// an empty PHP array. Guzzle's `json` option encodes that to the JSON
// array `[]`, which JSON-object endpoints (e.g. challengeFactor on a
// TOTP factor) reject with a 422. The body must serialize to `{}`.
$mock = new MockHandler([
new Response(200, ['Content-Type' => 'application/json'], '{}'),
]);
$history = [];
$handler = HandlerStack::create($mock);
$handler->push(\GuzzleHttp\Middleware::history($history));

$client = new HttpClient(
apiKey: 'test_key',
clientId: null,
baseUrl: 'https://api.workos.com',
timeout: 10,
maxRetries: 0,
handler: $handler,
);

$client->request('POST', 'auth/factors/auth_factor_123/challenge', body: []);

$request = $history[array_key_last($history)]['request'];
$this->assertSame('{}', (string) $request->getBody());
}

public function testNonEmptyBodyStillSerializesAsJsonObject(): void
{
$mock = new MockHandler([
new Response(200, ['Content-Type' => 'application/json'], '{}'),
]);
$history = [];
$handler = HandlerStack::create($mock);
$handler->push(\GuzzleHttp\Middleware::history($history));

$client = new HttpClient(
apiKey: 'test_key',
clientId: null,
baseUrl: 'https://api.workos.com',
timeout: 10,
maxRetries: 0,
handler: $handler,
);

$client->request('POST', 'auth/factors/auth_factor_123/challenge', body: ['sms_template' => 'Your code is {{code}}']);

$request = $history[array_key_last($history)]['request'];
$this->assertSame('{"sms_template":"Your code is {{code}}"}', (string) $request->getBody());
}

public function testNonStringCodeFieldIsIgnored(): void
{
$body = json_encode([
Expand Down
Loading