Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set the max buffersize for jsoniter #2459

Open
wants to merge 2 commits into
base: series/2.x
Choose a base branch
from
Open

Set the max buffersize for jsoniter #2459

wants to merge 2 commits into from

Conversation

kaaveland
Copy link

This is a first take on #2458

Let me know if there's something missing, or if I should try another way to do this.

kyri-petrou
kyri-petrou reviewed Nov 8, 2024
View reviewed changes
Copy link
Collaborator

@kyri-petrou kyri-petrou left a comment

Choose a reason for hiding this comment

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

Could we also add a test case in TapirAdapterSpec that would fail using the pre-PR code? That way we can ensure that the implicits are all properly used across all adapters.

adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala Outdated Show resolved Hide resolved
adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala Outdated Show resolved Hide resolved
adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala Outdated Show resolved Hide resolved
@kaaveland
Copy link
Author

kaaveland commented Nov 8, 2024 •
edited
Loading

I added a test case, but it currently also fails on this branch -- we're getting 413 Request Entity Too Large from PlayAdapterSpec and QuickAdapterSpec. I'm working on figuring out how to make these two servers accept larger payloads. I did verify that it reproduces the error on series/2.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyri-petrou kyri-petrou kyri-petrou left review comments

@ghostdogpr ghostdogpr ghostdogpr left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kaaveland @ghostdogpr @kyri-petrou