On 5 December 2013 12:52, Philippe Mouawad <[email protected]> wrote: > Hello sebb, > I missed the fact it was a draft , thanks for pointing at this. > > How come Java implementation handles it ? > Do you think we should rollback this or documenting it is enough ? > > Can you point me to the paragraph in RFC2616 that talks about Location , is > it this one: > http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30
Yes; that says: Location = "Location" ":" absoluteURI > So it means we didn't have a bug ? just a change in behaviour as we handled > it according to draft in previous versions < 2.10? Depends how you class a bug. Unfortunately servers don't always follow the specs - or the specs may take a while to be updated. So sometimes JMeter may need to be more lenient than should be necessary. As I already wrote, we need to ensure that the code and documentation is clear on what we are trying to support. In this case, if a server keeps to RFC 2616 JMeter will still behave correctly. The additional processing for non-absolute URLs does not affect this, but it does mean that JMeter should be able to cope better with the de facto behaviour of some servers. We could perhaps make the new processing depend on a property, so that users could switch it off in order to revert to strict RFC2616 mode. In this case I think it makes sense to allow the relaxed behaviour by default, as it does not affect well-behaved servers. > Regards > Philippe > > > On Mon, Dec 2, 2013 at 1:07 AM, sebb <[email protected]> wrote: > >> I've just looked again at this. >> >> RFC 2616 only allows Location to be an absolute URL. >> >> There does not seem to be a current RFC that allows any other kind of URL. >> >> The latest version of draft-ietf-httpbis-p2-semantics [1] has yet to >> be ratified. >> >> At the very least I think we need to document the code to say that the >> behaviour follows a document that has yet to be ratified. >> >> [1] http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-25 >> >> On 30 November 2013 09:24, Philippe Mouawad <[email protected]> >> wrote: >> > Hello, >> > Any feedback? >> > Thanks >> > >> > On Sunday, November 10, 2013, Philippe Mouawad wrote: >> > >> >> Did you look at the rfc I pointed at in bugzilla? >> >> It is allowed to have relative references . >> >> Or I misunderstand the issue you are pointing at. >> >> >> >> With fix we behave like java implementation. >> >> >> >> Regards >> >> >> >> On Thursday, November 7, 2013, sebb wrote: >> >> >> >>> On 6 November 2013 02:11, Philippe Mouawad <[email protected] >> > >> >>> wrote: >> >>> > Is there something wrong or it's just a note ypu make ? >> >>> > Thanks for clarifying. >> >>> >> >>> It may be something wrong. We should not change location URLs except >> >>> those that are supposed to be changed. >> >>> >> >>> It would therefore be better (and simpler) to check the location URL >> >>> and fix up any that start with "/" - any others can be left alone. >> >>> >> >>> > On Wednesday, November 6, 2013, sebb wrote: >> >>> > >> >>> >> On 2 November 2013 21:53, <[email protected]> wrote: >> >>> >> > Author: pmouawad >> >>> >> > Date: Sat Nov 2 21:53:49 2013 >> >>> >> > New Revision: 1538291 >> >>> >> > >> >>> >> > URL: http://svn.apache.org/r1538291 >> >>> >> > Log: >> >>> >> > Bug 55717 - Bad handling of Redirect when URLs are in relative >> >>> format by >> >>> >> HttpClient4 and HttpClient31 >> >>> >> > Bugzilla Id: 55717 >> >>> >> > >> >>> >> > Modified: >> >>> >> > >> >>> >> >> >>> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java >> >>> >> > >> >>> >> >> >>> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java >> >>> >> > >> >>> >> >> >>> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/util/ConversionUtils.java >> >>> >> > jmeter/trunk/xdocs/changes.xml >> >>> >> > >> >>> >> > Modified: >> >>> >> >> >>> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java >> >>> >> > URL: >> >>> >> >> >>> >> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java?rev=1538291&r1=1538290&r2=1538291&view=diff >> >>> >> > >> >>> >> >> >>> >> ============================================================================== >> >>> >> > --- >> >>> >> >> >>> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java >> >>> >> (original) >> >>> >> > +++ >> >>> >> >> >>> >> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC3Impl.java >> >>> >> Sat Nov 2 21:53:49 2013 >> >>> >> > @@ -321,7 +321,12 @@ public class HTTPHC3Impl extends HTTPHCA >> >>> >> > throw new IllegalArgumentException("Missing >> >>> >> location header"); >> >>> >> > } >> >>> >> > try { >> >>> >> > - >> >>> >> res.setRedirectLocation(ConversionUtils.sanitizeUrl(new >> >>> >> URL(headerLocation.getValue())).toString()); >> >>> >> > + String redirectLocation = >> >>> headerLocation.getValue(); >> >>> >> > + if(!(redirectLocation.startsWith("http:// >> >>> >> ")||redirectLocation.startsWith("https://";))) { >> >>> >> > + redirectLocation = >> >>> >> ConversionUtils.buildFullUrlFromRelative(url, redirectLocation); >> >>> >> > + } >> >>> >> > + >> >>> >> > + >> >>> >> res.setRedirectLocation(ConversionUtils.sanitizeUrl(new >> >>> >> URL(redirectLocation)).toString()); >> >>> >> > } catch (Exception e) { >> >>> >> > log.error("Error sanitizing >> >>> >> URL:"+headerLocation.getValue()+", message:"+e.getMessage()); >> >>> >> > } >> >>> >> > >> >>> >> > 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.java?rev=1538291&r1=1538290&r2=1538291&view=diff >> >>> >> > >> >>> >> >> >>> >> ============================================================================== >> >>> >> > --- >> >>> >> jmeter/trunk/src/protocol/ >> >> >> >> >> >> >> >> -- >> >> Cordialement. >> >> Philippe Mouawad. >> >> >> >> >> >> >> >> >> > >> > -- >> > Cordialement. >> > Philippe Mouawad. >> > > > > -- > Cordialement. > Philippe Mouawad.
