Bug 55110 - Wasted work in "TestNonLoginAndBasicAuthenticator.doTestBasic"
Wasted work in "TestNonLoginAndBasicAuthenticator.doTestBasic"
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Catalina
7.0.41
PC Linux
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
: PatchAvailable
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2013-06-17 18:37 UTC by Adrian Nistor
Modified: 2013-06-18 07:26 UTC (History)
1 user (show)



Attachments
patch (654 bytes, patch)
2013-06-17 18:37 UTC, Adrian Nistor
Details | Diff
patch2 (520 bytes, patch)
2013-06-17 18:37 UTC, Adrian Nistor
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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