CachingHandler host and port in caching key

[buzzer]

org.directwebremoting.servlet.CachingHandler uses request's contextPath, servletPath, pathInfo combination as a cache key. This leads to cross-domain related problems.

Imagine we have an application, which is hosted on the server with 2 host-ports: host1:port1, host2:port2. Once we accessed engine.js as <a href="http://host1:port1/context/dwr/engine.js">http://host1:port1/context/dwr/engine.js it will contain "host1:port1" URLs forever. E.g. this one <a href="http://host2:port2/context/dwr/engine.js">http://host2:port2/context/dwr/engine.js will contain line dwr.engine._pathToDwrServlet = "<a href="http://host1:port1/context/dwr">http://host1:port1/context/dwr"; what is not correct

[buzzer]

Btw IMO it is better to pass the whole request (instead of its certain parts) to methods like generateCachableContent(...), generateTemplate(...), getSearchReplacePairs(...) etc, thus methods will be much more flexible and easy to override.

Have you followed our cross-domain instructions?  Note steps 2 and 3 you are supposed to override paths to get it to work.
http://directwebremoting.org/dwr/documentation/browser/xdomain.html

"Btw IMO it is better to pass the whole request (instead of its certain parts)
to methods like generateCachableContent(...), generateTemplate(...),
getSearchReplacePairs(...) etc, thus methods will be much more flexible and
easy to override."

My guess is that the API was designed this way so that one could implement a CachingHandler that does not rely on having an HttpRequest. 

On Tue, Feb 7, 2012 at 9:42 AM, buzzer <[hidden email]> wrote:

Btw IMO it is better to pass the whole request (instead of its certain parts)
to methods like generateCachableContent(...), generateTemplate(...),
getSearchReplacePairs(...) etc, thus methods will be much more flexible and
easy to override.

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7262941.html

Sent from the DWR - Users mailing list archive at Nabble.com.

[buzzer]

Thanks, David, seems like points 2 and 3 should help in our situation. However this approach a bit looks like a workaround. Looks like for better design points 2 and 3 must be performed by DWR by default (until forbidden explicitly), because this is not a rear case when application is accessed by multiple URLs (each server has as minimum host name and IP address and often tests are performed for "localhost" first). It is troublesome to apply point 2 before each engine.js ref and point 3 for every interface (what if thousands of them!).
Our application has additional troubles, because we add (or not add) DWR stuff into HTML depending on condition, so we need to keep in mind point 2 and 3 for each adding.
Any chance to change this? Not wanted to override CachingHandler descendants and change DWR default configuration.


As for using request as an argument, all Handler descendants rely on HttpServletRequest, because it is an arg of handle(...) method and should be in classpath. Btw getCachingKey(...) method is already implemented in such a manner.
Maybe it is even worthwhile to add both (request and response) as args for further implementations, it would be not bad design.

Both step 2 and 3 can be skipped IF the url is known ahead of time.  Per the docs:
"If the remote URL is static/known ahead of time this step can be skipped in place of setting the overridePath parameter. "

Have you tried the overridePath parameter?

"As for using request as an argument, all Handler descendants rely on
HttpServletRequest, because it is an arg of handle(...) method and should be
in classpath. Btw getCachingKey(...) ."

Yes, you are correct.  Maybe we will change the API in the future.

On Tue, Feb 7, 2012 at 10:55 AM, buzzer <[hidden email]> wrote:

Thanks, David, seems like points 2 and 3 should help in our situation.
However this approach a bit looks like a workaround. Looks like for better
design points 2 and 3 must be performed by DWR by default (until forbidden
explicitly), because this is not a rear case when application is accessed by
multiple URLs (each server has as minimum host name and IP address and often
tests are performed for "localhost" first). It is troublesome to apply point
2 before each engine.js ref and point 3 for every interface (what if
thousands of them!).
Our application has additional troubles, because we add (or not add) DWR
stuff into HTML depending on condition, so we need to keep in mind point 2
and 3 for each adding.
Any chance to change this? Not wanted to override CachingHandler descendants
and change DWR default configuration.


As for using request as an argument, all Handler descendants rely on
HttpServletRequest, because it is an arg of handle(...) method and should be
in classpath. Btw getCachingKey(...) method is already implemented in such a
manner.
Maybe it is even worthwhile to add both (request and response) as args for
further implementations, it would be not bad design.

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263115.html

