Re: [Exist-development] Possible very serious concurrency issue with XQueryURLRewrite

[Casey J. <cas...@jo...>]

Ok, so I have finally had a moment to get back to this issue. I took some
time and was able to create a test that reproduced this very easily
(Basically a very specific stress test).

>From that I believe that I have found the issue.

URLRewrite.copyFrom(URLRewrite other) is not safe, if you look at the
method:

protected void copyFrom(URLRewrite other) {
        this.headers = other.headers;
        this.parameters = other.parameters;
        this.attributes = other.attributes;
    }

You can see it does not copy the contents of  the mappings for params,
attributes etc but instead maintains a reference to the existing
objects. This means that for static rewrites in high concurrency data will
absolutely get overwritten.

This method is only used in one place (XQueryURLRewrite.doRewrite()).

I believe that if this can be fixed it will correct this very serious
issue. That being said, I am not sure why it was designed this way to begin
with so I would appreciate some input from the team.

Thanks!




On Sat, Mar 29, 2014 at 7:33 PM, Casey Jordan <cas...@jo...>
wrote:

> I think I have found what I believe to be a very serious concurrency issue
> in the XQueryURLRewrite code (eXist develop). This has been very hard to
> track down so please bare with me as I describe what I have seen so far.
>
> I was noticing that every so often when there were several simultaneous
> requests to my application the result from one request would return the
> wrong result. Specifically, it would return the same result as another
> request that happened at a similar time.
>
> I started to debug this and found that the mixup happens with a parameter
> (via <add-parameter/>) somewhere in the rewrite code. I tried to create a
> reproducible test for this, but I can only seem to recreate it every so
> often and only with my production code, which is obviously much more
> complex than the simple tests cases I was creating.
>
> So I started doing a code review on the rewrite servlet, but after a few
> hours I decided it might be a good idea to ask the community.
>
> My controller is located at /api/get/controller.xql and  the xquery it
> gets forwarded to is located at /api/get/document.xql
>
> My request calls look like this:
> http://localhost:8080/api/get/[document-uri]?view=web
>
> For instance:
> http://localhost:8080/api/get/db/documents/langref/preface.dita?view=web
>
> My controllers role is to parse out the document-uri from this string and
> turn it into a request param forwarded to document.xql. This is essentially
> what my controller.xql looks like.
>
> let $path := replace(replace(substring-after(request:get-url(), 'get'),
> "//", "/"), ' ', '%20')
>
> let $out := util:log-system-out(concat("RW: ", $path))
>
> <dispatch xmlns="http://exist.sourceforge.net/NS/exist">
>
>  <forward url="/api/get/document.xql" >
>
>   <add-parameter name="path" value="{$path}"/>
>
>  </forward>
>
>  </dispatch>
>
> and in document.xql I get the param I set like so:
>
> let $path := request:get-parameter('path', '')
>
> let $out := util:log-system-out(concat("Compiling: ",$path))
>
> ....
>
> (Note the bolded debug messages in the code above as I will reference them
> later)
>
> Now I have noticed that every so often, the "path" parameter which comes
> into the request gets replaced with a different one from another request.
>
> I know this because I can look at the jetty request logs and I can see a
> request come into the server, then I can see the debug output in my
> controller, that shows that $path is parsed correctly, but then in my
> document.xql I never see this path get printed in the debug statement.
> Instead I see two debug statements with the "path" for a different request.
>
> Consider the following debug output:
>
> (Line: 18 /api/get/controller.xql) RW:
> /db/documents/archSpec/dita_technicalContent_InformationTypes.dita
>
> (Line: 18 /api/get/controller.xql) RW:
> /db/documents/archSpec/dita_concept_topic.dita
>
> (Line: 18 /api/get/controller.xql) RW:
> /db/documents/archSpec/dita_generic_task_topic.dita
>
> (Line: 18 /api/get/controller.xql) RW:
> /db/documents/archSpec/dita_task_topic.dita
>
> (Line: 18 /api/get/controller.xql) RW:
> /db/documents/archSpec/dita_reference_topic.dita
>
> (Line: 40 /api/get/document.xql) Compiling:
> /db/documents/archSpec/dita_task_topic.dita
>
> (Line: 40 /api/get/document.xql) Compiling:
> /db/documents/archSpec/dita_technicalContent_InformationTypes.dita
>
> (Line: 40 /api/get/document.xql) Compiling:
> /db/documents/archSpec/dita_task_topic.dita
>
> You can see here that the call for dita_generic_task_topic.dita comes
> into the controller, but never makes it into document.xql. Instead, there
> are two calls where dita_task_topic.dita is the "path" param.
>
> So this leads me to believe that there is a very serious issue where some
> data is being shared between request threads. My code review lead me to
> believe this is related  to the RewriteConfig because it keeps a set of
> mappings in memory, and while the methods to get these mappings are
> synchronized, it seems that they can return a URLRewrite instance (In this
> case PathForward) that is shared between multiple threads, and thus when
> the controller result is parsed can overwrite parameters stored in the
> URLRewrite instance.
>
> I am hoping that someone who is more familiar with this code can take a
> look for me. Obviously if params are indeed being overwritten then this
> could inadvertently cause all kinds of problems in applications that may
> delete or alter data.
>
> Thanks!
>
>
> --
> --
> Casey Jordan
> easyDITA a product of Jorsek LLC
> "CaseyDJordan" on LinkedIn, Twitter & Facebook
> (585) 348 7399
> easydita.com
>
>
> This message is intended only for the use of the Addressee(s) and may
> contain information that is privileged, confidential, and/or exempt from
> disclosure under applicable law.  If you are not the intended recipient,
> please be advised that any disclosure  copying, distribution, or use of
> the information contained herein is prohibited.  If you have received
> this communication in error, please destroy all copies of the message,
> whether in electronic or hard copy format, as well as attachments, and
> immediately contact the sender by replying to this e-mail or by phone.
> Thank you.
>



-- 
--
Casey Jordan
easyDITA a product of Jorsek LLC
"CaseyDJordan" on LinkedIn, Twitter & Facebook
(585) 348 7399
easydita.com


This message is intended only for the use of the Addressee(s) and may
contain information that is privileged, confidential, and/or exempt from
disclosure under applicable law.  If you are not the intended recipient,
please be advised that any disclosure  copying, distribution, or use of
the information contained herein is prohibited.  If you have received
this communication in error, please destroy all copies of the message,
whether in electronic or hard copy format, as well as attachments, and
immediately contact the sender by replying to this e-mail or by phone.
Thank you.