Bug 63025 - Search and replace not working for port number
Summary: Search and replace not working for port number
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: HTTP (show other bugs)
Version: 5.0
Hardware: All All
: P2 minor (vote)
Target Milestone: JMETER_5.1
Assignee: JMeter issues mailing list
URL:
Keywords: FixedInTrunk
Depends on:
Blocks:
 
Reported: 2018-12-20 11:02 UTC by j8t
Modified: 2019-01-07 08:28 UTC (History)
2 users (show)



Attachments
Jar to be replaced in JMETER_DIR\lib\ext\ (441.44 KB, patch)
2019-01-02 10:02 UTC, Mohamed Ibrahim
Details | Diff
Source code to fix search and replace functionality in Jmeter 5.0 (85.79 KB, text/x-csrc)
2019-01-02 10:05 UTC, Mohamed Ibrahim
Details
Replace port and protocol in HTTP Samplers (3.62 KB, patch)
2019-01-02 12:09 UTC, Felix Schumacher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description j8t 2018-12-20 11:02:31 UTC
Description

Port numbers of HTTP requests cannot be changed through search and replace.

Steps to reproduce

1. Create a HTTP Request sampler with a port 'XXX'
2. Click [ctrl+f]
3. Enter the port 'XXX' into the search field.
4. Enter something into the 'Replace by' field.
Comment 1 Mohamed Ibrahim 2019-01-02 10:02:22 UTC
Created attachment 36358 [details]
Jar to be replaced in JMETER_DIR\lib\ext\

Replace the jar to get the search and replace functionality for entire Jmeter test plan.
Comment 2 Mohamed Ibrahim 2019-01-02 10:05:04 UTC
Created attachment 36359 [details]
Source code to fix search and replace functionality in Jmeter 5.0




Source code location to replace:
/jmeter/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
Comment 3 Mohamed Ibrahim 2019-01-02 10:07:08 UTC
Hello,

Issue has been fixed by adding a code block in Jmeter Http sampler base.

Kindly verify and confirm.


Thanks,
Mohamed Ibrahim
Comment 4 Felix Schumacher 2019-01-02 12:06:39 UTC
A bug entry should be closed, when the bug report is either invalid, or solved in trunk.

As this is a valid report (thank you for opening it) and it hasn't been fixed in trunk, it should be in open status.
Comment 5 Felix Schumacher 2019-01-02 12:09:54 UTC
Created attachment 36360 [details]
Replace port and protocol in HTTP Samplers