Sent from the DWR - Users mailing list archive at Nabble.com.

[buzzer]

[e-mail] wrote

Have you tried the overridePath parameter?

This will not help here, because we don't know the host name beforehand and overridePath only allows to set one host name once and forever. Generally our situation is kind of "we don't know the URL of our application". Because of many reasons:
1) Different users access the app by different URLs (because of network configuration nuances).
2) The IP and host name of the server is changed more often then application is restarted.

Ok.  Going back to your previous email:

"Looks like for better design points 2 and 3 must be performed by DWR by default (until forbidden explicitly)"

Maybe I am missing something here but If you don't know the hose name before than how do you expect DWR to?

On Tue, Feb 7, 2012 at 11:24 AM, buzzer <[hidden email]> wrote:

[hidden email] wrote

>
> Have you tried the overridePath parameter?
>

This will not help here, because we don't know the host name beforehand and
/overridePath/ only allows to set one host name once and forever. Generally
our situation is kind of "we don't know the URL of our application". Because
of many reasons:
1) Different users access the app by different URLs (because of network
configuration nuances).
2) The IP and host name of the server is changed more often then application
is restarted.

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263205.html

Sent from the DWR - Users mailing list archive at Nabble.com.

[buzzer]

We don't know it on the configuration stage (before app is started), but it can be gotten from request.
Actually some DWR methods (e.g. org.directwebremoting.impl.DefaultRemoter.getPathToDwrServlet(String) for engine.js) do this, but only once and then value is cached. The problem here is that for building URL one set of request properties is used: (scheme, port, serverName, contextPath, servletPath), but for getting the value from cache - another set: (contextPath, servletPath, pathInfo). So cached value for another schema, port and host is used if other params are equal.
Similar thing for interfaces and maybe some other stuff which inherits CachingHandler.

Btw as for arguments, all methods related to this part of DWR have String args (all of which are request properties or their combination), but request is obtained as WebContextFactory.get().getHttpServletRequest(); what is a bit confusing. So maybe it is a good idea for these methods to have a request (or request wrapper with request and some additional values) as a sole arg.

I am having a difficult time understanding exactly how you have this configured and what the purpose of this configuration is.  I originally thought you had multiple DWR instances, but based on your description it sounds like you have one DWR instance being handled by multiple host names, is that correct?  If this is true this certainly seems like a pretty bizarre use case (especially since we have never had this problem over the last 8 years or so). 

On Tue, Feb 7, 2012 at 11:55 AM, buzzer <[hidden email]> wrote:

We don't know it on the configuration stage (before app is started), but it
can be gotten from request.
Actually some DWR methods (e.g.
org.directwebremoting.impl.DefaultRemoter.getPathToDwrServlet(String) for
engine.js) do this, but only once and then value is cached. The problem here
is that for building URL one set of request properties is used: (scheme,
port, serverName, contextPath, servletPath), but for getting the value from
cache - another set: (contextPath, servletPath, pathInfo). So cached value
for another schema, port and host is used if other params are equal.
Similar thing for interfaces and maybe some other stuff which inherits
CachingHandler.

Btw as for arguments, all methods related to this part of DWR have String
args (all of which are request properties or their combination), but request
is obtained as WebContextFactory.get().getHttpServletRequest(); what is a
bit confusing. So maybe it is a good idea for these methods to have a
request (or request wrapper with request and some additional values) as a
sole arg.

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263314.html

Sent from the DWR - Users mailing list archive at Nabble.com.

Also, have you tried implementing your own CachingHandler and using the extra attributes (scheme, host, port) as a caching key?  Did that resolve all issues for you?

On Tue, Feb 7, 2012 at 12:17 PM, David Marginian <[hidden email]> wrote:

I am having a difficult time understanding exactly how you have this configured and what the purpose of this configuration is.  I originally thought you had multiple DWR instances, but based on your description it sounds like you have one DWR instance being handled by multiple host names, is that correct?  If this is true this certainly seems like a pretty bizarre use case (especially since we have never had this problem over the last 8 years or so). 

On Tue, Feb 7, 2012 at 11:55 AM, buzzer <[hidden email]> wrote:

We don't know it on the configuration stage (before app is started), but it
can be gotten from request.
Actually some DWR methods (e.g.
org.directwebremoting.impl.DefaultRemoter.getPathToDwrServlet(String) for
engine.js) do this, but only once and then value is cached. The problem here
is that for building URL one set of request properties is used: (scheme,
port, serverName, contextPath, servletPath), but for getting the value from
cache - another set: (contextPath, servletPath, pathInfo). So cached value
for another schema, port and host is used if other params are equal.
Similar thing for interfaces and maybe some other stuff which inherits
CachingHandler.

Btw as for arguments, all methods related to this part of DWR have String
args (all of which are request properties or their combination), but request
is obtained as WebContextFactory.get().getHttpServletRequest(); what is a
bit confusing. So maybe it is a good idea for these methods to have a
request (or request wrapper with request and some additional values) as a
sole arg.

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263314.html

Sent from the DWR - Users mailing list archive at Nabble.com.

[buzzer]

Yes, one instance with multiple host names. This is not bizarre, because host name configuration is out of app's control and app must not rely on the fact that it will be accessed with the same host name - this is very often not the case. The question is important especially in AJAX where scheme:host:port part is very sensitive. I'm more wondering why nobody complained before :)

Implementing own CachingHandlers is one of our workaround in mind.

I made a small code change for you.  Let me know if it helps:
http://ci.directwebremoting.org/bamboo/browse/DWRTRUNK-ALL-388/artifact

If it works I need to discuss further with Mike before it is final.

On Tue, Feb 7, 2012 at 12:37 PM, buzzer <[hidden email]> wrote:

Yes, one instance with multiple host names. This is not bizarre, because host
name configuration is out of app's control and app must not rely on the fact
that it will be accessed with the same host name - this is very often not
the case. The question is important especially in AJAX where
scheme:host:port part is very sensitive. I'm more wondering why nobody
complained before :)

Implementing own CachingHandlers is one of our workaround in mind.

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263471.html

Sent from the DWR - Users mailing list archive at Nabble.com.

[buzzer]

Not working, more likely because "localXXX" properties are used instead of "serverXXX".
Actually to avoid extra effort, I think you can reuse the code of org.directwebremoting.impl.DefaultRemoter.getPathToDwrServlet(String) method for generating key. Thus we will avoid a lot of problems, because URL is mono-semantic, while different concatenations like host + port etc may be ambiguous (e.g. "host" + "8080" = "host80" + "80").

Also making getCachingKey() method "protected" would be great to allow some overridings if necessary.

Ok give this a try in a few minutes. 
http://ci.directwebremoting.org/bamboo/browse/DWRTRUNK-ALL-389/artifact

On Tue, Feb 7, 2012 at 1:56 PM, buzzer <[hidden email]> wrote:

Not working, more likely because "localXXX" properties are used instead of
"serverXXX".
Actually to avoid extra effort, I think you can reuse the code of
org.directwebremoting.impl.DefaultRemoter.getPathToDwrServlet(String) method
for generating key. Thus we will avoid a lot of problems, because URL is
mono-semantic, while different concatenations like host + port etc may be
ambiguous (e.g. "host" + "8080" = "host80" + "80").

Also making getCachingKey() method "protected" would be great to allow some
overridings if necessary.

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263728.html

Sent from the DWR - Users mailing list archive at Nabble.com.

[buzzer]

Now it is NullPointerException, because "remoter" is not set, also we don't need the whole method, only its part, kind of this:
     protected String getCachingKey(HttpServletRequest request) {
            StringBuilder absolutePath = new StringBuilder();
            String scheme = request.getScheme();
            int port = request.getServerPort();

            absolutePath.append(scheme);
            absolutePath.append("://");
            absolutePath.append(request.getServerName());

            if (port > 0 &&
                (("http".equalsIgnoreCase(scheme) && port != 80) ||
                 ("https".equalsIgnoreCase(scheme) && port != 443)))
            {
                absolutePath.append(':');
                absolutePath.append(port);
            }

            absolutePath.append(request.getContextPath());
            absolutePath.append(request.getServletPath());
            absolutePath.append(request.getPathInfo());

            actualPath = absolutePath.toString();
            return absolutePath.toString();
    }

Sorry, I am not in a place where I can test and just trying to get something out to you quickly so you can verify that it will resolve your issue.  Try the next build in a few minutes.  I used your method.  If it works I will refactor it so we can re-use most of the existing method in defaultremoter. 

On Tue, Feb 7, 2012 at 2:47 PM, buzzer <[hidden email]> wrote:

Now it is NullPointerException, because "remoter" is not set, also we don't
need the whole method, only its part, kind of this:
    protected String getCachingKey(HttpServletRequest request) {
           StringBuilder absolutePath = new StringBuilder();
           String scheme = request.getScheme();
           int port = request.getServerPort();

           absolutePath.append(scheme);
           absolutePath.append("://");
           absolutePath.append(request.getServerName());

           if (port > 0 &&
               (("http".equalsIgnoreCase(scheme) && port != 80) ||
                ("https".equalsIgnoreCase(scheme) && port != 443)))
           {
               absolutePath.append(':');
               absolutePath.append(port);
           }

           absolutePath.append(request.getContextPath());
           absolutePath.append(request.getServletPath());
           absolutePath.append(request.getPathInfo());

           actualPath = absolutePath.toString();
           return absolutePath.toString();
   }



--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263904.html

Sent from the DWR - Users mailing list archive at Nabble.com.

Also, did you actually run it and get a null pointer or did you just look at the code?  I ask because I would expect this to be injected by our container.

On Tue, Feb 7, 2012 at 2:56 PM, David Marginian <[hidden email]> wrote:

Sorry, I am not in a place where I can test and just trying to get something out to you quickly so you can verify that it will resolve your issue.  Try the next build in a few minutes.  I used your method.  If it works I will refactor it so we can re-use most of the existing method in defaultremoter. 

On Tue, Feb 7, 2012 at 2:47 PM, buzzer <[hidden email]> wrote:

Now it is NullPointerException, because "remoter" is not set, also we don't
need the whole method, only its part, kind of this:
    protected String getCachingKey(HttpServletRequest request) {
           StringBuilder absolutePath = new StringBuilder();
           String scheme = request.getScheme();
           int port = request.getServerPort();

           absolutePath.append(scheme);
           absolutePath.append("://");
           absolutePath.append(request.getServerName());

           if (port > 0 &&
               (("http".equalsIgnoreCase(scheme) && port != 80) ||
                ("https".equalsIgnoreCase(scheme) && port != 443)))
           {
               absolutePath.append(':');
               absolutePath.append(port);
           }

           absolutePath.append(request.getContextPath());
           absolutePath.append(request.getServletPath());
           absolutePath.append(request.getPathInfo());

           actualPath = absolutePath.toString();
           return absolutePath.toString();
   }



--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263904.html

Sent from the DWR - Users mailing list archive at Nabble.com.

[buzzer]

Last version works as expected. I'll do more tests onsite and report tomorrow.

I have seen NullPointerException in debugger, but not sure what was null, because it was caught far away from that class and I haven't attached new sources (debugged by method entries breakpoints).

Ok Thanks!

On Tue, Feb 7, 2012 at 3:16 PM, buzzer <[hidden email]> wrote:

Last version works as expected. I'll do more tests onsite and report
tomorrow.

I have seen NullPointerException in debugger, but not sure what was null,
because it was caught far away from that class and I haven't attached new
sources (debugged by method entries breakpoints).

--
View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263978.html

Sent from the DWR - Users mailing list archive at Nabble.com.

Try this build I did some testing tonight and made it so DefaultRemoter
and CachingHandler share the bulk of the logic.

http://ci.directwebremoting.org/bamboo/browse/DWRTRUNK-ALL-391/artifact



On 02/07/2012 03:16 PM, buzzer wrote:

> Last version works as expected. I'll do more tests onsite and report
> tomorrow.
>
> I have seen NullPointerException in debugger, but not sure what was null,
> because it was caught far away from that class and I haven't attached new
> sources (debugged by method entries breakpoints).
>
> --
> View this message in context: http://dwr.2114559.n2.nabble.com/CachingHandler-host-and-port-in-caching-key-tp7262906p7263978.html
> Sent from the DWR - Users mailing list archive at Nabble.com.
>