jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1412311 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java xdocs/changes.xml
Date Sun, 25 Nov 2012 19:50:15 GMT
Hello sebb,
Which use case leads to the use of setPath ?
Looking a call hierarchy I only see:

   - AnchorModifier
   - URL Rewriting modifier
   - Webservice sampler
   - access log sampler

In fact I don't understand what has been fixed with this issue.

I am not sure the fix does its job, as the path is set in UrlConfigGui
through the call to:

   - element.setProperty(HTTPSamplerBase.PATH, path.getText());
   - and not element.setPath()

Which makes this code never called for HttpSampler.


Regards

Philippe

On Thu, Nov 22, 2012 at 12:58 AM, sebb <sebbaz@gmail.com> wrote:

> On 21 November 2012 23:46, sebb <sebbaz@gmail.com> wrote:
> > On 21 November 2012 22:56, Philippe Mouawad <philippe.mouawad@gmail.com>
> wrote:
> >> On Wednesday, November 21, 2012, wrote:
> >>
> >>> Author: sebb
> >>> Date: Wed Nov 21 21:36:36 2012
> >>> New Revision: 1412311
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1412311&view=rev
> >>> Log:
> >>> Allow query strings in paths that start with HTTP or HTTPS
> >>> (so setPath behaves the same as if the path were set in the GUI)
> >>> Bugzilla Id: 54185
> >>>
> >>> Modified:
> >>>
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
> >>>     jmeter/trunk/xdocs/changes.xml
> >>>
> >>> Modified:
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
> >>> URL:
> >>>
> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java?rev=1412311&r1=1412310&r2=1412311&view=diff
> >>>
> >>>
> ==============================================================================
> >>> ---
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
> >>> (original)
> >>> +++
> >>>
> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
> >>> Wed Nov 21 21:36:36 2012
> >>> @@ -395,8 +395,9 @@ public abstract class HTTPSamplerBase ex
> >>>      }
> >>>
> >>>      /**
> >>> -     * Sets the PATH property; also calls {@link
> #parseArguments(String,
> >>> String)}
> >>> -     * to extract and store any query arguments if the request is a
> GET
> >>> or DELETE.
> >>> +     * Sets the PATH property; if the request is a GET or DELETE (and
> the
> >>> path
> >>> +     * does not start with http[s]://) it also calls {@link
> >>> #parseArguments(String, String)}
> >>> +     * to extract and store any query arguments.
> >>>       *
> >>>       * @param path
> >>>       *            The new Path value
> >>> @@ -404,7 +405,8 @@ public abstract class HTTPSamplerBase ex
> >>>       *            The encoding used for the querystring parameter
> values
> >>>       */
> >>>      public void setPath(String path, String contentEncoding) {
> >>> -        if (HTTPConstants.GET.equals(getMethod()) ||
> >>> HTTPConstants.DELETE.equals(getMethod())) {
> >>> +        boolean fullUrl = path.startsWith(HTTP_PREFIX) ||
> >>> path.startsWith(HTTPS_PREFIX);
> >>> +        if (!fullUrl && (HTTPConstants.GET.equals(getMethod())
||
> >>> HTTPConstants.DELETE.equals(getMethod()))) {
> >>>              int index = path.indexOf(QRY_PFX);
> >>>              if (index > -1) {
> >>>                  setProperty(PATH, path.substring(0, index));
> >>>
> >>> Pa
> >>  Patch seems to me a bit risky as of impacts.
> >> Previously with full path and arguments were parsed and encoded If
> >> contenttype provided.
> >> Now they won't be, what if a test plan has assumed they were ?
> >
> > Does not affect paths provided via the GUI which were stored as is
> anyway.
> >
> > I'm not sure anything else should be providing full URLs (i.e. with
> > http[s]: prefix).
>
> I've just run a full test, and the fullUrl boolean was never set true.
> However, I suppose it's theoretically possible there is test that
> relies on the parameters being extracted encoded.
>
> It would also be possible to provide a method such as
>
> setPathProperty(path) {setProperty(PATH, path)
>
> instead.
> Feel free to revert my change and implement that instead.
> >> Modified: jmeter/trunk/xdocs/changes.xml
> >>> URL:
> >>>
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1412311&r1=1412310&r2=1412311&view=diff
> >>>
> >>>
> ==============================================================================
> >>> --- jmeter/trunk/xdocs/changes.xml (original)
> >>> +++ jmeter/trunk/xdocs/changes.xml Wed Nov 21 21:36:36 2012
> >>> @@ -159,6 +159,7 @@ and right angle bracket (&gt;) in search
> >>>
> >>>  <h3>HTTP Samplers</h3>
> >>>  <ul>
> >>> +<li><bugzilla>54185</bugzilla> - Allow query strings
in paths that
> start
> >>> with HTTP or HTTPS</li>
> >>>  </ul>
> >>>
> >>>  <h3>Other samplers</h3>
> >>>
> >>>
> >>>
> >>
> >> --
> >> Cordialement.
> >> Philippe Mouawad.
>



-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message