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

Useless processing in SoapSampler.setPostHeaders #3147

Closed
asfimport opened this issue Jun 29, 2013 · 2 comments
Closed

Useless processing in SoapSampler.setPostHeaders #3147

asfimport opened this issue Jun 29, 2013 · 2 comments

Comments

@asfimport
Copy link
Collaborator

Adrian Nistor (Bug 55161):
The problem appears in version 2.9 and in revision 1498029. I
attached a two-line patch (patch.diff) that fixes it.

In method "SoapSampler.setPostHeaders", the loop over the
"HeaderManager mngr" keeps overriding "length" with
"Integer.parseInt(hd.getValue())". Therefore, only the last written
value is visible out of the loop and all the other writes and
iterations are not necessary. The patch iterates from the end of
"HeaderManager mngr" and breaks the first time when "length" is set.

The above fix (in patch.diff) is certainly correct (it's easy to see
through code inspection), but I think we can have an even shorter
patch (one line, in patchShort.diff): just break as soon as "length"
is set, without reversion the loop order. patchShort.diff is correct
only if there can be only one "hd.getName()" equal to
"HTTPConstants.HEADER_CONTENT_LENGTH" (which I think it's the case),
or if it doesn't matter which attribute value "length" gets, as long
as the condition
HTTPConstants.HEADER_CONTENT_LENGTH.equalsIgnoreCase(hd.getName()) is
satisfied.

Created attachment patch.diff: patch

patch.diff
Index: src/protocol/http/org/apache/jmeter/protocol/http/sampler/SoapSampler.java
===================================================================
--- src/protocol/http/org/apache/jmeter/protocol/http/sampler/SoapSampler.java	(revision 1498029)
+++ src/protocol/http/org/apache/jmeter/protocol/http/sampler/SoapSampler.java	(working copy)
@@ -137,10 +137,11 @@
             // to use it.
             HeaderManager mngr = getHeaderManager();
             int headerSize = mngr.size();
-            for (int idx = 0; idx < headerSize; idx++) {
+            for (int idx = headerSize - 1; idx >= 0; idx--) {
                 Header hd = mngr.getHeader(idx);
                 if (HTTPConstants.HEADER_CONTENT_LENGTH.equalsIgnoreCase(hd.getName())) {// Use this to override file length
                     length = Integer.parseInt(hd.getValue());
+                    break;
                 }
                 // All the other headers are set up by HTTPSampler2.setupConnection()
             }

Severity: normal
OS: All

@asfimport
Copy link
Collaborator Author

Adrian Nistor (migrated from Bugzilla):
Created attachment patchShort.diff: patchShort

patchShort.diff
Index: src/protocol/http/org/apache/jmeter/protocol/http/sampler/SoapSampler.java
===================================================================
--- src/protocol/http/org/apache/jmeter/protocol/http/sampler/SoapSampler.java	(revision 1498029)
+++ src/protocol/http/org/apache/jmeter/protocol/http/sampler/SoapSampler.java	(working copy)
@@ -141,6 +141,7 @@
                 Header hd = mngr.getHeader(idx);
                 if (HTTPConstants.HEADER_CONTENT_LENGTH.equalsIgnoreCase(hd.getName())) {// Use this to override file length
                     length = Integer.parseInt(hd.getValue());
+                    break;
                 }
                 // All the other headers are set up by HTTPSampler2.setupConnection()
             }

@asfimport
Copy link
Collaborator Author

@pmouawad (migrated from Bugzilla):
Thanks for patch, applied.

Date: Sun Jun 30 13:00:30 2013
New Revision: 1498110

URL: http://svn.apache.org/r1498110
Log:
#3147 - Useless processing in SoapSampler.setPostHeaders
#3147

Modified:
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/SoapSampler.java
jmeter/trunk/xdocs/changes.xml

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

No branches or pull requests

1 participant