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

Allow for either getOutputStream or getWriter to be used before sendRedirect is called #119

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reversefold
Copy link

@reversefold reversefold commented Jun 19, 2017

The current code always calls getOutputStream() in sendRedirect, which makes the response fail if getWriter() is used in the calling code before sendRedirect() is called. This PR closes either the writer or output stream depending on which was used.


This change is Reviewable

@reversefold reversefold force-pushed the close-correct-output-stream branch from d2ec085 to 9dca307 Compare June 19, 2017 18:41
@reversefold reversefold force-pushed the close-correct-output-stream branch from 9dca307 to bccccae Compare June 19, 2017 18:44
@jglick
Copy link
Member

jglick commented Aug 3, 2017

Sounds right but can this be proven in a test? Is there a particular filed bug this is addressing?

@reversefold
Copy link
Author

This is related to a PR I wrote for the jenkins-artifactory-plugin: https://github.com/JFrogDev/jenkins-artifactory-plugin/pull/54/files Originally I was using getWriter() but I got a confusing error message about getWriter() already having been called. It took me a while to track down exactly what was going wrong and switch to getOutputStream(). This PR makes it so that either can be used before a redirect.

@jglick
Copy link
Member

jglick commented Sep 6, 2017

OK. Putting this on hold until I have a moment to figure out if this is amenable to unit testing.

@jglick jglick marked this pull request as draft June 15, 2020 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants