| Summary: | fmt:bundle tag unnecessarily buffers body content | ||
|---|---|---|---|
| Product: | Taglibs | Reporter: | Dan Armstrong <support> |
| Component: | Standard Taglib | Assignee: | Tomcat Developers Mailing List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | ||
| Priority: | P2 | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Attachments: | Diff for changes to current trunk of BundleSupport.java | ||
FYI: I have confirmed this unnecessary buffering remains in the most recent 1.2 branch in SVN. Please could you attach a patch file rather than just comments. I'm a little concerned about changing the flushing/buffering behaviour but I don't see anything in the spec that precludes early flushing so am inclined to apply this. One could argue that <fmt:bundle> might actually be used to intentionally buffer content a bit, just in case an error occurs. Once the response is committed, errors occurring during rendering are usually catastrophic to the page display. How about making this a "JSP property" that is disabled by default (i.e. the current behavior is the default, but easily changed)? (In reply to Christopher Schultz from comment #3) > One could argue that <fmt:bundle> might actually be used to intentionally > buffer content a bit, just in case an error occurs. Once the response is > committed, errors occurring during rendering are usually catastrophic to the > page display. > > How about making this a "JSP property" that is disabled by default (i.e. the > current behavior is the default, but easily changed)? One can argue anything, but that doesn't make it a valid argument. This is precisely what the buffer page attribute is for. fmt:bundle implemented with an unbounded buffer is not "buffering a little". For example: <%@ page language="java" buffer="512kb" autoFlush="true" pageEncoding="UTF-8" session="false" %> Created attachment 30899 [details]
Diff for changes to current trunk of BundleSupport.java
I have attached a diff for the current trunk version.
Changed doStartTag to return EVAL_BODY_INCLUDE.
Removed doEndTag method entirely.
(In reply to Dan Armstrong from comment #4) > (In reply to Christopher Schultz from comment #3) > > One could argue that <fmt:bundle> might actually be used to intentionally > > buffer content a bit, just in case an error occurs. Once the response is > > committed, errors occurring during rendering are usually catastrophic to the > > page display. > > > > How about making this a "JSP property" that is disabled by default (i.e. the > > current behavior is the default, but easily changed)? > > One can argue anything, but that doesn't make it a valid argument. It's a fairly valid argument that changing the way a component behaves can inconvenience a lot of people. Hence my suggestion that it be made into a configurable setting, defaulting to the old behavior. Jeremy may apply your patch, but I certainly wouldn't. I don't think the page directive and its attributes really have any bearing on this, but since I generally agree with the changes, it's probably not worth going into. The JSTL 1.1 & 1.2 specs have this to say about the <fmt:bundle> body content: JSP. The JSP container processes the body content and then writes it to the current JspWriter. The action ignores the body content. The last sentence is the key and I'd claim that it actually makes more sense for BundleSupport to extend TagSupport instead of BodyTagSupport. Agreed that changing component behavior, even if the change is inline with the spec, can be problematic. Especially within a minor release in series like 1.1.2 -> 1.1.3. Consider Oracle changing Java string implementation dramatically in 1.7.0_06 as a prime example of the problems this can cause. I hope this change will make it into the 1.2 branch still under development. If there are release notes this is a good one to add to the list. Developers can understand a change during a 1.1 -> 1.2 upgrade as long as it is documented. I agree that this tag could extend TagSupport as well and achieve the same behavior. +1 for basing the decision off the specification. Thank you very much for your help. - Dan Patch applied to trunk and will be included in 1.2.0.RC2 http://svn.apache.org/r1528526 This does change the buffering characteristic of the implementation but that is an undocumented side effect. The primary use for this tag is that it "Creates an i18n localization context to be used by its body content" and the specification indicates that "the JSP container processes the body content" which is in line with this patch is doing. This change also brings this tag's behaviour in line with <c:catch> which also returns EVAL_BODY_INCLUDE. |
The fmt:bundle tag buffers its body content and then simply writes it all on doEnd. This causes both unnecessary heap space consumption, and can cause out of memory conditions on large pages where: 1) The page is surrounded by a single large <fmt:bundle ...> ... </fmt:bundle> tag. 2) The body generates a large amount of content. With buffer="X" and autoFlush="true", the JSP developer expects the body to be flushed when the buffer becomes X in size. The fmt:bundle tag has a unbounded buffer and thus this expectation is not meet. The patch for JSTL 1.1.2 is: In org/apache/taglibs/standard/tag/common/fmt/BundleSupport.java: public int doStartTag() throws JspException { locCtxt = getLocalizationContext(pageContext, basename); // Begin removed by AO Industries, Inc. // return EVAL_BODY_BUFFERED; // End removed by AO Industries, Inc. // Begin added by AO Industries, Inc. // No need to buffer our body since it is just written in full on doEndTag. return EVAL_BODY_INCLUDE; // End added by AO Industries, Inc. } public int doEndTag() throws JspException { // Begin removed by AO Industries, Inc. // if (bodyContent != null) { // try { // pageContext.getOut().print(bodyContent.getString()); // } catch (IOException ioe) { // throw new JspTagException(ioe.toString(), ioe); // } // } // End removed by AO Industries, Inc. return EVAL_PAGE; } Please include this in future releases of the taglib. It will help us work with large sets of data using the comfortable and productive JSP environment.