You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by sebb <se...@gmail.com> on 2013/11/06 02:12:18 UTC

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

On 2 November 2013 21:53,  <pm...@apache.org> 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/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java (original)
> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java Sat Nov  2 21:53:49 2013
> @@ -332,7 +332,10 @@ public class HTTPHC4Impl extends HTTPHCA
>                  if (headerLocation == null) { // HTTP protocol violation, but avoids NPE
>                      throw new IllegalArgumentException("Missing location header in redirect for " + httpRequest.getRequestLine());
>                  }
> -                final String redirectLocation = headerLocation.getValue();
> +                String redirectLocation = headerLocation.getValue();
> +                if(!(redirectLocation.startsWith("http://")|| redirectLocation.startsWith("https://"))) {
> +                    redirectLocation = ConversionUtils.buildFullUrlFromRelative(url, redirectLocation);
> +                }
>                  try {
>                      final URL redirectUrl = new URL(redirectLocation);
>                      res.setRedirectLocation(ConversionUtils.sanitizeUrl(redirectUrl).toString());
>
> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/util/ConversionUtils.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/util/ConversionUtils.java?rev=1538291&r1=1538290&r2=1538291&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/util/ConversionUtils.java (original)
> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/util/ConversionUtils.java Sat Nov  2 21:53:49 2013
> @@ -262,4 +262,54 @@ public class ConversionUtils {
>          s.replace(pathStartIndex, pathEndIndex, newPath.toString());
>          return s.toString();
>      }
> +
> +    /**
> +     * Builds Full url (containing scheme, host,port) from relative URL
> +     * as per RFC http://tools.ietf.org/html/rfc3986#section-4.2
> +     * @param lastUrl URL
> +     * @param redirectLocation absolute URL
> +     * @return Full URL
> +     *
> +     */
> +    public static final String buildFullUrlFromRelative(URL lastUrl,
> +            String redirectLocation) {
> +        StringBuilder builder = new StringBuilder();
> +        builder.append(lastUrl.getProtocol())
> +            .append("://")
> +            .append(lastUrl.getHost());
> +        if(lastUrl.getPort()!= -1) {
> +            builder.append(":").append(lastUrl.getPort());
> +        }
> +        if(redirectLocation.startsWith("/")) {
> +            // A relative reference that begins with a single slash
> +            // character is termed an absolute-path reference
> +            builder.append(redirectLocation);
> +        } else {

I thought redirects did not allow relative references?

> +            // A relative reference that does not begin with a
> +            // slash character is termed a relative-path reference
> +            // We need to merge a relative-path reference with the path of the base URI
> +            // http://tools.ietf.org/html/rfc3986#section-5.2.3
> +            if(lastUrl.getPath().isEmpty()) {
> +                // If the base URI has a defined authority component and an empty
> +                // path, then return a string consisting of "/" concatenated with the
> +                // reference's path; otherwise,
> +                builder.append("/").append(redirectLocation);
> +            } else {
> +                // string consisting of the reference's path component
> +                // appended to all but the last segment of the base URI's path (i.e.,
> +                // excluding any characters after the right-most "/" in the base URI
> +                // path, or excluding the entire base URI path if it does not contain
> +                // any "/" characters).
> +                String path = lastUrl.getPath();
> +                int index = path.lastIndexOf("/");
> +                if(index == -1) {
> +                    builder.append("/").append(redirectLocation);
> +                } else {
> +                    builder.append(path.substring(0, index+1))
> +                        .append(redirectLocation);
> +                }
> +            }
> +        }
> +        return builder.toString();
> +    }
>  }
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1538291&r1=1538290&r2=1538291&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Sat Nov  2 21:53:49 2013
> @@ -128,6 +128,7 @@ A workaround is to use a Java 7 update 4
>
>  <h3>HTTP Samplers and Proxy</h3>
>  <ul>
> +<li><bugzilla>55717</bugzilla> - Bad handling of Redirect when URLs are in relative format by HttpClient4 and HttpClient31</li>
>  </ul>
>
>  <h3>Other Samplers</h3>
>
>

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by Philippe Mouawad <ph...@gmail.com>.
I commited property addition and change mention.
Not sure naming is good feel free to change it.


On Thu, Dec 5, 2013 at 5:19 PM, sebb <se...@gmail.com> wrote:

> On 5 December 2013 12:52, Philippe Mouawad <ph...@gmail.com>
> 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 <se...@gmail.com> 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 <philippe.mouawad@gmail.com
> >
> >> 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 <
> philippe.mouawad@gmail.com
> >> >
> >> >>> 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,  <pm...@apache.org> 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.
>



-- 
Cordialement.
Philippe Mouawad.

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by sebb <se...@gmail.com>.
On 5 December 2013 12:52, Philippe Mouawad <ph...@gmail.com> 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 <se...@gmail.com> 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 <ph...@gmail.com>
>> 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 <philippe.mouawad@gmail.com
>> >
>> >>> 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,  <pm...@apache.org> 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.

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by Philippe Mouawad <ph...@gmail.com>.
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

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?

Regards
Philippe


On Mon, Dec 2, 2013 at 1:07 AM, sebb <se...@gmail.com> 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 <ph...@gmail.com>
> 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 <philippe.mouawad@gmail.com
> >
> >>> 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,  <pm...@apache.org> 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.

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by sebb <se...@gmail.com>.
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 <ph...@gmail.com> 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 <ph...@gmail.com>
>>> 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,  <pm...@apache.org> 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.

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by Philippe Mouawad <ph...@gmail.com>.
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 <ph...@gmail.com>
>> 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,  <pm...@apache.org> 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.

svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by Philippe Mouawad <ph...@gmail.com>.
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 <ph...@gmail.com>
> 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,  <pm...@apache.org> 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.

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by sebb <se...@gmail.com>.
On 6 November 2013 02:11, Philippe Mouawad <ph...@gmail.com> 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,  <pm...@apache.org> 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/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
>> (original)
>> > +++
>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
>> Sat Nov  2 21:53:49 2013
>> > @@ -332,7 +332,10 @@ public class HTTPHC4Impl extends HTTPHCA
>> >                  if (headerLocation == null) { // HTTP protocol
>> violation, but avoids NPE
>> >                      throw new IllegalArgumentException("Missing I
>> thought redirects did not allow relative references?
>>
>> > +            // A relative reference that does not begin with a
>> > +            // slash character is termed a relative-path reference
>> > +            // We need to merge a relative-path reference with the path
>> of the base URI
>> > +            // http://tools.ietf.org/html/rfc3986#section-5.2.3
>> > +            if(lastUrl.getPath().isEmpty()) {
>> > +                // If the base URI has a defined authority component
>> and an empty
>> > +                // path, then return a string consisting of "/"
>> concatenated with the
>> > +                // reference's path; otherwise,
>> > +                builder.append("/").append(redirectLocation);
>> > +            } else {
>> > +                // string consisting of the reference's path component
>> > +                // appended to all but the last segment of the base
>> URI's path (i.e.,
>> > +                // excluding any characters after the right-most "/" in
>> the base URI
>> > +                // path, or excluding the entire base URI path if it
>> does not contain
>> > +                // any "/" characters).
>> > +                String path = lastUrl.getPath();
>> > +                int index = path.lastIndexOf("/");
>> > +                if(index == -1) {
>> > +                    builder.append("/").append(redirectLocation);
>> > +                } else {
>> > +                    builder.append(path.substring(0, index+1))
>> > +                        .append(redirectLocation);
>> > +                }
>> > +            }
>> > +        }
>> > +        return builder.toString();
>> > +    }
>> >  }
>> >
>> > Modified: jmeter/trunk/xdocs/changes.xml
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1538291&r1=1538290&r2=1538291&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/xdocs/changes.xml (original)
>> > +++ jmeter/trunk/xdocs/changes.xml Sat Nov  2 21:53:49 2013
>> > @@ -128,6 +128,7 @@ A workaround is to use a Java 7 update 4
>> >
>> >  <h3>HTTP Samplers and Proxy</h3>
>> >  <ul>
>> > +<li><bugzilla>55717</bugzilla> - Bad handling of Redirect when URLs are
>> in relative format by HttpClient4 and HttpClient31</li>
>> >  </ul>
>> >
>> >  <h3>Other Samplers</h3>
>> >
>> >
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: svn commit: r1538291 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/ src/protocol/http/org/apache/jmeter/protocol/http/util/ xdocs/

Posted by Philippe Mouawad <ph...@gmail.com>.
Is there something wrong or it's just a note ypu make ?
Thanks for clarifying.

On Wednesday, November 6, 2013, sebb wrote:

> On 2 November 2013 21:53,  <pm...@apache.org> 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/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
> (original)
> > +++
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPHC4Impl.java
> Sat Nov  2 21:53:49 2013
> > @@ -332,7 +332,10 @@ public class HTTPHC4Impl extends HTTPHCA
> >                  if (headerLocation == null) { // HTTP protocol
> violation, but avoids NPE
> >                      throw new IllegalArgumentException("Missing I
> thought redirects did not allow relative references?
>
> > +            // A relative reference that does not begin with a
> > +            // slash character is termed a relative-path reference
> > +            // We need to merge a relative-path reference with the path
> of the base URI
> > +            // http://tools.ietf.org/html/rfc3986#section-5.2.3
> > +            if(lastUrl.getPath().isEmpty()) {
> > +                // If the base URI has a defined authority component
> and an empty
> > +                // path, then return a string consisting of "/"
> concatenated with the
> > +                // reference's path; otherwise,
> > +                builder.append("/").append(redirectLocation);
> > +            } else {
> > +                // string consisting of the reference's path component
> > +                // appended to all but the last segment of the base
> URI's path (i.e.,
> > +                // excluding any characters after the right-most "/" in
> the base URI
> > +                // path, or excluding the entire base URI path if it
> does not contain
> > +                // any "/" characters).
> > +                String path = lastUrl.getPath();
> > +                int index = path.lastIndexOf("/");
> > +                if(index == -1) {
> > +                    builder.append("/").append(redirectLocation);
> > +                } else {
> > +                    builder.append(path.substring(0, index+1))
> > +                        .append(redirectLocation);
> > +                }
> > +            }
> > +        }
> > +        return builder.toString();
> > +    }
> >  }
> >
> > Modified: jmeter/trunk/xdocs/changes.xml
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1538291&r1=1538290&r2=1538291&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/xdocs/changes.xml (original)
> > +++ jmeter/trunk/xdocs/changes.xml Sat Nov  2 21:53:49 2013
> > @@ -128,6 +128,7 @@ A workaround is to use a Java 7 update 4
> >
> >  <h3>HTTP Samplers and Proxy</h3>
> >  <ul>
> > +<li><bugzilla>55717</bugzilla> - Bad handling of Redirect when URLs are
> in relative format by HttpClient4 and HttpClient31</li>
> >  </ul>
> >
> >  <h3>Other Samplers</h3>
> >
> >
>


-- 
Cordialement.
Philippe Mouawad.