This is basically the diff from the originally attached source. It has been refactored to eliminate duplicate code. That should make it easy to add more parameters to be replaced, like the proxy parameters.
Comment 6 Philippe Mouawad 2019-01-02 13:24:03 UTC
(In reply to Felix Schumacher from comment #5)
> Created attachment 36360 [details]
> Replace port and protocol in HTTP Samplers
> 
> This is basically the diff from the originally attached source. It has been
> refactored to eliminate duplicate code. That should make it easy to add more
> parameters to be replaced, like the proxy parameters.

Hello, 
Thanks for patch.
Look good to me except that if I understand correctly, it would not be possible to replace port by ${portNumber} due to this:

(s) -> setPort(Integer.parseInt(s)

Regards
Comment 7 Mohamed Ibrahim 2019-01-02 14:14:28 UTC
Hello all,

This is my first try in contribution to open source.
I just followed the steps given in the following apache page https://jmeter.apache.org/building.html



I did exactly whatever has been given in the instructions. But, I don't know how it was not updated in the trunk.

You can still check my forked repo in github.

https://github.com/rollno748/jmeter/blob/63025/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerProxy.java


I was trying my best to do it to push the code.

Please let me know where i went wrong and Is there any proper documentation available code committing?
Comment 8 Felix Schumacher 2019-01-02 18:55:33 UTC
(In reply to Philippe Mouawad from comment #6)
> (In reply to Felix Schumacher from comment #5)
> > Created attachment 36360 [details]
> > Replace port and protocol in HTTP Samplers
> > 
> > This is basically the diff from the originally attached source. It has been
> > refactored to eliminate duplicate code. That should make it easy to add more
> > parameters to be replaced, like the proxy parameters.
> 
> Hello, 
> Thanks for patch.
> Look good to me except that if I understand correctly, it would not be
> possible to replace port by ${portNumber} due to this:
> 
> (s) -> setPort(Integer.parseInt(s)

That has to be done, as setPort accepts int, only. The expression is probably set on the ui element and not on the sampler.


> 
> Regards
Comment 9 Felix Schumacher 2019-01-02 19:03:18 UTC
(In reply to Mohamed Ibrahim from comment #7)
> Hello all,
> 
> This is my first try in contribution to open source.
> I just followed the steps given in the following apache page
> https://jmeter.apache.org/building.html

And you did great.

> 
> 
> 
> I did exactly whatever has been given in the instructions. But, I don't know
> how it was not updated in the trunk.

JMeter's trunk can only be updated by Committers. See https://www.apache.org/foundation/getinvolved.html for more on this.

I suspect, you followed the "Create a PR using GIT" instruction on the building page. There is one last step there, that you missed. It is the creation of the pull request. That way we would have been notified of your changes.

For more information and or further discussion about contributing, I suggest that you subscribe to the users (or developers) mailing list and send your questions/suggestions there.


> 
> You can still check my forked repo in github.
> 
> https://github.com/rollno748/jmeter/blob/63025/src/protocol/http/org/apache/
> jmeter/protocol/http/sampler/HTTPSamplerProxy.java
> 
> 
> I was trying my best to do it to push the code.

And thanks again for trying.

> 
> Please let me know where i went wrong and Is there any proper documentation
> available code committing?
Comment 10 Philippe Mouawad 2019-01-04 19:50:55 UTC
(In reply to Felix Schumacher from comment #8)
> (In reply to Philippe Mouawad from comment #6)
> > (In reply to Felix Schumacher from comment #5)
> > > Created attachment 36360 [details]
> > > Replace port and protocol in HTTP Samplers
> > > 
> > > This is basically the diff from the originally attached source. It has been
> > > refactored to eliminate duplicate code. That should make it easy to add more
> > > parameters to be replaced, like the proxy parameters.
> > 
> > Hello, 
> > Thanks for patch.
> > Look good to me except that if I understand correctly, it would not be
> > possible to replace port by ${portNumber} due to this:
> > 
> > (s) -> setPort(Integer.parseInt(s)
> 
> That has to be done, as setPort accepts int, only. The expression is
> probably set on the ui element and not on the sampler.
> 
> 
> > 
> > Regards

Hi Felix,
I think we can safely introduce this setPort(String value) method.
When we read it, it's read as StringProperty.

I don't want to commit your work as you should be the committer (I don't want to have credit for your work :-) ), so if you can commit this code, I can add it later or you can also do it if you want.

Thanks
Comment 11 Felix Schumacher 2019-01-05 12:24:08 UTC
Thanks for the report and initial code example.

This will be included in version 5.1. It would be really nice, if you could test the next nightly, if this fix does what you want. (You can close this enhancement, if it works for you)

In the end I decided to change directly the properties that contain the string representation for port and protocol instead of the derived values, that getPort and getProtocol return.

Date: Sat Jan  5 12:06:49 2019
New Revision: 1850475

URL: http://svn.apache.org/viewvc?rev=1850475&view=rev
Log:
Extract duplicate code into private method

Refactoring to make bug 63025 easier to fix.

Bugzilla Id: 63025

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java

Date: Sat Jan  5 12:19:26 2019
New Revision: 1850478

URL: http://svn.apache.org/viewvc?rev=1850478&view=rev
Log:
Enhance Search & Replace functionality for HTTP Request to include port and protocol field.

Bugzilla Id: 63025

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
    jmeter/trunk/xdocs/changes.xml
Comment 12 Felix Schumacher 2019-01-05 12:27:07 UTC
(In reply to Philippe Mouawad from comment #10)
> (In reply to Felix Schumacher from comment #8)
> > (In reply to Philippe Mouawad from comment #6)
> > > (In reply to Felix Schumacher from comment #5)
> > > > Created attachment 36360 [details]
> > > > Replace port and protocol in HTTP Samplers
> > > > 
> > > > This is basically the diff from the originally attached source. It has been
> > > > refactored to eliminate duplicate code. That should make it easy to add more
> > > > parameters to be replaced, like the proxy parameters.
> > > 
> > > Hello, 
> > > Thanks for patch.
> > > Look good to me except that if I understand correctly, it would not be
> > > possible to replace port by ${portNumber} due to this:
> > > 
> > > (s) -> setPort(Integer.parseInt(s)
> > 
> > That has to be done, as setPort accepts int, only. The expression is
> > probably set on the ui element and not on the sampler.
> > 
> > 
> > > 
> > > Regards
> 
> Hi Felix,
> I think we can safely introduce this setPort(String value) method.
> When we read it, it's read as StringProperty.

I opted to change the property directly. That makes it really easy to add more properties if we want to change them as well.

I wonder if we should use that path for the already implemented getDomain and getPath as getPath changes the property value by encoding spaces.

> 
> I don't want to commit your work as you should be the committer (I don't
> want to have credit for your work :-) ), so if you can commit this code, I
> can add it later or you can also do it if you want.

No problem, I would not have been angry with you.

> 
> Thanks
Comment 13 Felix Schumacher 2019-01-05 12:46:17 UTC
Date: Sat Jan  5 12:45:32 2019
New Revision: 1850484

URL: http://svn.apache.org/viewvc?rev=1850484&view=rev
Log:
Mention contributor of initial code for fix of bug 63025

Bugzilla Id: 63025

Modified:
    jmeter/trunk/xdocs/changes.xml
Comment 14 Felix Schumacher 2019-01-05 21:52:00 UTC
Date: Sat Jan  5 21:51:33 2019
New Revision: 1850515

URL: http://svn.apache.org/viewvc?rev=1850515&view=rev
Log:
Further code deduplication for Replaceable

Part of Bugzilla Id: 63025

Modified:
    jmeter/trunk/src/jorphan/org/apache/jorphan/util/JOrphanUtils.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/HeaderManager.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
    jmeter/trunk/test/src/org/apache/jorphan/util/TestJorphanUtils.java
Comment 15 Felix Schumacher 2019-01-05 21:57:05 UTC
Date: Sat Jan  5 21:56:31 2019
New Revision: 1850516

URL: http://svn.apache.org/viewvc?rev=1850516&view=rev
Log:
Correct number of replaced values

Bugzilla Id: 63025

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/HeaderManager.java