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.moua...@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.moua...@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.moua...@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.moua...@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.moua...@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.schumac...@internetallee.de> wrote: >> >>>> >> >>>>> >> >>>>> >> >>>>> Am 26. August 2017 15:11:19 MESZ schrieb Philippe Mouawad < >> >>>>> philippe.moua...@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, <fschumac...@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.
