On Sunday, May 1, 2016, sebb <[email protected]> wrote: > On 1 May 2016 at 22:14, Philippe Mouawad <[email protected] > <javascript:;>> wrote: > > On Sunday, May 1, 2016, sebb <[email protected] <javascript:;>> wrote: > > > >> On 1 May 2016 at 21:27, Philippe Mouawad <[email protected] > <javascript:;> > >> <javascript:;>> wrote: > >> > On Sunday, May 1, 2016, sebb <[email protected] <javascript:;> > <javascript:;>> wrote: > >> > > >> >> On 1 May 2016 at 21:12, Philippe Mouawad <[email protected] > <javascript:;> > >> <javascript:;> > >> >> <javascript:;>> wrote: > >> >> > On Sunday, May 1, 2016, sebb <[email protected] <javascript:;> > <javascript:;> > >> <javascript:;>> wrote: > >> >> > > >> >> >> On 1 May 2016 at 20:53, Philippe Mouawad < > [email protected] <javascript:;> > >> <javascript:;> > >> >> <javascript:;> > >> >> >> <javascript:;>> wrote: > >> >> >> > Hello, > >> >> >> > As you know a regression has been reported on 3.0 related to > >> >> Compressed > >> >> >> > responses management. > >> >> >> > > >> >> >> > HC4.5.2 differs in its behaviour with 4.2.6, it removes 3 > headers > >> >> after > >> >> >> > uncompressing the response: > >> >> >> > - Content-Length > >> >> >> > - Content-Encoding > >> >> >> > - Content-MD5 > >> >> >> > > >> >> >> > I attached a fix to Bug 59401 that introduces a > >> ResponseInterceptor at > >> >> >> > first position to save initial Headers. > >> >> >> > These headers are then used by JMeter to fill in > >> >> >> > SampleResult#responseHeaders > >> >> >> > > >> >> >> > I don't think the fix can introduce regressions but your review > is > >> >> >> welcome > >> >> >> > as long as alternative solutions proposals. > >> >> >> > > >> >> >> > The drawback I see in this patch is that it introduces a new > >> >> >> > ResponseInterceptor and saves Headers in localContext impacting > >> >> slightly > >> >> >> > memory and CPU usage. > >> >> >> > > >> >> >> > > >> >> >> > An alternative solution, would be to modify slightly > >> >> >> > > >> >> >> > >> >> > >> > https://github.com/apache/httpclient/blob/4.5.x/httpclient/src/main/java/org/apache/http/client/protocol/ResponseContentEncoding.java#L142 > >> >> >> > to remove the code that removes the headers. > >> >> >> > >> >> >> -1; the headers cannot remain as they are no longer correct. > >> >> >> > >> >> >> But this can break existing test plans that would use the missing > >> >> headers > >> >> > no ? > >> >> > > >> >> >> However an alternative might be to copy the original values to an > >> >> >> X-prefixed header before removal. > >> >> > > >> >> > isn't it strange that JMeter adds headers ? > >> >> > How users can distinguish between servers headers and jmeter ones ? > >> >> > >> >> X-JMeter-Content-xxx > >> >> > >> >> Also JMeter can remove them again before storing the response. > >> >> They would only be used as temporary storage > >> > > >> > > >> > I don't get the whole picture of what you propose ans how it > >> > avoid breaking tests. > >> > >> You were proposing to add a ResponseInterceptor that saves the headers > >> in localContext > >> > >> I'm suggesting saving them on the response instead. > >> > >> So when processing the response, JMeter looks for > >> > >> X-JMeter-Content-xxx > >> rather than > >> Content-xxx > >> > >> This assumes it knows which reponses have been uncompressed. > >> > >> Alternatively, if it cannot find Content-xxx it looks for > >> X-JMeter-Content-xxx. > > > > > > Doing so it changes response size. > > It's easy enough to calculate the response size after the temporary > headers have been removed. > > > I'm afraid of the impacts of this solution and possible regressions. > > How would it be different from your patch?
First because as I don't have the patch, it looks to me more invasive , so a patch would make the discussion either useless or at least more simple > > > But a patch would make it clear for me. > > It's basically the same as your patch. > Just the storage method is different. > > One reason I proposed this is that it would be a possible option for > HC to provide the headers. I don't get this > I don't know if that would be acceptable, but if it is, then it would > be possible to drop the interceptor. > > > > > > > > >> > >> > Could you provide a patch ? > >> > > >> > thanks > >> > > >> > > >> >> > >> >> > >> >> > >> > > >> >> Thx > >> >> > > >> >> >> > >> >> >> > > >> >> >> > > >> >> >> > Regards > >> >> >> > Philippe > >> >> >> > >> >> > > >> >> > > >> >> > -- > >> >> > Cordialement. > >> >> > Philippe Mouawad. > >> >> > >> > > >> > > >> > -- > >> > Cordialement. > >> > Philippe Mouawad. > >> > > > > > > -- > > Cordialement. > > Philippe Mouawad. > -- Cordialement. Philippe Mouawad.
