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

Broken exception handling in org.glassfish.jersey.servlet.WebComponent#initContainerRequest since 2.44 (also in 3.1.9) #5780

Open
mheinzerling opened this issue Oct 25, 2024 · 2 comments

Comments

@mheinzerling
Copy link

mheinzerling commented Oct 25, 2024

@jansupol , with d8f3e0f#diff-937cc5e5415a0412aa6850cc416fffb46747e443997e2d50b69162ba3af82b5c (#5669) you implemented an exception handling, that is not working as intended. This was a fix for #4867 . Let me first walk you through the issue.

You updated org.glassfish.jersey.servlet.WebComponent#initContainerRequest from (2.43):

requestContext.setEntityStream(servletRequest.getInputStream());

to (2.44):

try {
    requestContext.setEntityStream(new InputStreamWrapper() {
        @Override
        protected InputStream getWrapped() {
            try {
                return servletRequest.getInputStream();
            } catch (IOException e) {
                throw new UncheckedIOException(e);
            }
        }
    });
} catch (UncheckedIOException e) {
    throw e.getCause();
}

In case of an IOException on calling getInputStream initContainerRequest failed right away with the same IOException in 2.43.

After this change the getInputStream is not called at all in initContainerRequest and everything is fine for now.
Nevertheless, at the first call on the EntityStream e.g.: requestContext.getEntityStream().available() the call will fail with your UncheckedIOException. But at this point in time we long left the try-catch-block and the UncheckedIOException is being propagated through the system.

Please check mheinzerling@1473d8d for a simple test demonstrating the old behavior.

And mheinzerling@7a677ee is showing the new behavior.

In both cases I just extracted a static method to reduce the test setup and the mock*Request methods are also identical.

As mentioned above, I don't know what you tried to achieve with this change, so I can't recommend a fix. But at least in our system, I didn't notice any issue in just rolling back this snippet.

The same issue exists in 3.1.9.

Related to #5739

Please don't hesitate to send me a message if there are further questions. Thanks.

@mheinzerling mheinzerling changed the title Broken exception handing in org.glassfish.jersey.servlet.WebComponent#initContainerRequest since 2.44 Broken exception handling in org.glassfish.jersey.servlet.WebComponent#initContainerRequest since 2.44 (also in 3.1.9) Oct 30, 2024
@mheinzerling
Copy link
Author

Just updated the ticket. This is also an issue in 3.1.9 .

@RameshVE123
Copy link

I've observed the following issue with 3.1.8: (Mentioning it here as this change could be the reason for the failure)

  • Input request body logging throws an exception due to an error with the setting of the EntityStream.

requestContext.setEntityStream(new InputStreamWrapper() {
}

Please let me know if you need any additional information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants