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

MockHttpServletRequest should not use a shared reader when no content is available #32820

Closed
superbob opened this issue May 14, 2024 · 1 comment
Assignees
Labels
in: test Issues in the test module status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@superbob
Copy link

Affects: At least from 5.3.31 to latest (2024-05-14)


If you create two different instances of MockHttpServletRequest without specifying content and try to use the reader of each of them. The second one will raise an exception as all these instances share the same underlying reader (EMPTY_BUFFERED_READER -> https://github.com/spring-projects/spring-framework/blob/main/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletRequest.java#L741).

Here is a kotlin example of it:

class MockHttpServletRequestReaderTest {
    @Test
    fun `call once succeeds`() {
        MockHttpServletRequest().reader.use { it.readText() }
    }

    @Test
    fun `call twice fails`() {
        MockHttpServletRequest().reader.use { it.readText() }
        MockHttpServletRequest().reader.use { it.readText() }
    }
}

If you run the first test alone in its own JVM, it'll work.

If you run the second test alone, it'll fail on the second MockHttpServletRequest.reader use.

If you repeat the first test at least twice in the same JVM, only the first execution will run fine, all the other ones will fail.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 14, 2024
@snicoll snicoll changed the title Cannot use reader of two different empty MockHttpServletRequest instances. MockHttpServletRequest should not use a shared reader when no content is available May 20, 2024
@snicoll snicoll added in: test Issues in the test module type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 20, 2024
@snicoll snicoll added this to the 6.1.8 milestone May 20, 2024
@snicoll snicoll self-assigned this May 20, 2024
@snicoll
Copy link
Member

snicoll commented May 20, 2024

Thanks for the report and it's interesting to see this was never reported before. I guess if you don't have content, you don't exhaust the body in the test (in particular closing the reader, which is what this method does). Also getInputStream does not have this problem as it creates a new stream, even for an empty body.

@snicoll snicoll added for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels May 20, 2024
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels May 20, 2024
snicoll added a commit that referenced this issue May 20, 2024
This commit avoids several instances of MockHttpServletRequest to
have a common reader for empty content as closing it will have an
unwanted side effect on the others.

Closes gh-32820
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants