Repository navigation
Copy ServletHolder class/instance properly during startWebapp - #6214
Merged
Merged
Conversation
…tor. Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
- using status code 555 for testcase eliminating 503 false success from different code path than what we are attempting to fix Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
- ServletHolder needs a mapping to allow RejectUncompiledJspServlet to respond to requests for JSPs that haven't been precompiled. - Precompiled test class is now properly placed into a generated webapp base directory without contaminating the webapp classloader with other src/test/java classes Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
gregw
previously requested changes
Apr 27, 2021
gregw
left a comment
gregw
left a comment
Contributor
There was a problem hiding this comment.
@janbartel can you review this and then let's chat. I think we need to think very carefully about what it means to have 2 holders sharing the same instance.
janbartel
requested changes
Apr 27, 2021
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
janbartel
requested changes
Apr 28, 2021
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
janbartel
requested changes
Apr 29, 2021
janbartel
left a comment
janbartel
left a comment
Contributor
There was a problem hiding this comment.
Both of the remaining test servlet classes could be inner classes too.
| @@ -0,0 +1,37 @@ | |||
| // | |||
Contributor
There was a problem hiding this comment.
Make this an inner class.
| @@ -0,0 +1,37 @@ | |||
| // | |||
Contributor
There was a problem hiding this comment.
Make this one an inner class of ForcedServletTest too.
use inner classes Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw
pushed a commit
that referenced
this pull request
May 16, 2021
ServletHolder.copyClassServlet() method added to correctly copy held class. Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com> Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw
added a commit
that referenced
this pull request
May 16, 2021
Copy ServletHolder class/instance properly during startWebapp (#6214) ServletHolder.copyClassServlet() method added to correctly copy held class. Cherry picked from 9.4 Signed-off-by: Greg Wilkins <gregw@webtide.com> Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#6280