Bug 55110

Summary: Wasted work in "TestNonLoginAndBasicAuthenticator.doTestBasic"
Product: Tomcat 7 Reporter: Adrian Nistor <nistor1>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal CC: nistor1
Priority: P2 Keywords: PatchAvailable
Version: 7.0.41   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Attachments: patch
patch2

Description Adrian Nistor 2013-06-17 18:37:18 UTC
Created attachment 30450 [details]
patch

The problem appears in version 7.0.41 and in revision 1493861.  I
attached a one-line patch (patch.diff) that fixes it.

In method "TestNonLoginAndBasicAuthenticator.doTestBasic", the loop
over "authHeaders" should break immediately after "methodFound" is set
to "true".  All the iterations after "methodFound" is set to "true" do
not perform any useful work, at best they just set "methodFound" again
to "true".

Method "TestWsWebSocketContainer.testSessionExpiryContainer" has a
similar problem (the loop over "setA" should break immediately after
"isOpen" is set to true).  I attached another one-line patch
(patch2.diff) for this problem.

Method "startInternal" in class "StandardHost" has a similar loop
(over "valves"), and this loop breaks immediately after "found" is set
to "true", just like in the proposed patches.  Other methods (e.g.,
"MapperListener.findDefaultHost", "CollectVisitor.checkSeen",
"JspDocumentParser.processChars", "ParameterParser.isOneOf") also have
similar loops with similar breaks, just like in the proposed patches.
Comment 1 Adrian Nistor 2013-06-17 18:37:39 UTC
Created attachment 30451 [details]
patch2
Comment 2 Mark Thomas 2013-06-18 07:26:03 UTC
Thanks for the patches. Both were applied to trunk and thre first was back-ported to 7.0.x and will be included in 7.0.42 onwards.

Some tips for future contributions:
- when fixing a common problem like this a single patch for all instances is better than lots of little patches
- patches are best against trunk (svn will do most of the back-port work for us) and we'll back-port it to the reported version
- some code in trunk doesn't exist in 7.0.x & some code in 7.0.x doesn't exist in trunk. We can work around this but you need to be careful about the versions you are reporting things against