ratpack / ratpack

Lean & powerful HTTP apps
https://ratpack.io
Other
1.94k stars 396 forks source link

Thymeleaf: Defect Using URL Expressions #517

Open sbonnick opened 9 years ago

sbonnick commented 9 years ago

Previously discussed here: http://forum.ratpack.io/th-href-failures-with-Thymeleaf-td808.html

Using th:href, with a constant string set in the template:

<link rel="stylesheet" type="text/css" href="css/bootstrap.min.css" th:href="@{thymeleaf/css/bootstrap.min.css}" />

Regardless of the string constant, the above fails with the following error:

org.thymeleaf.exceptions.TemplateProcessingException: Link base "thymeleaf/css/bootstrap.min.css" cannot be context relative (/) or page relative unless you implement the org.thymeleaf.context.IWebContext interface (context is of class: org.thymeleaf.context.Context) (index:7)
sbonnick commented 9 years ago

After digging deeper into the code for thymeleaf and ratpack, this issue is not as simple as originally thought. The original plan was to change Context to WebContext (in ratpack.thymeleaf.Template.java) passing in null values for the javax.servlet objects. This becomes a problem however because thymleaf assumes these are not null and throw an exception during runtime.

I think the solution here is to create the following objects and then pass them into WebContext:

javax.servlet.ServletContext;
javax.servlet.http.HttpServletRequest;
javax.servlet.http.HttpServletResponse;

This will also require adding the following package as a dependency in gradle: compile 'javax.servlet:javax.servlet-api:3.1.0'

So I would be interested in feedback from core RatPack team members on the following:

  1. I believe I read somewhere that Ratpack does not depend on any javaEE libraries. I think this is a JavaEE library. Do we have a problem with bringing this in as a dependency?
  2. Is there already a factory of sorts to create the request object? Or is there a factory/object which is similar I can use to populate the request object? Here is what the request will need to provide: http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest.html
rhart commented 9 years ago

In answer to 1 you're right, we don't have a dependency on JEE libraries. From Luke's forum comment "we can look at providing implementations of those Servlet API type in so far makes sense" I don't think adding this dependency to the Thymeleaf module would be an issue but prob best to wait for @alkemist to comment. He's not around much this week though..

In the meantime it would be worth finding out exactly how these servlet classes are used by Thymeleaf. Does it just expect them to be not null or are any of their properties used? And what for? It would be worth a better understanding here I think so we know how far we have to go or if there's anything reqiured that we can't create.

sbonnick commented 9 years ago

So best I can tell, For ServletRequest we will need to implement based on the following usages (and downstream usages) of the ServletRequest:

WebContext.java
114:    Validate.notNull(request, "Request cannot be null");

WebRequestParamsVariablesMap.java
57:    Validate.notNull(request, "Request cannot be null");
58:    this.parameterMap = request.getParameterMap();

WebSessionVariablesMap.java
67:    final HttpSession session = this.request.getSession(false);
72:          final Enumeration<String> attributeNames = session.getAttributeNames();
96:          return session.getAttribute((String)key);  
133:        session.setAttribute(key, value);
147:        session.setAttribute(mEntry.getKey(), mEntry.getValue());
161:        session.removeAttribute((String)key);
181:        session.removeAttribute(attributeName);
276:        result = prime * result + session.hashCode();

LinkExpression.java
360:     url = request.getContextPath() + linkBase + parametersBuilder + urlFragment;

So it seems we need to support:

  1. getSession() -> and CRUD operations in the session
  2. getParameterMap()
  3. getContextPath()

For ServletResponse I think can be null based on the only point of access:

LinkExpression.java
378:    return (response != null? response.encodeURL(url) : url);
response 

For ServletContext I think we also need to implement based on the following:

ServletContextDocTypeResolutionEntry.java
50:    Validate.notNull(servletContext, "Servlet context cannot be null");

ServletContextResourceResolver.java
87:    return servletContext.getResourceAsStream(resourceName);

WebServletContextVariablesMap.java
58:      Validate.notNull(servletContext, "Servlet context cannot be null");
81:      return !this.servletContext.getAttributeNames().hasMoreElements();
88:      return this.servletContext.getAttribute((String)key);
96:      final Enumeration<String> attributeNames = this.servletContext.getAttributeNames();
116:    this.servletContext.setAttribute(key, value);
125:    this.servletContext.setAttribute(mEntry.getKey(), mEntry.getValue());
134:    this.servletContext.removeAttribute((String)key);
150:    this.servletContext.removeAttribute(attributeName);

So it seems we need to support:

  1. CRUD operations on servletContext
sbonnick commented 9 years ago

I'm thinking we may want to think about having a factory somewhere that any guice supported module can call to generate these objects based on the netty/ratpack request/context/response data. That way other servlet based libraries can be easily plugged in, in the future too. Alot more work though then just querying the data and building the objects within the context of the thymeleaf module.

ldaley commented 9 years ago

I believe I read somewhere that Ratpack does not depend on any javaEE libraries. I think this is a JavaEE library. Do we have a problem with bringing this in as a dependency?

It's perfectly fine for the Thymeleaf module to depend on the Servlet API.

Is there already a factory of sorts to create the request object?

No, we haven't needed to bridge to the servlet types before.

So it seems we need to support:

  1. getSession() -> and CRUD operations in the session
  2. getParameterMap()
  3. getContextPath()

I don't think we need to support writing to the Session. The view should not be doing this. The rest should be reasonably easy, but let's see.

I'm thinking we may want to think about having a factory somewhere that any guice supported module can call to generate these objects based on the netty/ratpack request/context/response data.

This probably makes sense, but we can do it as a second step. It may make sense to extract the bridging to a ratpack-servlet-api type library. Let's see how successful we are with implementing them though first.

todd1251 commented 9 years ago

I tried the approach suggested by @sbonnick and it prevents the TemplateProcessingException. Almost everything can just throw UnsupportedOperationException with a few trivial implementations needed (context path as empty string i.e. root, request attributes as a map, encode URL as a no-op - but that might be wrong). Surely it is still broken in many ways...

A robust set of adapters to integrate Ratpack with any Servlet API based library would certainly be nice to have. That said, it does seem like a limitation in Thymeleaf to assume that the only possible web context is the servlet API, given it doesn't seem to use very much of it (alas exposing the request/response objects to the view).

Is it worth first putting something incomplete in now? Then at least the @{...} syntax works when using Thymeleaf. Happy to create a PR but not able to build out the entire servlet API... is it okay to leave a bunch of methods unimplemented? ;-)

ldaley commented 9 years ago

This would be very welcome. If it only solves this particular issue then that's valuable. Leaving methods unimplemented is no problem.

danielfernandez commented 8 years ago

Just for the record, Thymeleaf 3.0 will ease this integration by means of its new ILinkBuilder extension point, which will allow building URLs at @{...} link expressions –even context relative URLs like @{/product/list}– without any kind of dependency on the Java Servlet API. See thymeleaf/thymeleaf#458