jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1806215 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java xdocs/changes.xml
Date Fri, 01 Sep 2017 20:53:32 GMT
Hi Felix,
Did you have time to look at this ?

I think it's the remaining issue before releasing a 3.3 version.

Regards

On Tue, Aug 29, 2017 at 4:18 PM, Philippe Mouawad <
philippe.mouawad@gmail.com> wrote:

> Hi Felix,
> Am I wrong to think that I should revet my last commit (which uses charset
> for parameters) ?
> and it would be acceptable that unencoded Chinese named file would end up
> being corrupt ?
>
> This is my understand as per Oleg answer and yours.
>
> Thanks
>
> On Mon, Aug 28, 2017 at 11:06 AM, Felix Schumacher <felix.schumacher@
> internetallee.de> wrote:
>
>>
>>
>> Am 27. August 2017 22:23:20 MESZ schrieb Philippe Mouawad <
>> philippe.mouawad@gmail.com>:
>> >Might be interesting :
>> >https://stackoverflow.com/questions/20591599/why-arent-post
>> -names-with-unicode-sent-correctly-when-using-multipart-
>> form-data/20592910#20592910
>>
>> In my opinion RFC 2388 4.4 applies here and the filenames can be encoded
>> using RFC 2231.
>>
>> Felix
>>
>> >
>> >On Sun, Aug 27, 2017 at 8:04 PM, Philippe Mouawad <
>> >philippe.mouawad@gmail.com> wrote:
>> >
>> >> Hi Felix,
>> >> I noticed while testing that Java Implementation also corrupts
>> >Parameter
>> >> name, so I think it's a bug also, but  I have a doubt regarding the
>> >> encoding of parameter names, does RFC force a particular encoding for
>> >them
>> >> (US-ASCII) or can they be encoded in whatever encoding we want ?
>> >>
>> >> If you look at current code, I have added a check for Java
>> >Implementation
>> >> to check for corrupt "?_param" instead of "安_param"
>> >>
>> >> Please review and give your feedback.
>> >>
>> >> Thanks
>> >>
>> >> On Sun, Aug 27, 2017 at 2:41 PM, Philippe Mouawad <
>> >> philippe.mouawad@gmail.com> wrote:
>> >>
>> >>> Hi Felix,
>> >>> Look also at this report for Aka HTTP following their fix to
>> >>> https://github.com/akka/akka-http/issues/338
>> >>>
>> >>>    - https://github.com/akka/akka-http/issues/647
>> >>>
>> >>> I confirmed current trunk has a similar issue, see
>> >>> https://bz.apache.org/bugzilla/show_bug.cgi?id=61384#c6.
>> >>>
>> >>> So I committed my alternative patch, please review.
>> >>>
>> >>> Still , I don't think it fixes https://bz.apache.org/bugzilla
>> >>> /show_bug.cgi?id=56141
>> >>>
>> >>>
>> >>> Regards
>> >>>
>> >>> On Sun, Aug 27, 2017 at 12:21 PM, Philippe Mouawad <
>> >>> philippe.mouawad@gmail.com> wrote:
>> >>>
>> >>>> Hi Felix,
>> >>>> I attached an alternative patch which :
>> >>>>
>> >>>>    - set surrounding header only if we have a charset
>> >>>>    - same for parameters
>> >>>>
>> >>>> I have asked a question on HC client mailing list:
>> >>>>
>> >>>>    - http://mail-archives.apache.org/mod_mbox/hc-httpclient-users
>> >>>>    /201704.mbox/%3CCAH9fUpbxye8-rydo143Bk%3Dr6q2QDJTEndhPmd5GQ3
>> >>>>    TjxtLpDxA%40mail.gmail.com%3E
>> >>>>
>> ><http://mail-archives.apache.org/mod_mbox/hc-httpclient-use
>> rs/201704.mbox/%3CCAH9fUpbxye8-rydo143Bk%3Dr6q2QDJTEndhPmd5G
>> Q3TjxtLpDxA%40mail.gmail.com%3E>
>> >>>>
>> >>>> I think the following bugs have potentially the same root cause:
>> >>>>
>> >>>>    - https://bz.apache.org/bugzilla/show_bug.cgi?id=61384
>> >>>>    - https://bz.apache.org/bugzilla/show_bug.cgi?id=60800
>> >>>>    - https://bz.apache.org/bugzilla/show_bug.cgi?id=56141
>> >>>>
>> >>>> See this interesting comment also:
>> >>>>
>> >>>>    - https://bz.apache.org/bugzilla/show_bug.cgi?id=56141#c4
>> >>>>
>> >>>>
>> >>>> Regards
>> >>>>
>> >>>> On Sun, Aug 27, 2017 at 10:59 AM, Felix Schumacher <
>> >>>> felix.schumacher@internetallee.de> wrote:
>> >>>>
>> >>>>>
>> >>>>>
>> >>>>> Am 26. August 2017 15:11:19 MESZ schrieb Philippe Mouawad <
>> >>>>> philippe.mouawad@gmail.com>:
>> >>>>> >Hi Felix,
>> >>>>> >Are we sure that when encoding is UTF-8 there is no need
to set
>> >charset
>> >>>>> >?
>> >>>>>
>> >>>>> We keep the charset. We only remove it from the surrounding
>> >header.
>> >>>>>
>> >>>>> >
>> >>>>> >AFAIK, there were already issue with Multipart forms even
before
>> >>>>> >refactoring.
>> >>>>>
>> >>>>> Right. The most questions I found stated that they had problems
>> >when
>> >>>>> the charset was set.
>> >>>>>
>> >>>>> What do you think would be the correct way?
>> >>>>>
>> >>>>> Felix
>> >>>>>
>> >>>>> >
>> >>>>> >Thanks
>> >>>>> >Thanks
>> >>>>> >
>> >>>>> >On Fri, Aug 25, 2017 at 9:02 PM, <fschumacher@apache.org>
wrote:
>> >>>>> >
>> >>>>> >> Author: fschumacher
>> >>>>> >> Date: Fri Aug 25 19:02:36 2017
>> >>>>> >> New Revision: 1806215
>> >>>>> >>
>> >>>>> >> URL: http://svn.apache.org/viewvc?rev=1806215&view=rev
>> >>>>> >> Log:
>> >>>>> >> Don't set the charset on enclosing multipart/form-data
header.
>> >It
>> >>>>> >> irritates some servers.
>> >>>>> >>
>> >>>>> >> The charset was added sometime back while refactoring
to use a
>> >newer
>> >>>>> >api
>> >>>>> >> of http client.
>> >>>>> >> See https://bz.apache.org/bugzilla/show_bug.cgi?id=56141
for
>> >more
>> >>>>> >info.
>> >>>>> >>
>> >>>>> >> Bugzilla Id: 61384
>> >>>>> >>
>> >>>>> >>
>> >>>>> >> Modified:
>> >>>>> >>     jmeter/trunk/src/protocol/http/org/apache/jmeter/
>> >>>>> >> protocol/http/sampler/HTTPHC4Impl.java
>> >>>>> >>     jmeter/trunk/xdocs/changes.xml
>> >>>>> >>
>> >>>>> >> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/
>> >>>>> >> protocol/http/sampler/HTTPHC4Impl.java
>> >>>>> >> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/
>> >>>>> >>
>> >>>>> >http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.ja
>> >>>>> va?rev=1806215&
>> >>>>> >> r1=1806214&r2=1806215&view=diff
>> >>>>> >> ============================================================
>> >>>>> >> ==================
>> >>>>> >> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/
>> >>>>> >> protocol/http/sampler/HTTPHC4Impl.java (original)
>> >>>>> >> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/
>> >>>>> >> protocol/http/sampler/HTTPHC4Impl.java Fri Aug 25 19:02:36
2017
>> >>>>> >> @@ -1242,7 +1242,7 @@ public class HTTPHC4Impl extends
HTTPHCA
>> >>>>> >>          if(getUseMultipartForPost()) {
>> >>>>> >>              // If a content encoding is specified,
we use that
>> >as
>> >>>>> >the
>> >>>>> >>              // encoding of any parameter values
>> >>>>> >> -            Charset charset = null;
>> >>>>> >> +            Charset charset;
>> >>>>> >>              if(haveContentEncoding) {
>> >>>>> >>                  charset = Charset.forName(contentEncoding);
>> >>>>> >>              } else {
>> >>>>> >> @@ -1254,8 +1254,7 @@ public class HTTPHC4Impl extends
HTTPHCA
>> >>>>> >>                          getDoBrowserCompatibleMultipart(),
>> >charset,
>> >>>>> >> haveContentEncoding);
>> >>>>> >>              }
>> >>>>> >>              // Write the request to our own stream
>> >>>>> >> -            MultipartEntityBuilder multipartEntityBuilder
=
>> >>>>> >> MultipartEntityBuilder.create()
>> >>>>> >> -                    .setCharset(charset);
>> >>>>> >> +            MultipartEntityBuilder multipartEntityBuilder
=
>> >>>>> >> MultipartEntityBuilder.create();
>> >>>>> >>              if(getDoBrowserCompatibleMultipart())
{
>> >>>>> >>                  multipartEntityBuilder.setLaxMode();
>> >>>>> >>              } else {
>> >>>>> >>
>> >>>>> >> Modified: jmeter/trunk/xdocs/changes.xml
>> >>>>> >> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.
>> >>>>> >> xml?rev=1806215&r1=1806214&r2=1806215&view=diff
>> >>>>> >> ============================================================
>> >>>>> >> ==================
>> >>>>> >> --- jmeter/trunk/xdocs/changes.xml [utf-8] (original)
>> >>>>> >> +++ jmeter/trunk/xdocs/changes.xml [utf-8] Fri Aug
25 19:02:36
>> >2017
>> >>>>> >> @@ -167,6 +167,9 @@ Incorporated feed back about unclear
doc
>> >>>>> >>
>> >>>>> >>  <h3>HTTP Samplers and Test Script Recorder</h3>
>> >>>>> >>  <ul>
>> >>>>> >> +  <li><bug>61384</bug>Don't set
the charset on enclosing
>> >>>>> >> <code>multipart/form-data</code> header.
It irritates some
>> >>>>> >servers.<br/>
>> >>>>> >> +     The charset was added sometime back while refactoring
to
>> >use a
>> >>>>> >newer
>> >>>>> >> api of http client.
>> >>>>> >> +     See <bug>56141</bug> for more info.</li>
>> >>>>> >>  </ul>
>> >>>>> >>
>> >>>>> >>  <h3>Other Samplers</h3>
>> >>>>> >>
>> >>>>> >>
>> >>>>> >>
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> Cordialement.
>> >>>> Philippe Mouawad.
>> >>>>
>> >>>>
>> >>>>
>> >>>
>> >>>
>> >>> --
>> >>> Cordialement.
>> >>> Philippe Mouawad.
>> >>>
>> >>>
>> >>>
>> >>
>> >>
>> >> --
>> >> Cordialement.
>> >> Philippe Mouawad.
>> >>
>> >>
>> >>
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
>
>


-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message