Hmmm, based on Girish's feedback I think perhaps the next step should be to
open a JIRA and further investigate. Thinking about it some more, maybe
this would have an impact on the REST API project we're working on.

On Sun, Aug 26, 2018, 7:57 AM Girish Vasmatkar <
girish.vasmat...@hotwaxsystems.com> wrote:

> Hi Ritesh
>
> It does look like an issue to me.
>
> I believe (correct me if I am wrong) it is not so much about whether GET is
> appropriate here, it is more about that the framework is unable to handle
> multiple request parameters with same name, which is a common case when we
> talk about multiple check boxes on a form representing a single entity. The
> fact that GET is doing it's job correctly when the user is logged in (means
> when you change the method from POST to GET and the user is already logged
> in) and not so much when the user is logged out and the request is made via
> bookmark, shows that the code is not working properly.
>
> Then, there is also an issue with URL encoding and decoding that becomes
> apparent with executing your scenario. Whether it is correct to change the
> form method is arguable, but the code should be able to handle it. If a
> form were to be designed with GET method and the only elements present on
> the form are two check boxes and a text field and if you were to select
> both check boxes and have value with spaces in the text box, this scenario
> would still fail.
>
> I think the simplest approach would have been to just store query string
> (request.getQueryString()) in the session attribute instead of Map (but I
> think it was well pondered upon to use Map) and then just redirecting to
> the saved URL once the user loges in. I did it on my local workstation and
> it just worked perfectly. May be one reason why a map was used instead of
> storing query string was to handle unencoded requests coming from browsers
> such as IE. I may be wrong so please correct me if anyone has an idea
> around this piece of code as to why Map was used instead of storing the
> query string in session to be used later on.
>
> If you can do something to fix the existing code, that would be better
> approach, IMO.
>
> May be it is not a major issue but certainly worthy of having a dedicated
> JIRA for. Everybody, please chime in and provide your thoughts.
>
> Thanks and Best regards,
> Girish Vasmatkar
> HotWax Systems
>
> On Sat, Aug 25, 2018 at 8:03 PM Taher Alkhateeb <
> slidingfilame...@gmail.com>
> wrote:
>
> > Okay, I understand this issue. I don't think it is possible to
> > abstract away a complex search screen with http GET method for
> > bookmarks. The performFind service is quite complex and it is
> > difficult to replicate the requirements using GET. GET is not designed
> > to handle multiple languages, spaces, and other peculiarities that are
> > needed for such a screen to work.
> >
> > There are multiple solutions that I can see here. One of them is
> > simply to create a new entity, let's call it SearchFilter, that saves
> > search parameters, which can be applied later on. Either way, you need
> > to customize, your problem is not OFBiz, your problem is http GET
> > limitations.
> > On Fri, Aug 24, 2018 at 12:35 PM Ritesh Kumar
> > <ritesh.ku...@hotwaxsystems.com> wrote:
> > >
> > > Using the POST method does not append form data to the URL, i.e, the
> > > parameters will not be visible in the URL.
> > > For example, take a Find Screen (say, FindWorkEffort) which send data
> > > through a form with POST method. Apply some filters (say, status). No
> > > applied filters appear in the URL.  Bookmark this page. Next time when
> I
> > > open this bookmark, those applied filters will not be there as this
> page
> > is
> > > being rendered using data from the URL and since the applied filters
> were
> > > not there in the bookmarked URL, this page is rendered without the
> > applied
> > > filters. That is why I used GET form method so that I am able to get
> the
> > > page with applied filters when I open a bookmarked page.
> > >
> > > The bug here is (supposing the GET method is used)
> > > 1. On opening the bookmark, the page is rendered with double encoding
> (if
> > > the value had a space character initially, the space character was
> > already
> > > encoded into '+' in the URL and when this bookmark is opened, this '+'
> is
> > > again encoded).
> > > 2. Suppose the bookmarked URL had multiple values from the same filter
> > > (say, Cancelled and Declined status), it renders with just one of the
> > > statutes applied. It is because the request handler prepares a Map of
> > > parameters from the query string and as is the property of Map to
> replace
> > > the old value if a new value is being added with the same key (in this
> > > example, first Cancelled status is put in this Map and then Declined),
> > only
> > > Declined status is put in this Map.
> > >
> > > Hope, this clears the confusion. I will be happy to provide more
> > > information if needed.
> > >
> > > On Fri, Aug 24, 2018 at 1:46 PM Taher Alkhateeb <
> > slidingfilame...@gmail.com>
> > > wrote:
> > >
> > > > Not enough information. What happens exactly? What is the bug? What
> do
> > you
> > > > mean by it does not let us do that?
> > > >
> > > > On Fri, Aug 24, 2018, 11:09 AM Ritesh Kumar <
> > > > ritesh.ku...@hotwaxsystems.com>
> > > > wrote:
> > > >
> > > > > Hello Taher,
> > > > >
> > > > > Changing form method to GET is just to make the query parameters
> > visible
> > > > in
> > > > > the URL so that a user is able to bookmark or share it. Using the
> > POST
> > > > > method does not let us do that.
> > > > >
> > > > > On Fri, Aug 24, 2018 at 11:54 AM Taher Alkhateeb <
> > > > > slidingfilame...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Why did you change the method to GET?
> > > > > >
> > > > > > On Fri, Aug 24, 2018, 9:20 AM Ritesh Kumar <
> > > > > ritesh.ku...@hotwaxsystems.com
> > > > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Just to put my point more clearly, let me add the steps to
> > generate
> > > > the
> > > > > > > above-mentioned case. Please refer demo-trunk
> > > > > > > <https://demo-trunk.ofbiz.apache.org/webtools/control/main>.
> > > > > > >
> > > > > > > 1. Open this link, FindWorkEffort
> > > > > > > <
> > > >
> https://demo-trunk.ofbiz.apache.org/workeffort/control/FindWorkEffort
> > > > > >.
> > > > > > > Find Work Effort screen will be rendered.
> > > > > > > 2. Inspect and change the form method to "GET".
> > > > > > > 3. Apply any of the two statuses (say, Cancelled and Declined).
> > Click
> > > > > on
> > > > > > > Find.
> > > > > > > 4. Records will be fetched according to the applied filters.
> > > > > > > 5. Check the URL. Cancelled and Declined statuses must be there
> > in
> > > > the
> > > > > > URL.
> > > > > > > 6. Bookmark this page and log out.
> > > > > > > 7. Now, open the bookmark.
> > > > > > > 8. The login page will be rendered. Check the URL here. It will
> > be
> > > > the
> > > > > > same
> > > > > > > as it was when the page was being bookmarked.
> > > > > > > 9. Type in the credentials and log in.
> > > > > > > 10. The result may be different. Check the URL. One of the
> > statuses
> > > > is
> > > > > > > gone.
> > > > > > >
> > > > > > > Due to business requirement, I need to show query parameters in
> > the
> > > > URL
> > > > > > so
> > > > > > > that the user is able to bookmark the page. And, we normally
> > pass Id
> > > > in
> > > > > > the
> > > > > > > parameters, but, due to some reason, I may have to pass values
> > with
> > > > > space
> > > > > > > characters.
> > > > > > >
> > > > > > > I hope, this demo puts forth my concern.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Thu, Aug 23, 2018 at 6:27 PM Ritesh Kumar <
> > > > > > > ritesh.ku...@hotwaxsystems.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hello All,
> > > > > > > >
> > > > > > > > I faced an issue while trying to open a bookmarked page with
> > OFBiz.
> > > > > > > >
> > > > > > > > Suppose, the URL of this bookmarked page contains a parameter
> > with
> > > > > > > > multiple values and the value may have space character. The
> > query
> > > > > > string
> > > > > > > in
> > > > > > > > the URL looks somewhat like this
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> "?categoryHierarchy=3%2FCompany+Catalog%2FBrowse+Root%2FCloths%2FMen%2F"&statusId=approved&statusId=created".
> > > > > > > > The "%2F" and "+" are encoded value of  "/", a separator and
> > space
> > > > > > > > character respectively. The status id parameter appears twice
> > and
> > > > the
> > > > > > > > category hierarchy value has space character.
> > > > > > > >
> > > > > > > > The user is logged out at this instance and this bookmarked
> > page is
> > > > > > > > opened. Since the user is not logged in, the login page is
> > > > rendered.
> > > > > I
> > > > > > > feed
> > > > > > > > in the credentials and the intended URL is hit. Here, I do
> not
> > get
> > > > > the
> > > > > > > > required result.
> > > > > > > >
> > > > > > > > When I check the URL, the parameter with multiple values just
> > has
> > > > the
> > > > > > > last
> > > > > > > > value of the list and "+" is encoded into "%2B". The URL now
> is
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> "?categoryHierarchy=3%2FCompany%2BCatalog%2FBrowse%2BRoot%2FCloths%2FMen%2F"&statusId==created."
> > > > > > > >
> > > > > > > > I did some digging and found out that
> LoginWorker.checkLogin()
> > > > comes
> > > > > > into
> > > > > > > > action and what it does is that it creates a new session
> object
> > > > > > (because
> > > > > > > > the previous session becomes invalid) and in the session
> > object, it
> > > > > > puts
> > > > > > > > the previous URL parameters. This previous URL parameters are
> > > > fetched
> > > > > > > using
> > > > > > > > UtilHttp.getUrlOnlyParameterMap(request) which internally
> calls
> > > > > > > > getQueryStringOnlyParameterMap(). This method returns a map
> by
> > > > > breaking
> > > > > > > the
> > > > > > > > query string into key and value pair. A map can not have
> > duplicate
> > > > > keys
> > > > > > > (in
> > > > > > > > this case removes the approved status) and the value is not
> > decoded
> > > > > > > before
> > > > > > > > putting it into the map ('+' is not decoded). This map is
> then
> > used
> > > > > to
> > > > > > > > create an encoded ('+' is encoded into '%2B' ) redirect
> target
> > and
> > > > > then
> > > > > > > > callRedirect() is called on this new redirect target, ending
> up
> > > > with
> > > > > > > > unintended URL (inside RequestHandler.doRequest()).
> > > > > > > >
> > > > > > > > I could resolve this issue by decoding the already encoded
> > value
> > > > > before
> > > > > > > > putting it into the Map and if the key is already present in
> > the
> > > > Map,
> > > > > > it
> > > > > > > > must create a list of the values.
> > > > > > > >
> > > > > > > > Am I missing something or is this really a bug and needs to
> be
> > > > > > addressed
> > > > > > > > OOTB?
> > > > > > > > If this is a bug, is proposed solution the right one?
> > > > > > > >
> > > > > > > > --
> > > > > > > > Best,
> > > > > > > > Ritesh Kumar
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
>

Reply via email to