Bug 59047

Summary: Http sampler : Redirect configuration is hard to understand
Product: JMeter - Now in Github Reporter: benoit.wiart
Component: MainAssignee: JMeter issues mailing list <issues>
Status: NEW ---    
Severity: enhancement CC: p.mouawad
Priority: P2    
Version: 2.13   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: redirects combo box

Description benoit.wiart 2016-02-22 20:05:16 UTC
Even for a regular user it's hard to understand...

Redirect Automatically -> this one will follow redirects
Follow Redirects -> this one will follow redirects too ...

As an user, I'm too scared to change the checkboxes as it may break the world or randomly add tabs in the jmeter source code.

IMHO we should only keep one of the 2 / merge them.

The labels should at least be modified to make the options more explicits.
Comment 1 benoit.wiart 2016-02-23 08:40:57 UTC
Correct me if I'm wrong but in the current version we have the following options :

- 1 : No follow (both checkboxes not checked)
- 2 : Follow strict (Redirect Automatically is checked)
       GET and HEAD only
       the jmeter documentation mention some pb with headers and cookies
        is it a bug ?
- 3 : Follow lax (Follow Redirects is checked)
       More methods allowed
       keep the subsamples
       sum the bytes

Is number 2 really needed ?
Is there a bug in current number 2 implementation ?

A select list + new labels should be a better ui.
Comment 2 benoit.wiart 2016-02-23 15:19:46 UTC
Created attachment 33584 [details]
redirects combo box

