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 Tue, 29 Aug 2017 14:18:26 GMT
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-
> users/201704.mbox/%3CCAH9fUpbxye8-rydo143Bk%3Dr6q2QDJTEndhPmd5GQ3TjxtLpDxA
> %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.

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