Commons Jelly
  1. Commons Jelly
  2. JELLY-177

In JellyServlet, the procedure used to determine the script's location is too simplistic

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.0-beta-5, 1.0-RC1
    • Fix Version/s: 1.0.1
    • Component/s: core / taglib.core
    • Labels:
      None
    • Environment:

      Servlet, 1.0RC1

      Description

      In JellyServlet, the procedure used to determine the script's location is too simplistic; it misses simple cases like the a *.jelly servlet-mapping.

      I suggest replacing the getScript method with something like (taken in part from the Freemarker servlet):

      protected URL getScript(HttpServletRequest req)
      throws MalformedURLException {
      String scriptUrl = null;

      String includedPath = (String) req.getAttribute("javax.servlet.include.servlet_path");
      if (includedPath != null) { //This is the result of a RequestDispatcher include...
      String includedPathInfo = (String) req.getAttribute("javax.servlet.include.path_info");
      if (includedPathInfo != null)

      { scriptUrl = includedPathInfo; }

      else

      { scriptUrl = includedPath; }

      } else {
      scriptUrl = req.getParameter("script");
      if (scriptUrl == null)

      { scriptUrl = req.getPathInfo(); }

      if (scriptUrl == null)

      { scriptUrl = req.getServletPath(); }

      }

      URL url = getServletContext().getResource(scriptUrl);
      if (url == null)

      { throw new IllegalArgumentException("Invalid script url:" + scriptUrl); }

      return url;
      }

        Activity

        Hide
        Marc DeXeT added a comment -

        Could we have some unit test about ?

        Show
        Marc DeXeT added a comment - Could we have some unit test about ?
        Hide
        Marc DeXeT added a comment -

        I use your modification for a couple of days. It's working fine indeed.
        It solves suffix context mapping problem I have encoutered by past.
        Really great stuff.
        I'm not HTTP/Servlet/JSP expert, but it's looking to be promoted.

        Show
        Marc DeXeT added a comment - I use your modification for a couple of days. It's working fine indeed. It solves suffix context mapping problem I have encoutered by past. Really great stuff. I'm not HTTP/Servlet/JSP expert, but it's looking to be promoted.
        Hide
        Denis Robert added a comment -

        Don't thank me, thank the Freemarker developers, since the technique was lifted from them.

        Show
        Denis Robert added a comment - Don't thank me, thank the Freemarker developers, since the technique was lifted from them.
        Hide
        Denis Robert added a comment -

        So as to properly render unto Gaius Julius, here's the reference to the complete source of the freemarker servlet I used as source:

        http://cvs.sourceforge.net/viewcvs.py/freemarker/freemarker/src/freemarker/ext/servlet/FreemarkerServlet.java

        Show
        Denis Robert added a comment - So as to properly render unto Gaius Julius, here's the reference to the complete source of the freemarker servlet I used as source: http://cvs.sourceforge.net/viewcvs.py/freemarker/freemarker/src/freemarker/ext/servlet/FreemarkerServlet.java
        Hide
        dion gillard added a comment -

        Seems like a good addition to the jelly servlet.

        I'm happy to include this.

        Show
        dion gillard added a comment - Seems like a good addition to the jelly servlet. I'm happy to include this.
        Hide
        Hans Gilde added a comment -

        +1

        Dion, if you don't object, I'm going to put this in now for RC2.

        Show
        Hans Gilde added a comment - +1 Dion, if you don't object, I'm going to put this in now for RC2.
        Hide
        Hans Gilde added a comment -

        Marc, could you give me an example of someting that works after this change but not before?

        Show
        Hans Gilde added a comment - Marc, could you give me an example of someting that works after this change but not before?
        Hide
        Marc DeXeT added a comment -

        hans Gilde said :
        > Marc, could you give me an example of someting that works after this change but not before?

        Sure.

        It's about url mapping.
        I have create a little jelly web library (remind me to ask to create a new subproject jelly web ) where you can create a web page as a JSP : include, forward and so on.

        An url is http://myhost:8082/demo/appli.jelly?page=browse.jelly

        URL mapping is in WEB-INF/web.xml
        ...
        <servlet>
        <servlet-name>Jelly</servlet-name>
        <servlet-class>fr.dsi.cnrs.jetty.servlet.ImprovedCachedJellyServlet</servlet-class>
        </servlet>
        ...
        <servlet-mapping>
        <servlet-name>Jelly</servlet-name>
        <url-pattern>*.jelly</url-pattern>
        </servlet-mapping>

        when such url-mapping is used, the "/appli.jelly" part of path is not in javax.servlet.http.HttpServletRequest.getPathInfo() BUT in javax.servlet.http.HttpServletRequest.getServletInfo().

        So I get the script with this last (getServletInfo).

        So this improvement (JELLY-177) enable real url-mapping to a jellyServlet.

        Do I have answered to you ?

        Waht do you think about create a jelly-web sandbox to use jelly script as an full web application script ?

        Show
        Marc DeXeT added a comment - hans Gilde said : > Marc, could you give me an example of someting that works after this change but not before? Sure. It's about url mapping. I have create a little jelly web library (remind me to ask to create a new subproject jelly web ) where you can create a web page as a JSP : include, forward and so on. An url is http://myhost:8082/demo/appli.jelly?page=browse.jelly URL mapping is in WEB-INF/web.xml ... <servlet> <servlet-name>Jelly</servlet-name> <servlet-class>fr.dsi.cnrs.jetty.servlet.ImprovedCachedJellyServlet</servlet-class> </servlet> ... <servlet-mapping> <servlet-name>Jelly</servlet-name> <url-pattern>*.jelly</url-pattern> </servlet-mapping> when such url-mapping is used, the "/appli.jelly" part of path is not in javax.servlet.http.HttpServletRequest.getPathInfo() BUT in javax.servlet.http.HttpServletRequest.getServletInfo(). So I get the script with this last (getServletInfo). So this improvement ( JELLY-177 ) enable real url-mapping to a jellyServlet. Do I have answered to you ? Waht do you think about create a jelly-web sandbox to use jelly script as an full web application script ?
        Hide
        Paul Libbrecht added a comment -

        I understand there's no unit or integration test yet for jelly core.

        This is annoying but maybe too hard to solve for now. Something such as a cargo or jetty-run plugin could help us one day.

        Marc, can you provide a full webapp that runs fine with the suggested patch and wrong without?
        It's not a unit test but is still checkable.

        thanks in advance

        paul

        Show
        Paul Libbrecht added a comment - I understand there's no unit or integration test yet for jelly core. This is annoying but maybe too hard to solve for now. Something such as a cargo or jetty-run plugin could help us one day. Marc, can you provide a full webapp that runs fine with the suggested patch and wrong without? It's not a unit test but is still checkable. thanks in advance paul

          People

          • Assignee:
            Unassigned
            Reporter:
            Denis Robert
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development