Re: Issue with opening a bookmarked page when the user is logged out

[Michael Brohl]

OFBiz handles multiple request parameters with the same name well if you use POST requests.

For GET requests, there seem to be no standard for multiple parameters with the same name, see [1]. I thinks it would be good to investigate further and see if we can fix this. I don't see a problem with REST here, the data will most probably transferred in some kind of JSON structure. [1] https://stackoverflow.com/questions/24059773/correct-way-to-pass-multiple-values-for-same-parameter-name-in-get-request#24728298

Regards,

Michael


Am 26.08.18 um 07:09 schrieb Taher Alkhateeb:

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

smime.p7s
Description: S/MIME Cryptographic Signature