Skip to content

Conversation

@pskiran1
Copy link
Member

@pskiran1 pskiran1 commented Oct 1, 2025

What does the PR do?

This change addresses an issue where an excessively large HTTP JSON payload could cause an unhandled std::length_error exception and terminate the server unexpectedly.
A validation check is now performed in the EVBufferToJson function. It compares the incoming JSON data size against the configured --http-max-input-size limit. Requests that exceed this limit are now gracefully rejected.

  • A new test case has been added to test the failing scenario.
  • Existing test cases have been updated, ensuring that the entire request JSON is within the --http-max-input-size limit.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

Where should the reviewer start?

Test plan:

  • CI Pipeline ID: 36107617

Caveats:

Background

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

@pskiran1 pskiran1 added the PR: fix A bug fix label Oct 1, 2025
@pskiran1 pskiran1 marked this pull request as draft October 1, 2025 15:01
@pskiran1 pskiran1 requested a review from Copilot October 1, 2025 16:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a server crash issue by adding input size validation for large HTTP JSON requests. The change prevents std::length_error exceptions by checking the JSON payload size against the configured --http-max-input-size limit before processing.

  • Added validation check in EVBufferToJson function to compare incoming JSON size against the maximum allowed size
  • Updated test cases to verify the new error handling behavior
  • Added new test case specifically for large string payloads in JSON requests

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/http_server.cc Added size validation check with descriptive error message
qa/L0_http/test.sh Added test execution for large string in JSON scenario
qa/L0_http/http_test.py Updated error message assertions to match new validation
qa/L0_http/http_input_size_limit_test.py Added new test case and debug logging for large string payloads
qa/L0_cuda_shared_memory/cuda_shared_memory_test.py Updated error message assertions to match new validation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

pskiran1 and others added 5 commits October 1, 2025 22:00
…'BaseException'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@pskiran1 pskiran1 requested a review from Copilot October 6, 2025 05:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@pskiran1 pskiran1 marked this pull request as ready for review October 6, 2025 06:16
@whoisj
Copy link
Contributor

whoisj commented Oct 7, 2025

LGTM and Matt already commented on everything that I noticed 👍

Copy link
Contributor

@yinggeh yinggeh left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@mattwittwer mattwittwer left a comment

Choose a reason for hiding this comment

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

LGTM

@pskiran1 pskiran1 merged commit be7d4b1 into main Oct 10, 2025
3 checks passed
@pskiran1 pskiran1 deleted the spolisetty/tri-50-psirt-overflow-vulnerability branch October 10, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fix A bug fix

Development

Successfully merging this pull request may close these issues.

5 participants