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.

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

Spring MVC: Unexpected bytes added to the HTTP response for kotlin.Unit return type in controller methods #31648

Closed
estigma88 opened this issue Nov 21, 2023 · 6 comments
Assignees
Labels
theme: kotlin An issue related to Kotlin support type: regression A bug that is also a regression
Milestone

Comments

@estigma88
Copy link

Affects: Spring Boot 3.2.0-RC2, Spring Web 3.2.0-RC2, Kotlin 1.9.20

Issue

Using a Controller, with return type kotlin.Unit, which uses directly HttpServletResponse to write the response, adds unexpected bytes to the response. The following is a controller example where a file is read and written to the output directly:

    @GetMapping(path = ["public/file.txt"])
    fun getFile(
        httpServletResponse: HttpServletResponse
    ) {
        httpServletResponse.status = 200

        httpServletResponse.outputStream.write(Files.readAllBytes(Path.of("src/main/resources/file.txt")))
    }

The following shows the bytes expected vs current:

Current: [49, 50, 51, 52, 53, 54, 55, 56, 57, 123, 125]
Expected: [49, 50, 51, 52, 53, 54, 55, 56, 57]

123 and 125 bytes are { and }

How to reproduce the issue

This is a Spring Boot project with a test that reproduces the issue.

Debugging

My guess would be: there is a new converter which is taking kotlin.Unit and tries to serialize it, outputting { and }.

@quaff
Copy link
Contributor

quaff commented Nov 22, 2023

It should be fixed at framework not boot side, I've created #31647 to handle this.

@quaff
Copy link
Contributor

quaff commented Nov 22, 2023

@estigma88 You can add suspend modifier to the method as workaround, don't forget adding dependency org.jetbrains.kotlinx:kotlinx-coroutines-reactor

@estigma88
Copy link
Author

@quaff Thanks for the suggestion, but does that work with virtual threads? We are using virtual threads heavily in the project

@quaff
Copy link
Contributor

quaff commented Nov 22, 2023

@quaff Thanks for the suggestion, but does that work with virtual threads? We are using virtual threads heavily in the project

I'm not expert of kotlin, but I don't think it will conflict with virtual threads.

@snicoll snicoll transferred this issue from spring-projects/spring-boot Nov 22, 2023
@sdeleuze sdeleuze self-assigned this Nov 22, 2023
@sdeleuze sdeleuze added theme: kotlin An issue related to Kotlin support type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 22, 2023
@sdeleuze sdeleuze added this to the 6.1.1 milestone Nov 22, 2023
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Nov 22, 2023
This commit fix a regression introduced by spring-projectsgh-21139
via the usage of Kotlin reflection to invoke HTTP
handler methods. It ensures that kotlin.Unit is treated
as void by returning null.

It also polish CoroutinesUtils to have a consistent
handling with the regular case, and add related
tests to prevent future regressions.

Closes spring-projectsgh-31648
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Nov 22, 2023
This commit fixes a regression introduced by spring-projectsgh-21139
via the usage of Kotlin reflection to invoke HTTP
handler methods. It ensures that kotlin.Unit is treated
as void by returning null.

It also polish CoroutinesUtils to have a consistent
handling compared to the regular case, and add related
tests to prevent future regressions.

Closes spring-projectsgh-31648
@sdeleuze
Copy link
Contributor

Thanks for catching this regression, this should now be fixed.

@estigma88
Copy link
Author

I confirm it is working with Spring Boot 3.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: kotlin An issue related to Kotlin support type: regression A bug that is also a regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants