servlet-spec
  1. servlet-spec
  2. SERVLET_SPEC-51

Clarification is needed for ServletContext.addFilter(...) and ServletContext.addServlet(...)

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      Hi,

      Javadoc for ServletContext.addFilter(...)/ServletContext.addServlet(...) does not specify what should be the behaviour when filterName/servletName is NULL.

      Would you specify whether filterName/servletName is mandatory or optional and what should be the behaviour of these methods.

      From my point of view filterName/servletName should be mandatory and IllegalArgumentException should be thrown when filterName/servletName is NULL.

      Thanks in advance
      Violeta Georgieva

        Activity

        Hide
        BGehrels added a comment -

        My feelings about this issue are a little bit ambivalent.

        On the one hand side the servlet and filter names are used in other parts of the ServletContext API, for example in getServletRegistrations(). If one would make filterName/servletName optional, then the behaviour for those methods would be unclear.

        On the other hand, there is no need for a name, if you configure your whole WebApp using the addServlet/addFilter-Mechanism: The filterName is mainly usefull to reference your servlet from other locations. When using the Java API, javax.servlet.ServletRegistration.Dynamic/javax.servlet.FilterRegistration.Dynamic are used to reference your Servlet/Filter registrations. So, for those users, having to generate a unique name for each servlet and each filter is useless and may also be non-trivial when you are writing generic frameworks (for example).

        From my point of view, having a way to define anonymous filters/servlets would be a very nice thing to have. One (ugly) way to allow this would be, to allow null values hier. A better way would probably be to add

        public ServletRegistration.Dynamic addServlet(String className);
        public ServletRegistration.Dynamic addServlet(Servlet servlet);
        public ServletRegistration.Dynamic addServlet(Class <? extends Servlet> servletClass);
        public FilterRegistration.Dynamic addFilter(String className);
        public FilterRegistration.Dynamic addFilter(Filter filter);
        public FilterRegistration.Dynamic addFilter(Class <? extends Filter> filterClass);

        to the ServletContext interface.

        Show
        BGehrels added a comment - My feelings about this issue are a little bit ambivalent. On the one hand side the servlet and filter names are used in other parts of the ServletContext API, for example in getServletRegistrations(). If one would make filterName/servletName optional, then the behaviour for those methods would be unclear. On the other hand, there is no need for a name, if you configure your whole WebApp using the addServlet/addFilter-Mechanism: The filterName is mainly usefull to reference your servlet from other locations. When using the Java API, javax.servlet.ServletRegistration.Dynamic/javax.servlet.FilterRegistration.Dynamic are used to reference your Servlet/Filter registrations. So, for those users, having to generate a unique name for each servlet and each filter is useless and may also be non-trivial when you are writing generic frameworks (for example). From my point of view, having a way to define anonymous filters/servlets would be a very nice thing to have. One (ugly) way to allow this would be, to allow null values hier. A better way would probably be to add public ServletRegistration.Dynamic addServlet(String className); public ServletRegistration.Dynamic addServlet(Servlet servlet); public ServletRegistration.Dynamic addServlet(Class <? extends Servlet> servletClass); public FilterRegistration.Dynamic addFilter(String className); public FilterRegistration.Dynamic addFilter(Filter filter); public FilterRegistration.Dynamic addFilter(Class <? extends Filter> filterClass); to the ServletContext interface.
        Hide
        markt_asf added a comment - - edited

        ServletContext#getServletRegistrations() and ServletContext#getFilterRegistrations() can't work if there are multiple anonymous Servlets or multiple anonymous Filters since while Map implementations may support null keys, they don't (and can't) support multiple null keys in a single map since keys have to be unique.

        Given the above and the wording of the Javadoc for ServletContext#addFilter() and ServletContext#addServlet() I think there is a strong argument that the names are required and that the Servlet specification should be clarified make this clear. I'd be fine with adding a requirement for a IAE if a null name is provided.

        Show
        markt_asf added a comment - - edited ServletContext#getServletRegistrations() and ServletContext#getFilterRegistrations() can't work if there are multiple anonymous Servlets or multiple anonymous Filters since while Map implementations may support null keys, they don't (and can't) support multiple null keys in a single map since keys have to be unique. Given the above and the wording of the Javadoc for ServletContext#addFilter() and ServletContext#addServlet() I think there is a strong argument that the names are required and that the Servlet specification should be clarified make this clear. I'd be fine with adding a requirement for a IAE if a null name is provided.
        Hide
        Shing Wai Chan added a comment -

        I agree that we should throw IllegalArgumentException for ServletContext#addFilter and ServletContext#addServlet if null name is used.
        In no. 7 of section 14.4 of the spec, we have:
        "The element content of filter-name element must not be empty."

        In this case, I think we need to throw IllegalArgumentException if the name is "".

        Show
        Shing Wai Chan added a comment - I agree that we should throw IllegalArgumentException for ServletContext#addFilter and ServletContext#addServlet if null name is used. In no. 7 of section 14.4 of the spec, we have: "The element content of filter-name element must not be empty." In this case, I think we need to throw IllegalArgumentException if the name is "".
        Hide
        violetagg added a comment - - edited

        Hi,

        In this case, I think we need to throw IllegalArgumentException if the name is "".

        If we start returning IAE when the filterName/servetName is "" then we need clarification also for WebFilter and WebServlet.
        In the javadoc "" is default for filterName/servletName.

        javax.servlet.annotation.WebFilter

        filterName

        public abstract java.lang.String filterName
        The name of the filter
        Default:
        ""

        javax.servlet.annotation.WebServlet

        name

        public abstract java.lang.String name
        The name of the servlet
        Default:
        ""

        We can use the class name of the filter/servlet as default instead of "".
        What do you think?

        Thanks
        Violeta

        Show
        violetagg added a comment - - edited Hi, In this case, I think we need to throw IllegalArgumentException if the name is "". If we start returning IAE when the filterName/servetName is "" then we need clarification also for WebFilter and WebServlet. In the javadoc "" is default for filterName/servletName. javax.servlet.annotation.WebFilter filterName public abstract java.lang.String filterName The name of the filter Default: "" javax.servlet.annotation.WebServlet name public abstract java.lang.String name The name of the servlet Default: "" We can use the class name of the filter/servlet as default instead of "". What do you think? Thanks Violeta
        Hide
        Shing Wai Chan added a comment -

        For @WebServlet, we have
        "The default name of the Servlet if not specified is the fully qualified class name."
        Similarly for @WebFilter.
        So the name is "fully qualified class name" in this case.

        Show
        Shing Wai Chan added a comment - For @WebServlet, we have "The default name of the Servlet if not specified is the fully qualified class name." Similarly for @WebFilter. So the name is "fully qualified class name" in this case.
        Hide
        Shing Wai Chan added a comment -

        The fix has been to javadoc of ServletContext.java
        svn 57969

        Show
        Shing Wai Chan added a comment - The fix has been to javadoc of ServletContext.java svn 57969

          People

          • Assignee:
            Shing Wai Chan
            Reporter:
            violetagg
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: