Skip to content

Fix to 63130#467

Closed
sanjeevnec wants to merge 12 commits intoapache:masterfrom
sanjeevnec:master
Closed

Fix to 63130#467
sanjeevnec wants to merge 12 commits intoapache:masterfrom
sanjeevnec:master

Conversation

@sanjeevnec
Copy link
Copy Markdown

Description

Class name: HTTPArgument.java
Method name: HTTPArgument (constructor)
Class name: HttpHC4Impl.java
Method name: createUrlEncodedFormEntity

Replaced java.net.URLDecoder class from org.apache.commons.codec.net.URLCodec class

Motivation and Context

JMeter uses java.net.URLDecoder class for URL encoding.
Its drawback : This library can not encode Shift_JIS correctly because of different specific interpretation between Java and IE.
Because of browser specification, char code is zipped.
Browser encode Japanese text as following.
初期値大阪市 ⇒ %8F%89%8A%FA%92l%91%E5%8D%E3%8Es
初 = %8F%89
期 = %8A%FA
値 = %92l ← % is lack
大 = %91%E5
阪 = %8D%E3
市 = %8Es ← % is lack

JMeter should use org.apache.commons.codec.net.URLCodec class for URL encoding.

https://bz.apache.org/bugzilla/show_bug.cgi?id=63130

How Has This Been Tested?

I have created one sample application, which contain some pages having shift_jis encoding, and deployed the same in tomcat server.
Created the test plan in Jmeter tool and configure it so that it can receive all the incoming request from the application.
Start the Http(s) Test Script Recorder
Run the application, and post the request from the application.
Validate the result in response page.
Run the "ant test" command to be ensured that no existing tests are breaking because of these changes.
Run the "ant package-and-check" to check the coding guidelines.

Other Details:

OS: windows 8
Java: 8
Jmeter version: 5.0
Hardware: PC

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

[x ] My code follows the code style of this project.
I have updated the documentation accordingly.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 14, 2019

Codecov Report

Merging #467 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #467      +/-   ##
============================================
+ Coverage     53.68%   53.68%   +<.01%     
+ Complexity     9402     9401       -1     
============================================
  Files          1000     1000              
  Lines         61554    61557       +3     
  Branches       6937     6937              
============================================
+ Hits          33044    33046       +2     
  Misses        26099    26099              
- Partials       2411     2412       +1
Impacted Files Coverage Δ Complexity Δ
...apache/jmeter/protocol/http/util/HTTPArgument.java 75% <100%> (+5.13%) 23 <0> (ø) ⬇️
...re/org/apache/jmeter/engine/DistributedRunner.java 82.35% <0%> (-3.37%) 31% <0%> (-1%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02b9d8a...6b8c0aa. Read the comment docs.

@sanjeevnec sanjeevnec force-pushed the master branch 4 times, most recently from 9867c1f to be10948 Compare June 25, 2019 11:11

public class TestHTTPArgument {

@Rule
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use 4 spaces for indents

@pmouawad
Copy link
Copy Markdown
Contributor

pmouawad commented Oct 3, 2019

Thanks for this PR.
Still it is not mergeable as is and introduces too much risks of breaking existing tests, as it touches critical parts without covering it with Non Regression Testing.
I am closing it for now.

@pmouawad pmouawad closed this Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants