fix:修复 Codex API 测试参数兼容性问题与增加支持流式测试#185
Conversation
新增对流式响应（如 SSE 和 NDJSON）的识别逻辑，在检测到流式响应时返回明确提示信息。 同时优化 JSON 响应解析流程，增加对非法 JSON 格式的捕获与日志记录。 此外，调整 OpenAI Responses API 测试输入格式，确保符合其接口规范。
某些中转服务不支持 max_output_tokens 参数，导致测试失败。根据注释说明， 该参数在当前上下文中并非必需，因此将其移除以确保与所有服务兼容。
新增对 SSE 和 NDJSON 格式流式响应的解析能力，包括： - 添加 `streamInfo` 字段以记录接收到的数据块数量及格式 - 实现 `parseSSEText` 和 `parseStreamResponse` 函数用于解析流式数据 - 引入 `mergeStreamChunks` 方法将多个响应块合并为完整响应 - 更新 API 测试逻辑以正确处理并展示流式响应结果 - 增强错误处理机制以应对流式响应解析失败的情况
- 调整 API 测试配置中响应内容预览最大长度从 100 到 500 字符 - 在测试结果中新增 streamInfo 字段用于记录流式响应数据 - UI 上增加流式响应信息的展示区域，包括接收到的数据块数量和流格式 - 支持在结果详情和错误信息区域显示流式响应相关统计
Summary of ChangesHello @Silentely, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在增强供应商 API 连接测试的健壮性和功能性。它通过解决 Codex API 测试中因参数不兼容导致的错误，并引入对流式响应的测试支持，确保了更广泛的 API 能够被准确有效地验证。这些改进提升了系统与各种第三方服务集成的可靠性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
这个 PR 的核心目标是修复 Codex API 测试的兼容性问题，并增加了对流式响应的测试支持，完成度很高。主要改进包括：
- 移除了
testProviderOpenAIResponses中不兼容的max_output_tokens参数，并修正了input的格式，解决了核心的兼容性问题。 - 在
executeProviderApiTest中增加了对流式响应（SSE）的处理逻辑，包括基于Content-Type的判断和对内容格式的启发式检测，这大大增强了测试的健壮性。 - 新增了
parseStreamResponse、parseSSEText和mergeStreamChunks等函数来解析和合并流式数据块，并能适配多种 API 的响应格式。 - 前端相应地更新了 UI，可以展示流式测试的结果，例如接收到的数据块数量。
代码整体质量不错，但新增加的流式解析函数中存在一些可以优化的地方，主要集中在代码重复和错误处理方面。我在具体的代码行上提出了一些建议，希望能帮助提升代码的可维护性。
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review - In Progress
I'm conducting a detailed code review focusing on correctness, security, and reliability issues. Review comments will be submitted shortly.
Initial Analysis: Found several critical and high-severity issues related to:
- Resource management in stream parsing
- Type safety concerns
- Error handling edge cases
Detailed inline comments coming next...
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
Code review with critical and high severity issues found
|
|
||
| // 跳过 [DONE] 标记 | ||
| if (dataContent === "[DONE]") { | ||
| continue; |
There was a problem hiding this comment.
🟠 High: Potential Type Safety Violation in Stream Merging
Why this is a problem: In mergeStreamChunks (line 1048+), when merging text content back into response objects, the code performs unsafe type assertions without validating the response structure. For example:
(base as OpenAIResponsesResponse).output = [...]If base does not actually match OpenAIResponsesResponse structure (e.g., it's an Anthropic response but we're in the OpenAI Responses branch), this creates invalid response objects that will fail extraction.
Suggested fix:
// OpenAI Responses API - only update if this is actually a Responses API response
if ("output" in base && Array.isArray(base.output)) {
const firstOutput = base.output[0];
(base as OpenAIResponsesResponse).output = [
{
type: "message",
id: firstOutput?.id || "msg_" + Date.now(),
status: firstOutput?.status || "completed",
role: "assistant",
content: [{ type: "output_text", text: mergedText }],
},
];
}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
Security and correctness issues
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
🔒 Security Scan Results - CRITICAL VULNERABILITIES FOUND
🚨 Vulnerabilities Found
- Critical (🔴): 1 vulnerability
- High (🟠): 1 vulnerability
- Medium (🟡): 0 weaknesses
- Low (🟢): 0 informational findings
⚡ IMMEDIATE ACTIONS REQUIRED
🔴 CRITICAL: Uncontrolled Resource Consumption (DoS) via Unbounded Stream Parsing
Location: src/actions/providers.ts lines 1055-1150
Security Impact:
An attacker with admin access can exhaust server memory by causing the server to parse an unbounded streaming response, leading to:
- Out-of-memory (OOM) crash
- Service unavailability for all users
- Potential for bypassing rate limits if memory exhaustion occurs before limit checks
Attack Scenario:
- Attacker gains admin access (or compromises admin account)
- Attacker configures a malicious provider URL pointing to their controlled server
- Malicious server responds with infinite SSE stream:
data: {...}\n\ndata: {...}\n\n...(never sends [DONE]) parseStreamResponse()continuously accumulates chunks in memory with no size limit:chunksarray grows unboundedbufferstring grows unboundedwhile (true)loop never terminates
- Server runs out of memory and crashes (DoS for all users)
Vulnerable Code:
async function parseStreamResponse(response: Response): Promise<StreamParseResult> {
const chunks: ProviderApiResponse[] = []; // ⚠️ No size limit
let buffer = "";
while (true) { // ⚠️ No maximum iteration count
const { done, value } = await reader.read();
if (done) break; // ⚠️ Attacker controls when 'done' is true
buffer += decoder.decode(value, { stream: true }); // ⚠️ Buffer grows unbounded
// ...
chunks.push(parsed); // ⚠️ Chunks array grows unbounded
}
}Same issue in parseSSEText() at lines 998-1051.
Remediation:
// Add to API_TEST_CONFIG
const API_TEST_CONFIG = {
TIMEOUT_MS: 10000,
MAX_RESPONSE_PREVIEW_LENGTH: 500,
TEST_MAX_TOKENS: 100,
TEST_PROMPT: "Hello",
// Add these limits:
MAX_STREAM_CHUNKS: 1000, // Max chunks to parse
MAX_STREAM_BUFFER_SIZE: 10 * 1024 * 1024, // 10MB max buffer
MAX_STREAM_ITERATIONS: 10000, // Prevent infinite loops
} as const;
async function parseStreamResponse(response: Response): Promise<StreamParseResult> {
if (\!response.body) {
throw new Error("响应体为空");
}
const reader = response.body.getReader();
const decoder = new TextDecoder();
const chunks: ProviderApiResponse[] = [];
let buffer = "";
let currentData = "";
let iterations = 0;
try {
while (true) {
// Prevent infinite loops
if (++iterations > API_TEST_CONFIG.MAX_STREAM_ITERATIONS) {
throw new Error(`流式响应超过最大迭代次数 (${API_TEST_CONFIG.MAX_STREAM_ITERATIONS})`);
}
// Prevent excessive chunks
if (chunks.length >= API_TEST_CONFIG.MAX_STREAM_CHUNKS) {
throw new Error(`流式响应超过最大数据块数量 (${API_TEST_CONFIG.MAX_STREAM_CHUNKS})`);
}
// Prevent buffer overflow
if (buffer.length > API_TEST_CONFIG.MAX_STREAM_BUFFER_SIZE) {
throw new Error(`流式响应缓冲区超过最大大小 (${API_TEST_CONFIG.MAX_STREAM_BUFFER_SIZE} 字节)`);
}
const { done, value } = await reader.read();
if (done) break;
buffer += decoder.decode(value, { stream: true });
// ... rest of parsing logic
}
} finally {
reader.releaseLock();
}
// ... rest of function
}
// Apply same limits to parseSSEText
function parseSSEText(text: string): StreamParseResult {
// Validate input size
if (text.length > API_TEST_CONFIG.MAX_STREAM_BUFFER_SIZE) {
throw new Error(`SSE 文本超过最大大小 (${API_TEST_CONFIG.MAX_STREAM_BUFFER_SIZE} 字节)`);
}
const lines = text.split("\n");
// Prevent excessive lines
if (lines.length > API_TEST_CONFIG.MAX_STREAM_ITERATIONS) {
throw new Error(`SSE 超过最大行数 (${API_TEST_CONFIG.MAX_STREAM_ITERATIONS})`);
}
const chunks: ProviderApiResponse[] = [];
let currentData = "";
for (const line of lines) {
// Prevent excessive chunks
if (chunks.length >= API_TEST_CONFIG.MAX_STREAM_CHUNKS) {
throw new Error(`SSE 超过最大数据块数量 (${API_TEST_CONFIG.MAX_STREAM_CHUNKS})`);
}
// ... rest of parsing logic
}
// ... rest of function
}References:
- CWE-400: Uncontrolled Resource Consumption
- CWE-770: Allocation of Resources Without Limits
- OWASP: Denial of Service
🟠 HIGH: Insecure Deserialization with Insufficient Validation
Location: src/actions/providers.ts lines 1017-1283 (multiple occurrences)
Security Impact:
While JSON.parse() itself is safe in JavaScript/TypeScript, the code parses untrusted streaming data from external providers without adequate validation, which could lead to:
- Injection of malicious data structures into the application
- Type confusion attacks if malformed data bypasses TypeScript type assertions
- Potential prototype pollution (though mitigated by modern JS engines)
- Resource exhaustion via deeply nested objects
Attack Scenario:
- Attacker controls a malicious provider endpoint
- Provider returns malicious SSE stream with crafted payloads:
data: {"__proto__": {"isAdmin": true}}
data: {"content": [{"text": "<script>alert('xss')</script>"}]}
data: {"usage": {"a": {"b": {"c": { ... 10000 levels deep ... }}}}}
JSON.parse(currentData) as ProviderApiResponseblindly trusts the parsed object- Type assertion (
as ProviderApiResponse) provides no runtime validation - Malicious data propagates through
mergeStreamChunks()and into application state
Vulnerable Code:
// parseSSEText (line 1017)
try {
const parsed = JSON.parse(currentData) as ProviderApiResponse; // ⚠️ No validation
chunks.push(parsed);
} catch {
// Silently ignores - could hide attacks
}
// parseStreamResponse (line 1100)
try {
const parsed = JSON.parse(currentData) as ProviderApiResponse; // ⚠️ No validation
chunks.push(parsed);
} catch {
// Silently ignores
}
// executeProviderApiTest (line 1601)
try {
result = JSON.parse(responseText) as ProviderApiResponse; // ⚠️ No validation
} catch (jsonError) {
// Only logs, doesn't validate structure
}Remediation:
// Add Zod schema for runtime validation
import { z } from 'zod';
// Define schemas for each API format
const AnthropicUsageSchema = z.object({
input_tokens: z.number().int().nonnegative(),
output_tokens: z.number().int().nonnegative(),
cache_creation_input_tokens: z.number().int().nonnegative().optional(),
cache_read_input_tokens: z.number().int().nonnegative().optional(),
});
const AnthropicResponseSchema = z.object({
id: z.string(),
type: z.literal('message'),
role: z.literal('assistant'),
content: z.array(z.object({
type: z.string(),
text: z.string().optional(),
})).max(100), // Prevent array DoS
model: z.string(),
usage: AnthropicUsageSchema.optional(),
}).passthrough(); // Allow other fields but validate critical ones
// ... similar schemas for OpenAI, Gemini, etc.
const ProviderResponseSchema = z.union([
AnthropicResponseSchema,
OpenAIChatResponseSchema,
OpenAIResponsesResponseSchema,
GeminiResponseSchema,
]);
// Safe parsing function with depth limit
function safeParseJSON(jsonString: string, maxDepth = 10): unknown {
const parsed = JSON.parse(jsonString);
// Check nesting depth to prevent stack overflow
function checkDepth(obj: unknown, depth = 0): void {
if (depth > maxDepth) {
throw new Error(`对象嵌套深度超过限制 (${maxDepth})`);
}
if (typeof obj === 'object' && obj \!== null) {
for (const value of Object.values(obj)) {
checkDepth(value, depth + 1);
}
}
}
checkDepth(parsed);
return parsed;
}
// Use in parseSSEText
try {
const parsed = safeParseJSON(currentData);
const validated = ProviderResponseSchema.parse(parsed); // Runtime validation
chunks.push(validated as ProviderApiResponse);
} catch (error) {
logger.warn('Invalid SSE chunk structure', {
error: error instanceof Error ? error.message : 'Unknown',
chunkPreview: clipText(currentData, 100),
});
// Don't silently ignore - log for security monitoring
continue; // Skip invalid chunk
}Alternative (Lightweight):
If Zod adds too much overhead, implement basic structural validation:
function validateProviderResponse(data: unknown): data is ProviderApiResponse {
if (typeof data \!== 'object' || data === null) return false;
// Check it has expected structure (at least one known field)
const obj = data as Record<string, unknown>;
const hasValidStructure =
'content' in obj || // Anthropic
'choices' in obj || // OpenAI Chat
'output' in obj || // OpenAI Responses
'candidates' in obj; // Gemini
if (\!hasValidStructure) {
logger.warn('Provider response missing expected fields', {
keys: Object.keys(obj).slice(0, 10)
});
return false;
}
return true;
}
// Use in parsing
const parsed = safeParseJSON(currentData);
if (\!validateProviderResponse(parsed)) {
logger.warn('Invalid provider response structure');
continue;
}
chunks.push(parsed);References:
- CWE-502: Deserialization of Untrusted Data
- CWE-1336: Improper Neutralization of Special Elements Used in a Template Engine
- OWASP: Insecure Deserialization
📋 OWASP Top 10 Coverage
- ✅ A01: Injection - Clean (no SQL/Command/XXE vulnerabilities)
- ✅ A02: Broken Authentication - Clean (admin-only access control enforced)
- ✅ A03: Sensitive Data Exposure - Clean (API keys properly redacted in logs)
- ❌ A04: XML External Entities - N/A (no XML parsing)
- ✅ A05: Broken Access Control - Clean (proper admin role checks)
- ✅ A06: Security Misconfiguration - Clean
- ✅ A07: XSS - Clean (React auto-escaping, no dangerouslySetInnerHTML)
- 🔴 A08: Insecure Deserialization - 1 HIGH severity finding (insufficient validation)
- ❌ A09: Known Vulnerabilities - N/A (code changes only)
- ✅ A10: Logging & Monitoring - Clean (proper logging with sensitive data redaction)
- 🔴 Additional: Uncontrolled Resource Consumption (DoS) - 1 CRITICAL finding
🛡️ Security Posture
Assessment: CRITICAL CONCERNS - DO NOT MERGE
This PR introduces serious security vulnerabilities that could lead to:
- Complete service outage via memory exhaustion (DoS)
- Data integrity issues via unvalidated deserialization
Recommendation:
- ❌ BLOCK MERGE until both vulnerabilities are fixed
- Implement resource limits (MAX_CHUNKS, MAX_BUFFER_SIZE, MAX_ITERATIONS)
- Add runtime validation for parsed JSON data (Zod schemas or manual validation)
- Add integration tests with malicious payloads to verify fixes
Positive Security Controls Observed:
- ✅ Admin-only access control properly enforced
- ✅ SSRF protection via URL validation (blocks localhost, internal IPs, dangerous ports)
- ✅ API key redaction in logs
- ✅ Proper error handling with try-catch blocks
- ✅ Timeout protection (10 second timeout)
🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage
Scanned commit: d1bee4c
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
Error handling and edge case issues
| format: "sse", | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
🟠 High: Silent Chunk Parsing Failures
Why this is a problem: In both parseSSEText and parseStreamResponse, chunk parsing errors are silently ignored:
try {
const parsed = JSON.parse(currentData) as ProviderApiResponse;
chunks.push(parsed);
} catch {
// 忽略解析失败的 chunk
}This leads to:
- Data loss: Valid chunks after the malformed one are silently dropped
- No visibility: Developers cannot debug why stream parsing fails
- Incorrect results: Partial responses appear as "successful" even when most chunks failed
Suggested fix:
try {
const parsed = JSON.parse(currentData) as ProviderApiResponse;
chunks.push(parsed);
} catch (parseError) {
logger.warn("Failed to parse SSE chunk", {
chunkData: currentData.slice(0, 100),
error: parseError instanceof Error ? parseError.message : String(parseError),
});
// Continue parsing other chunks
}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
Performance and code quality issues
| // 保留最后一行（可能不完整） | ||
| buffer = lines.pop() || ""; | ||
|
|
||
| for (const line of lines) { |
There was a problem hiding this comment.
🟡 Medium: Inefficient Text Merging with String Concatenation
Why this is a problem: In mergeStreamChunks, text is merged using repeated string concatenation (mergedText += ...) across potentially hundreds of chunks. This creates intermediate string objects, causing O(n²) time complexity.
Performance impact: For large responses (>10KB), this can cause noticeable slowdowns in the UI.
Suggested fix:
// Use array join for O(n) performance
const textChunks: string[] = [];
for (const chunk of chunks) {
if ("content" in chunk && Array.isArray(chunk.content)) {
for (const content of chunk.content) {
if (content.type === "text" && "text" in content) {
textChunks.push(content.text);
}
}
}
// ... other cases
}
const mergedText = textChunks.join("");
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
Medium severity - Maintainability
| if ("usageMetadata" in chunk && chunk.usageMetadata) { | ||
| (base as GeminiResponse).usageMetadata = chunk.usageMetadata; | ||
| break; | ||
| } |
There was a problem hiding this comment.
🟡 Medium: Hardcoded Test Prompt Position
Why this is a problem: The test prompt structure uses hardcoded array position [0] to access the first (and only) input item:
input: [
{
role: "user",
content: [
{
type: "input_text",
text: API_TEST_CONFIG.TEST_PROMPT,
},
],
},
],While correct for now, this pattern makes the code harder to extend. If someone wants to add system instructions or multi-turn conversations for testing, they would need to update both the array structure AND all code that assumes output[0] exists.
Suggested fix: Add a comment explaining this is a minimal test payload:
// Minimal test payload - only required fields
// Note: Production usage may include additional fields like instructions, etc.
input: [
{
role: "user",
content: [{ type: "input_text", text: API_TEST_CONFIG.TEST_PROMPT }],
},
],
| @@ -33,7 +33,7 @@ import { GeminiAuth } from "@/app/v1/_lib/gemini/auth"; | |||
| // API 测试配置常量 | |||
| const API_TEST_CONFIG = { | |||
There was a problem hiding this comment.
🟢 Low: Magic Number for Preview Length
Why this is a problem: The preview length was increased from 100 to 500 characters without justification in code:
MAX_RESPONSE_PREVIEW_LENGTH: 500, // 响应内容预览最大长度（增加到 500 字符以显示更多内容）While this improves UX, the value is arbitrary. For streaming responses with many chunks, this could still truncate important information.
Recommendation: Consider making this configurable or context-aware:
const API_TEST_CONFIG = {
TIMEOUT_MS: 10000,
MAX_RESPONSE_PREVIEW_LENGTH: 500,
// For streaming responses, show more content since we've already
// paid the cost of parsing the full stream
MAX_STREAMING_PREVIEW_LENGTH: 1000,
TEST_MAX_TOKENS: 100,
TEST_PROMPT: "Hello",
} as const;
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR adds streaming response support for API testing, which is a valuable feature. However, the implementation has several critical and high-severity issues that must be addressed before merging.
🔍 Issues Found
- Critical (🔴): 1 issue
- High (🟠): 3 issues
- Medium (🟡): 2 issues
- Low (🟢): 1 issue
🎯 Priority Actions
-
Fix stream reader resource leak (Critical): Add
await reader.cancel()in error path to prevent memory leaks and socket exhaustion -
Fix type safety in stream merging (High): Add proper type guards before casting response objects to avoid creating invalid responses
-
Improve SSE detection logic (High): Replace string matching with regex to prevent false positives/negatives
-
Add logging for chunk parse failures (High): Silent failures make debugging impossible - add warn-level logging
-
Optimize text concatenation (Medium): Use array join instead of repeated string concatenation for O(n) performance
💡 General Observations
Positive aspects:
- Good separation of concerns with dedicated parsing functions
- Comprehensive support for multiple API formats (Anthropic, OpenAI, Gemini)
- Proper error handling at the top level
Areas needing improvement:
- Resource management: Streams must be explicitly cancelled on errors
- Type safety: Heavy use of type assertions without runtime validation
- Observability: Silent error swallowing makes debugging difficult
- Performance: String concatenation in loops creates unnecessary overhead
Testing recommendations:
- Test with malformed SSE responses (invalid JSON chunks)
- Test with very large responses (>100 chunks) to verify performance
- Test error scenarios (connection drops mid-stream)
- Verify no resource leaks with multiple consecutive tests
🤖 Automated review by Claude AI - focused on identifying issues for improvement
Silentely
changed the title
应用 Prettier 格式化规则到 providers.ts，修复 Code Quality Check 失败问题。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🔒 Security Scan Results✅ No security vulnerabilities detected This PR has been scanned against OWASP Top 10, CWE Top 25, and common security anti-patterns. No security issues were identified in the code changes. 🛡️ Security Improvements FoundThis PR actually improves security posture by adding DoS protections:
📋 OWASP Top 10 Coverage
🔍 Additional Security Checks
📊 Code Quality ObservationsPositive Security Practices:
No Security Concerns Found 🎯 Recommendation✅ APPROVED from security perspective This PR enhances security by adding DoS protections for streaming responses. No vulnerabilities were identified. 🤖 Automated security scan by Claude AI - OWASP Top 10 & CWE coverage |
| * 解析流式响应（从 Response 对象读取） | ||
| */ | ||
| async function parseStreamResponse(response: Response): Promise<StreamParseResult> { | ||
| if (!response.body) { |
There was a problem hiding this comment.
🔴 Critical: 资源耗尽攻击风险 - parseStreamResponse 缺少资源限制
Why this is a problem:
parseStreamResponse() 函数在解析流式响应时缺少关键的资源限制检查，可能导致：
- 无限循环：
while (true)循环没有迭代次数限制 - 内存耗尽：
buffer变量可以无限增长，没有大小限制 - DoS 攻击：恶意供应商可以发送无限流数据，导致服务器崩溃
虽然代码定义了 MAX_STREAM_CHUNKS、MAX_STREAM_BUFFER_SIZE 和 MAX_STREAM_ITERATIONS 常量，但在 parseStreamResponse() 函数中完全没有使用这些限制。相比之下，parseSSEText() 函数正确实现了所有这些检查。
Suggested fix:
async function parseStreamResponse(response: Response): Promise<StreamParseResult> {
if (!response.body) {
throw new Error("响应体为空");
}
const reader = response.body.getReader();
const decoder = new TextDecoder();
const chunks: ProviderApiResponse[] = [];
let buffer = "";
let currentData = "";
let iterations = 0; // 添加迭代计数器
try {
while (true) {
// 防止无限循环（防止 DoS）
if (iterations++ > API_TEST_CONFIG.MAX_STREAM_ITERATIONS) {
logger.warn("Stream 解析达到最大迭代次数", {
maxIterations: API_TEST_CONFIG.MAX_STREAM_ITERATIONS,
});
break;
}
const { done, value } = await reader.read();
if (done) break;
buffer += decoder.decode(value, { stream: true });
// 防止缓冲区过大（防止内存耗尽）
if (buffer.length > API_TEST_CONFIG.MAX_STREAM_BUFFER_SIZE) {
throw new Error(`Stream 缓冲区超过最大大小 (${API_TEST_CONFIG.MAX_STREAM_BUFFER_SIZE} 字节)`);
}
const lines = buffer.split("\n");
buffer = lines.pop() || "";
for (const line of lines) {
const trimmed = line.trim();
if (trimmed.startsWith("data:")) {
const dataContent = trimmed.slice(5).trim();
if (dataContent === "[DONE]") {
continue;
}
if (dataContent) {
currentData = dataContent;
}
} else if (trimmed === "" && currentData) {
// 防止过多数据块（防止 DoS）
if (chunks.length >= API_TEST_CONFIG.MAX_STREAM_CHUNKS) {
logger.warn("Stream 解析达到最大数据块限制", {
maxChunks: API_TEST_CONFIG.MAX_STREAM_CHUNKS,
});
break;
}
try {
const parsed = JSON.parse(currentData) as ProviderApiResponse;
chunks.push(parsed);
currentData = "";
} catch {
// 忽略解析失败的 chunk
}
}
}
}
// ... rest of the function
} finally {
reader.releaseLock();
}
if (chunks.length === 0) {
throw new Error("未能从流式响应中解析出有效数据");
}
const mergedResponse = mergeStreamChunks(chunks);
return {
data: mergedResponse,
chunksReceived: chunks.length,
format: "sse",
};
}
|
|
||
| for (const chunk of chunks) { | ||
| // Anthropic Messages API | ||
| if ("content" in chunk && Array.isArray(chunk.content)) { |
There was a problem hiding this comment.
🟠 High: mergeStreamChunks 中的类型断言逻辑错误
Why this is a problem:
在 mergeStreamChunks() 函数中，OpenAI Chat Completions API 的流式响应合并逻辑存在错误。代码检查 delta 字段后，仍然尝试访问不存在的 message.content：
// 流式响应使用 delta
if (firstChoice && "delta" in firstChoice) {
const delta = firstChoice.delta as { content?: string };
if (delta.content) {
mergedText += delta.content;
}
}
// 非流式响应使用 message
else if (firstChoice?.message?.content) {
mergedText += firstChoice.message.content;
}问题：在流式响应中，firstChoice 有 delta 字段但没有 message 字段。如果第一个 chunk 有 delta 但后续 chunk 有 message，会导致数据丢失。
更严重的是，在最后写回响应对象时，代码假设所有流式响应都会转换为 message 格式：
// OpenAI Chat Completions API
if ("choices" in base && Array.isArray(base.choices)) {
base.choices = [
{
...base.choices[0],
message: { role: "assistant", content: mergedText }, // ❌ 丢失 delta 字段
finish_reason: "stop",
},
];
}这会导致原始流式响应结构被破坏。
Suggested fix:
// 在循环中收集文本
for (const chunk of chunks) {
if ("choices" in chunk && Array.isArray(chunk.choices)) {
const firstChoice = chunk.choices[0];
if (firstChoice) {
// 流式响应：delta 字段
if ("delta" in firstChoice && firstChoice.delta) {
const delta = firstChoice.delta as { content?: string };
if (delta.content) {
mergedText += delta.content;
}
}
// 非流式响应：message 字段
else if ("message" in firstChoice && firstChoice.message?.content) {
mergedText += firstChoice.message.content;
}
}
}
}
// 写回响应对象时，保留原始结构
if (mergedText) {
if ("choices" in base && Array.isArray(base.choices)) {
const firstChoice = base.choices[0];
if (firstChoice && "delta" in firstChoice) {
// 流式响应：使用 delta
base.choices = [
{
...firstChoice,
delta: { role: "assistant", content: mergedText },
finish_reason: "stop",
},
];
} else {
// 非流式响应：使用 message
base.choices = [
{
...base.choices[0],
message: { role: "assistant", content: mergedText },
finish_reason: "stop",
},
];
}
}
}
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
本 PR 为 Codex API 测试功能新增了流式响应支持，修复了参数兼容性问题。整体方向正确，但在流式响应解析的安全性和正确性方面存在需要立即修复的关键问题。
🔍 Issues Found
- Critical (🔴): 1 issue
- High (🟠): 1 issue
- Medium (🟡): 0 issues
- Low (🟢): 0 issues
🎯 Priority Actions
- [Critical] 在
parseStreamResponse()中添加资源限制检查（迭代次数、缓冲区大小、数据块数量），防止 DoS 攻击和内存耗尽 - [High] 修复
mergeStreamChunks()中 OpenAI Chat Completions API 的流式响应处理逻辑，避免 delta/message 字段混淆导致的数据丢失
💡 General Observations
优点:
- 新增的 DoS 防护常量（
MAX_STREAM_CHUNKS、MAX_STREAM_BUFFER_SIZE、MAX_STREAM_ITERATIONS）设计合理 parseSSEText()函数正确实现了所有资源限制检查，可作为参考- 移除
max_output_tokens参数提高了兼容性 - UI 展示流式响应信息的设计清晰实用
需要改进:
parseStreamResponse()虽然定义了防护常量，但未实际使用，存在严重安全风险- 流式响应的数据结构转换逻辑需要更加严谨，确保不同 API 格式的兼容性
🤖 Automated review by Claude AI - focused on identifying issues for improvement
根据 PR ding113#185 的代码审查建议，修复以下问题： **Critical 修复**: - 🔴 修复 Stream reader 资源泄漏 - 在错误路径中添加 `await reader.cancel()` 防止内存泄漏 - 确保在异常情况下正确释放资源 **High 修复**: - 🟠 添加日志记录 - 为 chunk 解析失败添加 warn 级别日志 - 记录跳过的 chunk 数量和错误详情 - 添加流式响应解析完成的 info 日志 - 🟠 改进 SSE 检测逻辑 - 使用正则表达式替代字符串匹配 - 提高检测的健壮性，避免误判 - 🟠 增强类型安全 - 在类型转换前添加运行时类型守卫 - 为空数组情况添加默认值处理 - 确保不会创建无效的响应对象 **改进点**: - 统一错误处理模式 - 提高代码可维护性 - 增强调试能力 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
|
|
There was a problem hiding this comment.
🟠 High: Missing chunk count limit in parseStreamResponse can cause memory exhaustion
Why this is a problem: The parseSSEText function checks chunks.length >= API_TEST_CONFIG.MAX_STREAM_CHUNKS before adding chunks, but parseStreamResponse has no such check. A malicious stream could send unlimited small chunks, exhausting memory.
Suggested fix:
} else if (trimmed === "" && currentData) {
// 防止过多数据块（防止 DoS）
if (chunks.length >= API_TEST_CONFIG.MAX_STREAM_CHUNKS) {
logger.warn("流式响应解析达到最大数据块限制", {
maxChunks: API_TEST_CONFIG.MAX_STREAM_CHUNKS,
skipped: skippedChunks,
});
break;
}
// 空行表示一个完整的 SSE 事件结束
try {
const parsed = JSON.parse(currentData) as ProviderApiResponse;
chunks.push(parsed);
currentData = "";
} catch (parseError) {
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🟡 Medium: Using Date.now() for message ID generation may cause collisions
Why this is a problem: Date.now() returns milliseconds, and multiple chunks processed in the same millisecond could generate the same ID. This could cause issues if these IDs are used for deduplication or tracking downstream.
Suggested fix:
if (
"id" in base &&
typeof base.id === "string" &&
"type" in base &&
base.type === "response"
) {
(base as OpenAIResponsesResponse).output = [
{
type: "message",
id: firstOutput?.id || `msg_${crypto.randomUUID()}`,
status: firstOutput?.status || "completed",
role: "assistant",
content: [{ type: "output_text", text: mergedText }],
},
];
}
📊 PR Size AnalysisThis PR is size/L with 595 lines changed across 3 files. While the PR is well-structured and addresses related concerns, large PRs are harder to review and more likely to introduce bugs. The current implementation combines two distinct changes that could be reviewed independently. 🔀 Suggested Split:Based on the changes, this PR could be split into 2 sequential PRs: PR 1: Fix OpenAI Responses API Parameter CompatibilityFiles:
Changes:
Estimated size: ~50 lines (size/XS) PR 2: Add Streaming Response Testing SupportFiles:
Changes:
Estimated size: ~540 lines (size/L) Why Split?
Alternative: Keep as Single PRIf you prefer to keep this as a single PR (which is also reasonable given the changes are logically related to API testing), I recommend:
🤖 Automated analysis by Claude AI |
ding113
left a comment
ding113
left a comment
There was a problem hiding this comment.
📋 Code Review Summary
This PR adds streaming response support and fixes Codex API compatibility. The implementation is solid overall, but there are some resource protection inconsistencies between the two parsing functions that should be addressed before merge.
🔍 Issues Found
- Critical (🔴): 0 issues
- High (🟠): 2 issues
- Medium (🟡): 3 issues
- Low (🟢): 0 issues
🎯 Priority Actions
-
Add buffer size check in parseStreamResponse - The streaming parser lacks the DoS protection present in parseSSEText. A malicious stream could cause memory exhaustion.
-
Add chunk count limit in parseStreamResponse - Missing
MAX_STREAM_CHUNKScheck allows unlimited chunk accumulation. -
Fix SSE detection regex - Current pattern may have false positives with JSON starting with "data:".
-
Preserve original finish_reason - Currently hardcoded to "stop", but should preserve the actual value from the last chunk.
-
Use crypto.randomUUID() for message IDs - Date.now() can cause ID collisions.
💡 General Observations
The DoS protections in parseSSEText (buffer size limit, chunk count limit, iteration limit) are comprehensive, but parseStreamResponse is missing equivalent protections. Consider extracting these limits into a shared validation approach to ensure consistency.
🤖 Automated review by Claude AI - focused on identifying issues for improvement
2 participants
概述 (Summary)
本 PR 修复了 Codex API 连接测试的兼容性问题，并新增流式响应测试支持，显著提升了供应商测试的可靠性和功能完整性。
主要改进:
问题描述 (Problem)
1. 参数兼容性问题
在供应商管理页面测试 OpenAI Responses API 连接时，某些 Codex 中转服务返回 400 错误：
{ "detail": "Unsupported parameter: max_output_tokens" }受影响的供应商:
max_output_tokens参数的中转服务2. 流式响应测试缺失
原测试系统仅支持非流式响应测试，无法验证：
解决方案 (Solution)
1. 移除可选参数，提高兼容性
修复前:
修复后:
关键改进:
max_output_tokens参数，避免兼容性问题input格式为符合规范的数组结构2. 新增流式响应测试支持
自动检测响应类型
流式数据解析
SSE 格式解析:
文本格式兜底:
智能数据块合并
支持多种 API 格式的流式数据合并：
content[].textchoices[].delta.contentoutput[].content[].textcandidates[].content.parts[].text3. UI 增强
测试结果详情
新增显示项:
Toast 通知
增强的通知内容:
技术细节 (Technical Details)
OpenAI Responses API 规范
必需参数:
可选参数:
max_output_tokens- 最大输出 token 数（某些中转服务不支持）temperature- 温度参数stream- 是否流式输出instructions- 系统指令流式响应格式
SSE (Server-Sent Events)
特征:
Content-Type: text/event-streamdata:开头[DONE]标记结束NDJSON (Newline-Delimited JSON)
特征:
Content-Type: application/x-ndjson错误处理策略
变更文件 (Changes)
核心逻辑 (
src/actions/providers.ts)新增函数:
parseStreamResponse()- 从 Response 对象解析流式数据（323 行）parseSSEText()- 从文本解析 SSE 格式（70 行）mergeStreamChunks()- 智能合并数据块（120 行）修改配置:
MAX_RESPONSE_PREVIEW_LENGTH: 100 → 500testProviderOpenAIResponses()参数修正UI 组件 (
src/app/[locale]/settings/providers/_components/forms/api-test-button.tsx)新增显示:
类型扩展:
国际化 (
messages/zh-CN/settings.json)文案优化:
测试验证 (Testing)
1. 参数兼容性测试
IKunCode/RightCode (Codex):
400 "Unsupported parameter: max_output_tokens"2. 流式响应测试
测试场景:
测试步骤:
3. 数据块合并测试
验证点:
相关资源 (References)
docs/fix-openai-responses-api-input-format.mdChecklist