Example of what can be done (missing the human readable labels)
Comment 3 Sebb 2016-02-23 17:34:02 UTC
(In reply to benoit.wiart from comment #1)
> Correct me if I'm wrong but in the current version we have the following
> options :
> 
> - 1 : No follow (both checkboxes not checked)
> - 2 : Follow strict (Redirect Automatically is checked)
>        GET and HEAD only
>        the jmeter documentation mention some pb with headers and cookies
>         is it a bug ?

No, it's just that JMeter does not see the intermediate responses, so relies on the HTTP library to do the correct thing with them.

> - 3 : Follow lax (Follow Redirects is checked)
>        More methods allowed
>        keep the subsamples
>        sum the bytes
> 
> Is number 2 really needed ?

Yes.

> Is there a bug in current number 2 implementation ?

No.
 
> A select list + new labels should be a better ui.
Comment 4 benoit.wiart 2016-02-23 19:10:42 UTC
@sebb what do you think of the ui change ?

currently the httpsampler.max_redirects configuration is not enforced when "Redirect Automatically" is used, is it a bug ?
Comment 5 Sebb 2016-02-24 12:16:29 UTC
(In reply to benoit.wiart from comment #4)
> @sebb what do you think of the ui change ?

I find it much harder to understand.

I think the best is to leave the existing GUI and improve the documentation.
And/or add tooltips with more information.


> currently the httpsampler.max_redirects configuration is not enforced when
> "Redirect Automatically" is used, is it a bug ?

Not a bug; the HTTP implementations provide their own redirect loop handling.
Comment 6 Philippe Mouawad 2016-02-25 20:09:06 UTC
Playing with GUI and reading docs and code, it appears to me that:
- The 3 options are mutually exclusive, so a Select Box is better than checkboxes combinations both for UX and code maintenance and readability
- Proposals of Benoit look good to me with the following notes:
1: No Follow : clear
2: Follow Strict : clear
3: I would rather call it brower_compatibility as it seems it's closer to how browser work


Regarding httpsampler.max_redirects, there is indeed a BUG, as we don't enforce the property "http.protocol.max-redirects" in HTTPHC4Impl to the value of JMeter property "httpsampler.max_redirects".
Comment 7 Sebb 2016-02-25 21:02:23 UTC
(In reply to Philippe Mouawad from comment #6)
> Playing with GUI and reading docs and code, it appears to me that:
> - The 3 options are mutually exclusive, so a Select Box is better than
> checkboxes combinations both for UX and code maintenance and readability

Perhaps; however it will need careful coding to preserve JMX compatibility.

> - Proposals of Benoit look good to me with the following notes:
> 1: No Follow : clear

Agreed

> 2: Follow Strict : clear

Huh? I've no idea what strict means in this case.
Besides, different HTTP implementations may do different things.

The point is that JMeter delegates the redirect handling to the library.

> 3: I would rather call it brower_compatibility as it seems it's closer to
> how browser work

That is clearer.

How about naming the drop-down 'Redirect handling/handler' with options:
- None (no follow)
- HTTP library
- JMeter

> 
> Regarding httpsampler.max_redirects, there is indeed a BUG, as we don't
> enforce the property "http.protocol.max-redirects" in HTTPHC4Impl to the
> value of JMeter property "httpsampler.max_redirects".

That needs a new Bugzilla issue
Comment 8 Philippe Mouawad 2016-02-25 21:20:57 UTC
(In reply to Sebb from comment #7)
> (In reply to Philippe Mouawad from comment #6)
> > Playing with GUI and reading docs and code, it appears to me that:
> > - The 3 options are mutually exclusive, so a Select Box is better than
> > checkboxes combinations both for UX and code maintenance and readability
> 
> Perhaps; however it will need careful coding to preserve JMX compatibility.
> 
> > - Proposals of Benoit look good to me with the following notes:
> > 1: No Follow : clear
> 
> Agreed
> 
> > 2: Follow Strict : clear
> 
> Huh? I've no idea what strict means in this case.
> Besides, different HTTP implementations may do different things.
> 
> The point is that JMeter delegates the redirect handling to the library.
> 
> > 3: I would rather call it brower_compatibility as it seems it's closer to
> > how browser work
> 
> That is clearer.
> 
> How about naming the drop-down 'Redirect handling/handler' with options:
> - None (no follow)
> - HTTP library

Difficult to guess from name what it does. HttpImplementation_based ? but I don't like it either.

Does Java HttpUrlConnection follows redirects on POSTs ? on only on GET/HEAD ?


> - JMeter 

Too technical for me, why not call it BROWSER_COMPATIBILITY , as HttpClient used to call CookieSpecs that matches browser way ?

> 
> > 
> > Regarding httpsampler.max_redirects, there is indeed a BUG, as we don't
> > enforce the property "http.protocol.max-redirects" in HTTPHC4Impl to the
> > value of JMeter property "httpsampler.max_redirects".
> 
> That needs a new Bugzilla issue
Comment 9 Sebb 2016-02-26 00:09:38 UTC
(In reply to Philippe Mouawad from comment #8)
> (In reply to Sebb from comment #7)
> > (In reply to Philippe Mouawad from comment #6)
> > > Playing with GUI and reading docs and code, it appears to me that:
> > > - The 3 options are mutually exclusive, so a Select Box is better than
> > > checkboxes combinations both for UX and code maintenance and readability
> > 
> > Perhaps; however it will need careful coding to preserve JMX compatibility.
> > 
> > > - Proposals of Benoit look good to me with the following notes:
> > > 1: No Follow : clear
> > 
> > Agreed
> > 
> > > 2: Follow Strict : clear
> > 
> > Huh? I've no idea what strict means in this case.
> > Besides, different HTTP implementations may do different things.
> > 
> > The point is that JMeter delegates the redirect handling to the library.
> > 
> > > 3: I would rather call it brower_compatibility as it seems it's closer to
> > > how browser work
> > 
> > That is clearer.
> > 
> > How about naming the drop-down 'Redirect handling/handler' with options:
> > - None (no follow)
> > - HTTP library
> 
> Difficult to guess from name what it does. HttpImplementation_based ? but I
> don't like it either.

"As provided by the HTTP implementation" ?

> Does Java HttpUrlConnection follows redirects on POSTs ? on only on GET/HEAD
> ?

No idea.

> 
> > - JMeter 
> 
> Too technical for me, why not call it BROWSER_COMPATIBILITY , as HttpClient
> used to call CookieSpecs that matches browser way ?

"Browser emulation" ?
Comment 10 Philippe Mouawad 2016-02-26 00:11:08 UTC
(In reply to Sebb from comment #9)
> (In reply to Philippe Mouawad from comment #8)
> > (In reply to Sebb from comment #7)
> > > (In reply to Philippe Mouawad from comment #6)
> > > > Playing with GUI and reading docs and code, it appears to me that:
> > > > - The 3 options are mutually exclusive, so a Select Box is better than
> > > > checkboxes combinations both for UX and code maintenance and readability
> > > 
> > > Perhaps; however it will need careful coding to preserve JMX compatibility.
> > > 
> > > > - Proposals of Benoit look good to me with the following notes:
> > > > 1: No Follow : clear
> > > 
> > > Agreed
> > > 
> > > > 2: Follow Strict : clear
> > > 
> > > Huh? I've no idea what strict means in this case.
> > > Besides, different HTTP implementations may do different things.
> > > 
> > > The point is that JMeter delegates the redirect handling to the library.
> > > 
> > > > 3: I would rather call it brower_compatibility as it seems it's closer to
> > > > how browser work
> > > 
> > > That is clearer.
> > > 
> > > How about naming the drop-down 'Redirect handling/handler' with options:
> > > - None (no follow)
> > > - HTTP library
> > 
> > Difficult to guess from name what it does. HttpImplementation_based ? but I
> > don't like it either.
> 
> "As provided by the HTTP implementation" ?
Yes better
> 
> > Does Java HttpUrlConnection follows redirects on POSTs ? on only on GET/HEAD
> > ?
> 
> No idea.
> 
> > 
> > > - JMeter 
> > 
> > Too technical for me, why not call it BROWSER_COMPATIBILITY , as HttpClient
> > used to call CookieSpecs that matches browser way ?
> 
> "Browser emulation" ?
Yes better
Comment 11 The ASF infrastructure team 2022-09-24 20:38:02 UTC
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/3848