You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/06/23 17:19:48 UTC
[tomcat] branch main updated: Simplify - identified by SpotBugs
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new b9bd126 Simplify - identified by SpotBugs
b9bd126 is described below
commit b9bd12608d3a14ed036a1602f39b148d91fb5489
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jun 23 18:18:37 2021 +0100
Simplify - identified by SpotBugs
---
java/org/apache/jasper/runtime/PageContextImpl.java | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/java/org/apache/jasper/runtime/PageContextImpl.java b/java/org/apache/jasper/runtime/PageContextImpl.java
index f732273..1477013 100644
--- a/java/org/apache/jasper/runtime/PageContextImpl.java
+++ b/java/org/apache/jasper/runtime/PageContextImpl.java
@@ -97,8 +97,6 @@ public class PageContextImpl extends PageContext {
private transient ELContextImpl elContext;
- private boolean isIncluded;
-
// initial output stream
private transient JspWriter out;
@@ -170,26 +168,13 @@ public class PageContextImpl extends PageContext {
setAttribute(CONFIG, config);
setAttribute(PAGECONTEXT, this);
setAttribute(APPLICATION, context);
-
- isIncluded = request.getAttribute(
- RequestDispatcher.INCLUDE_SERVLET_PATH) != null;
}
@Override
public void release() {
out = baseOut;
try {
- if (isIncluded) {
- ((JspWriterImpl) out).flushBuffer();
- // push it into the including jspWriter
- } else {
- // Old code:
- // out.flush();
- // Do not flush the buffer even if we're not included (i.e.
- // we are the main page. The servlet will flush it and close
- // the stream.
- ((JspWriterImpl) out).flushBuffer();
- }
+ ((JspWriterImpl) out).flushBuffer();
} catch (IOException ex) {
IllegalStateException ise = new IllegalStateException(Localizer.getMessage("jsp.error.flush"), ex);
throw ise;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Simplify - identified by SpotBugs
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 6/24/21 03:59, Mark Thomas wrote:
> On 23/06/2021 20:06, Christopher Schultz wrote:
>> Mark,
>>
>> On 6/23/21 13:19, markt@apache.org wrote:
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> markt pushed a commit to branch main
>>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>>
>>>
>>> The following commit(s) were added to refs/heads/main by this push:
>>> new b9bd126 Simplify - identified by SpotBugs
>>> b9bd126 is described below
>>>
>>> commit b9bd12608d3a14ed036a1602f39b148d91fb5489
>>> Author: Mark Thomas <ma...@apache.org>
>>> AuthorDate: Wed Jun 23 18:18:37 2021 +0100
>>>
>>> Simplify - identified by SpotBugs > ---
>>> java/org/apache/jasper/runtime/PageContextImpl.java | 17
>>> +----------------
>>> 1 file changed, 1 insertion(+), 16 deletions(-)
>>
>> Will this not end up flushing buffers potentially needlessly early,
>> converting responses to Chunked and preventing errors from being
>> reported by committing the response?
>
> No. This flushes the JSPWriterImpl. That flushes any data buffered in
> the JSPWriterImpl to the Writer obtained from the HttpServletResponse
> but that Writer is not then flushed.
Great, thanks for the explanation.
>> I realize that THIS patch doesn't do that, but it looks like the
>> history of the file (c.f. the "Old code" comment) has two different
>> behaviors depending upon whether the file is being included.
>
> The "Old code" has been commented out for more than 15 years. You'd need
> to go back to the 5.5.x svn archives to understand what was going on here.
Glad to see it go, then.
Thanks,
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Simplify - identified by SpotBugs
Posted by Mark Thomas <ma...@apache.org>.
On 23/06/2021 20:06, Christopher Schultz wrote:
> Mark,
>
> On 6/23/21 13:19, markt@apache.org wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> markt pushed a commit to branch main
>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>
>>
>> The following commit(s) were added to refs/heads/main by this push:
>> new b9bd126 Simplify - identified by SpotBugs
>> b9bd126 is described below
>>
>> commit b9bd12608d3a14ed036a1602f39b148d91fb5489
>> Author: Mark Thomas <ma...@apache.org>
>> AuthorDate: Wed Jun 23 18:18:37 2021 +0100
>>
>> Simplify - identified by SpotBugs > ---
>> java/org/apache/jasper/runtime/PageContextImpl.java | 17
>> +----------------
>> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> Will this not end up flushing buffers potentially needlessly early,
> converting responses to Chunked and preventing errors from being
> reported by committing the response?
No. This flushes the JSPWriterImpl. That flushes any data buffered in
the JSPWriterImpl to the Writer obtained from the HttpServletResponse
but that Writer is not then flushed.
> I realize that THIS patch doesn't do that, but it looks like the history
> of the file (c.f. the "Old code" comment) has two different behaviors
> depending upon whether the file is being included.
The "Old code" has been commented out for more than 15 years. You'd need
to go back to the 5.5.x svn archives to understand what was going on here.
Mark
>
> -chris
>
>> diff --git a/java/org/apache/jasper/runtime/PageContextImpl.java
>> b/java/org/apache/jasper/runtime/PageContextImpl.java
>> index f732273..1477013 100644
>> --- a/java/org/apache/jasper/runtime/PageContextImpl.java
>> +++ b/java/org/apache/jasper/runtime/PageContextImpl.java
>> @@ -97,8 +97,6 @@ public class PageContextImpl extends PageContext {
>> private transient ELContextImpl elContext;
>> - private boolean isIncluded;
>> -
>> // initial output stream
>> private transient JspWriter out;
>> @@ -170,26 +168,13 @@ public class PageContextImpl extends PageContext {
>> setAttribute(CONFIG, config);
>> setAttribute(PAGECONTEXT, this);
>> setAttribute(APPLICATION, context);
>> -
>> - isIncluded = request.getAttribute(
>> - RequestDispatcher.INCLUDE_SERVLET_PATH) != null;
>> }
>> @Override
>> public void release() {
>> out = baseOut;
>> try {
>> - if (isIncluded) {
>> - ((JspWriterImpl) out).flushBuffer();
>> - // push it into the including jspWriter
>> - } else {
>> - // Old code:
>> - // out.flush();
>> - // Do not flush the buffer even if we're not included
>> (i.e.
>> - // we are the main page. The servlet will flush it
>> and close
>> - // the stream.
>> - ((JspWriterImpl) out).flushBuffer();
>> - }
>> + ((JspWriterImpl) out).flushBuffer();
>> } catch (IOException ex) {
>> IllegalStateException ise = new
>> IllegalStateException(Localizer.getMessage("jsp.error.flush"), ex);
>> throw ise;
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch main updated: Simplify - identified by SpotBugs
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 6/23/21 13:19, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/main by this push:
> new b9bd126 Simplify - identified by SpotBugs
> b9bd126 is described below
>
> commit b9bd12608d3a14ed036a1602f39b148d91fb5489
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Wed Jun 23 18:18:37 2021 +0100
>
> Simplify - identified by SpotBugs > ---
> java/org/apache/jasper/runtime/PageContextImpl.java | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
Will this not end up flushing buffers potentially needlessly early,
converting responses to Chunked and preventing errors from being
reported by committing the response?
I realize that THIS patch doesn't do that, but it looks like the history
of the file (c.f. the "Old code" comment) has two different behaviors
depending upon whether the file is being included.
-chris
> diff --git a/java/org/apache/jasper/runtime/PageContextImpl.java b/java/org/apache/jasper/runtime/PageContextImpl.java
> index f732273..1477013 100644
> --- a/java/org/apache/jasper/runtime/PageContextImpl.java
> +++ b/java/org/apache/jasper/runtime/PageContextImpl.java
> @@ -97,8 +97,6 @@ public class PageContextImpl extends PageContext {
>
> private transient ELContextImpl elContext;
>
> - private boolean isIncluded;
> -
>
> // initial output stream
> private transient JspWriter out;
> @@ -170,26 +168,13 @@ public class PageContextImpl extends PageContext {
> setAttribute(CONFIG, config);
> setAttribute(PAGECONTEXT, this);
> setAttribute(APPLICATION, context);
> -
> - isIncluded = request.getAttribute(
> - RequestDispatcher.INCLUDE_SERVLET_PATH) != null;
> }
>
> @Override
> public void release() {
> out = baseOut;
> try {
> - if (isIncluded) {
> - ((JspWriterImpl) out).flushBuffer();
> - // push it into the including jspWriter
> - } else {
> - // Old code:
> - // out.flush();
> - // Do not flush the buffer even if we're not included (i.e.
> - // we are the main page. The servlet will flush it and close
> - // the stream.
> - ((JspWriterImpl) out).flushBuffer();
> - }
> + ((JspWriterImpl) out).flushBuffer();
> } catch (IOException ex) {
> IllegalStateException ise = new IllegalStateException(Localizer.getMessage("jsp.error.flush"), ex);
> throw ise;
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org