jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Schumacher <felix.schumac...@internetallee.de>
Subject Re: svn commit: r1806215 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java xdocs/changes.xml
Date Sat, 02 Sep 2017 10:40:34 GMT
Am Dienstag, den 29.08.2017, 16:18 +0200 schrieb Philippe Mouawad:
> 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.

My understanding of the rfc is, that the parameter defines the default
encoding of the bodies of the parts. We specify the encodings for each
part explicitly, so we can drop the default encoding.

My impression is, that it would be helpful to drop the default
encoding, as it seems to interfere with some servers (in particular
IIS). That seems to be regardless of the used encoding.

The case of the unencoded file names is - in my opinion - not part of
the encoding specification. It is part of the header and has to be
handled differently (by using quoted printable encoding).

So all in all, I think it would be better to revert your patches.

Regards,
 Felix

> 
> 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#c
> > > > > > 4
> > > > > > 
> > > > > > 
> > > > > > 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.or
> > > > > > > > g> wrote:
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Author: fschumacher
> > > > > > > > > Date: Fri Aug 25 19:02:36 2017
> > > > > > > > > New Revision: 1806215
> > > > > > > > > 
> > > > > > > > > URL: http://svn.apache.org/viewvc?rev=1806215&view=re
> > > > > > > > > v
> > > > > > > > > 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=56
> > > > > > > > > 141 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/pr
> > > > > > > > > otocol/
> > > > > > > > > 
> > > > > > > > http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Imp
> > > > > > > > l.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
> > > > > > > > >                          getDoBrowserCompatibleMultip
> > > > > > > > > art(),
> > > 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.
> > > > 
> > > > 
> > > > 
> 
> 

Mime
View raw message