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

CapturingInputStream discards -1 from delegate read #23381

Merged
merged 1 commit into from
Feb 2, 2022
Merged

CapturingInputStream discards -1 from delegate read #23381

merged 1 commit into from
Feb 2, 2022

Conversation

bengunter
Copy link

@bengunter bengunter commented Feb 2, 2022 •
edited by gastaldi
Loading

Fixes #23351

@bengunter bengunter mentioned this pull request Feb 2, 2022
Closed
geoand
geoand reviewed Feb 2, 2022
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks, but I think we will need an integration test for this

gastaldi
gastaldi previously requested changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

Marking as requesting changes until an integration test is added

@bengunter
Copy link
Author

I'm going to need some specifics as to what you're looking for. It's a big project, and I'm not familiar with any of it.

@geoand
Copy link
Contributor

geoand commented Feb 2, 2022

Something like https://github.com/quarkusio/quarkus/blob/main/independent-projects/resteasy-reactive/server/vertx/src/test/java/org/jboss/resteasy/reactive/server/vertx/test/multipart/InvalidEncodingTest.java

@gastaldi
Copy link
Contributor

gastaldi commented Feb 2, 2022

The snippet you added in #23351 (comment) will be useful too

@gastaldi gastaldi dismissed their stale review February 2, 2022 17:40

Leaving to @geoand to review it

@bengunter
Copy link
Author

Integration test has been added.

@gastaldi gastaldi requested a review from geoand February 2, 2022 19:57
@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 2, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Feb 2, 2022

Failing Jobs - Building 59a0988

Status Name Step Failures Logs Raw logs
✖ Native Tests - gRPC Download Maven Repo ⚠️ Check → Logs Raw logs

@gastaldi
Copy link
Contributor

gastaldi commented Feb 2, 2022

Error is unrelated. Merging.

Good job @bengunter !

@gastaldi gastaldi merged commit d236111 into quarkusio:main Feb 2, 2022
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Feb 2, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Feb 2, 2022
@bengunter bengunter deleted the capturing-input-stream-corruption branch February 3, 2022 03:48
@gsmet gsmet modified the milestones: 2.8 - main, 2.7.1.Final Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand approved these changes

@gastaldi gastaldi gastaldi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.7.1.Final
Development

Successfully merging this pull request may close these issues.

Data corruption when mixing @FormParam and MultivaluedMap
5 participants
@bengunter @geoand @gastaldi @gsmet @de-bgunter