You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by mi...@apache.org on 2020/04/18 19:59:57 UTC
[tomcat] branch 7.0.x updated: Remove redundant sole path/URI from
error page message on SC_NOT_FOUND
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/7.0.x by this push:
new 08879ae Remove redundant sole path/URI from error page message on SC_NOT_FOUND
08879ae is described below
commit 08879aeb5e30933bc0a6aaea6c1fa8a9ef4b8a58
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Sat Apr 18 20:58:40 2020 +0200
Remove redundant sole path/URI from error page message on SC_NOT_FOUND
When a component issues a SC_NOT_FOUND don't respond with the path/URI only in
the error message because it does not offer any more detail about the error,
plus the client knows the path/URI already.
---
java/org/apache/catalina/servlets/DefaultServlet.java | 5 ++---
java/org/apache/catalina/servlets/WebdavServlet.java | 2 +-
java/org/apache/catalina/ssi/SSIServlet.java | 6 +++---
java/org/apache/jasper/servlet/JspServlet.java | 3 +--
webapps/docs/changelog.xml | 10 +++++++++-
5 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java
index bb96786..a959844 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -849,7 +849,7 @@ public class DefaultServlet extends HttpServlet {
response.sendError(((Integer) request.getAttribute(
RequestDispatcher.ERROR_STATUS_CODE)).intValue());
} else {
- response.sendError(HttpServletResponse.SC_NOT_FOUND, requestUri);
+ response.sendError(HttpServletResponse.SC_NOT_FOUND);
}
return;
}
@@ -885,8 +885,7 @@ public class DefaultServlet extends HttpServlet {
// Skip directory listings if we have been configured to
// suppress them
if (!listings) {
- response.sendError(HttpServletResponse.SC_NOT_FOUND,
- request.getRequestURI());
+ response.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}
contentType = "text/html;charset=UTF-8";
diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java b/java/org/apache/catalina/servlets/WebdavServlet.java
index 8213c0f..4d0f8d4 100644
--- a/java/org/apache/catalina/servlets/WebdavServlet.java
+++ b/java/org/apache/catalina/servlets/WebdavServlet.java
@@ -656,7 +656,7 @@ public class WebdavServlet extends DefaultServlet {
}
if (!exists) {
- resp.sendError(HttpServletResponse.SC_NOT_FOUND, path);
+ resp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}
diff --git a/java/org/apache/catalina/ssi/SSIServlet.java b/java/org/apache/catalina/ssi/SSIServlet.java
index cee6202..a5a6d64 100644
--- a/java/org/apache/catalina/ssi/SSIServlet.java
+++ b/java/org/apache/catalina/ssi/SSIServlet.java
@@ -156,13 +156,13 @@ public class SSIServlet extends HttpServlet {
// (the "toUpperCase()" avoids problems on Windows systems)
if (path == null || path.toUpperCase(Locale.ENGLISH).startsWith("/WEB-INF")
|| path.toUpperCase(Locale.ENGLISH).startsWith("/META-INF")) {
- res.sendError(HttpServletResponse.SC_NOT_FOUND, path);
+ res.sendError(HttpServletResponse.SC_NOT_FOUND);
log("Can't serve file: " + path);
return;
}
URL resource = servletContext.getResource(path);
if (resource == null) {
- res.sendError(HttpServletResponse.SC_NOT_FOUND, path);
+ res.sendError(HttpServletResponse.SC_NOT_FOUND);
log("Can't find file: " + path);
return;
}
@@ -222,4 +222,4 @@ public class SSIServlet extends HttpServlet {
}
bufferedReader.close();
}
-}
\ No newline at end of file
+}
diff --git a/java/org/apache/jasper/servlet/JspServlet.java b/java/org/apache/jasper/servlet/JspServlet.java
index 79cc710..0db97ed 100644
--- a/java/org/apache/jasper/servlet/JspServlet.java
+++ b/java/org/apache/jasper/servlet/JspServlet.java
@@ -417,8 +417,7 @@ public class JspServlet extends HttpServlet implements PeriodicEventListener {
throw new ServletException(SecurityUtil.filter(msg));
} else {
try {
- response.sendError(HttpServletResponse.SC_NOT_FOUND,
- request.getRequestURI());
+ response.sendError(HttpServletResponse.SC_NOT_FOUND);
} catch (IllegalStateException ise) {
log.error(Localizer.getMessage("jsp.error.file.not.found",
jspUri));
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index fbfae60..2d98d99 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -59,7 +59,7 @@
They eventually become mixed with the numbered issues (i.e., numbered
issues do not "pop up" wrt. others).
-->
-<section name="Tomcat 7.0.104 (violetagg)">
+<section name="Tomcat 7.0.104 (violetagg)" rtext="in development">
<subsection name="Catalina">
<changelog>
<add>
@@ -85,6 +85,10 @@
that use a custom class loader that loads resources from non-standard
locations. (markt)
</fix>
+ <update>
+ Remove redundant sole path/URI from error page message on SC_NOT_FOUND.
+ (michaelo)
+ </update>
</changelog>
</subsection>
<subsection name="Coyote">
@@ -122,6 +126,10 @@
does not support these values, a warning will be logged and the latest
supported version will used. (markt)
</add>
+ <update>
+ Remove redundant sole path/URI from error page message on SC_NOT_FOUND.
+ (michaelo)
+ </update>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 7.0.x updated: Remove redundant sole path/URI
from error page message on SC_NOT_FOUND
Posted by Michael Osipov <mi...@apache.org>.
Am 2020-04-20 um 11:11 schrieb Rémy Maucherat:
> On Mon, Apr 20, 2020 at 10:58 AM Michael Osipov <mi...@apache.org> wrote:
>
>> Am 2020-04-20 um 10:05 schrieb Mark Thomas:
>>> On 18/04/2020 20:59, michaelo@apache.org wrote:
>>>> This is an automated email from the ASF dual-hosted git repository.
>>>>
>>>> michaelo pushed a commit to branch 7.0.x
>>>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>>>
>>>>
>>>> The following commit(s) were added to refs/heads/7.0.x by this push:
>>>> new 08879ae Remove redundant sole path/URI from error page
>> message on SC_NOT_FOUND
>>>> 08879ae is described below
>>>>
>>>> commit 08879aeb5e30933bc0a6aaea6c1fa8a9ef4b8a58
>>>> Author: Michael Osipov <mi...@apache.org>
>>>> AuthorDate: Sat Apr 18 20:58:40 2020 +0200
>>>>
>>>> Remove redundant sole path/URI from error page message on
>> SC_NOT_FOUND
>>>>
>>>> When a component issues a SC_NOT_FOUND don't respond with the
>> path/URI only in
>>>> the error message because it does not offer any more detail about
>> the error,
>>>> plus the client knows the path/URI already.
>>>
>>> Not necessarily. The client has no visibility of forwards or includes.
>>> In a more complex application it might not be obvious that a forward /
>>> include has occurred. Including the path for the resource that could not
>>> be found makes debugging significantly easier.
>>
>> For the dev likely, for the user still irrelevant. What about logging at
>> debug level for the dev to still be able to find issues (like in
>> SSIServlet)? At the end, only the dev cares about more context. WDYT?
>>
>
> I would also prefer keeping them. This way users can report a more useful
> error.
What about a more descriptive message rather than the blunt path/URL?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 7.0.x updated: Remove redundant sole path/URI
from error page message on SC_NOT_FOUND
Posted by Rémy Maucherat <re...@apache.org>.
On Mon, Apr 20, 2020 at 10:58 AM Michael Osipov <mi...@apache.org> wrote:
> Am 2020-04-20 um 10:05 schrieb Mark Thomas:
> > On 18/04/2020 20:59, michaelo@apache.org wrote:
> >> This is an automated email from the ASF dual-hosted git repository.
> >>
> >> michaelo pushed a commit to branch 7.0.x
> >> in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >>
> >>
> >> The following commit(s) were added to refs/heads/7.0.x by this push:
> >> new 08879ae Remove redundant sole path/URI from error page
> message on SC_NOT_FOUND
> >> 08879ae is described below
> >>
> >> commit 08879aeb5e30933bc0a6aaea6c1fa8a9ef4b8a58
> >> Author: Michael Osipov <mi...@apache.org>
> >> AuthorDate: Sat Apr 18 20:58:40 2020 +0200
> >>
> >> Remove redundant sole path/URI from error page message on
> SC_NOT_FOUND
> >>
> >> When a component issues a SC_NOT_FOUND don't respond with the
> path/URI only in
> >> the error message because it does not offer any more detail about
> the error,
> >> plus the client knows the path/URI already.
> >
> > Not necessarily. The client has no visibility of forwards or includes.
> > In a more complex application it might not be obvious that a forward /
> > include has occurred. Including the path for the resource that could not
> > be found makes debugging significantly easier.
>
> For the dev likely, for the user still irrelevant. What about logging at
> debug level for the dev to still be able to find issues (like in
> SSIServlet)? At the end, only the dev cares about more context. WDYT?
>
I would also prefer keeping them. This way users can report a more useful
error.
Rémy
>
> M
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
Re: [tomcat] branch 7.0.x updated: Remove redundant sole path/URI
from error page message on SC_NOT_FOUND
Posted by Michael Osipov <mi...@apache.org>.
Am 2020-04-20 um 10:05 schrieb Mark Thomas:
> On 18/04/2020 20:59, michaelo@apache.org wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> michaelo pushed a commit to branch 7.0.x
>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>
>>
>> The following commit(s) were added to refs/heads/7.0.x by this push:
>> new 08879ae Remove redundant sole path/URI from error page message on SC_NOT_FOUND
>> 08879ae is described below
>>
>> commit 08879aeb5e30933bc0a6aaea6c1fa8a9ef4b8a58
>> Author: Michael Osipov <mi...@apache.org>
>> AuthorDate: Sat Apr 18 20:58:40 2020 +0200
>>
>> Remove redundant sole path/URI from error page message on SC_NOT_FOUND
>>
>> When a component issues a SC_NOT_FOUND don't respond with the path/URI only in
>> the error message because it does not offer any more detail about the error,
>> plus the client knows the path/URI already.
>
> Not necessarily. The client has no visibility of forwards or includes.
> In a more complex application it might not be obvious that a forward /
> include has occurred. Including the path for the resource that could not
> be found makes debugging significantly easier.
For the dev likely, for the user still irrelevant. What about logging at
debug level for the dev to still be able to find issues (like in
SSIServlet)? At the end, only the dev cares about more context. WDYT?
M
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 7.0.x updated: Remove redundant sole path/URI
from error page message on SC_NOT_FOUND
Posted by Michael Osipov <mi...@apache.org>.
Am 2020-04-20 um 15:11 schrieb Konstantin Kolinko:
> пн, 20 апр. 2020 г. в 11:05, Mark Thomas <ma...@apache.org>:
>>
>> On 18/04/2020 20:59, michaelo@apache.org wrote:
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> michaelo pushed a commit to branch 7.0.x
>>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>>
>>>
>>> The following commit(s) were added to refs/heads/7.0.x by this push:
>>> new 08879ae Remove redundant sole path/URI from error page message on SC_NOT_FOUND
>>> 08879ae is described below
>>>
>>> commit 08879aeb5e30933bc0a6aaea6c1fa8a9ef4b8a58
>>> Author: Michael Osipov <mi...@apache.org>
>>> AuthorDate: Sat Apr 18 20:58:40 2020 +0200
>>>
>>> Remove redundant sole path/URI from error page message on SC_NOT_FOUND
>>>
>>> When a component issues a SC_NOT_FOUND don't respond with the path/URI only in
>>> the error message because it does not offer any more detail about the error,
>>> plus the client knows the path/URI already.
>>
>> Not necessarily. The client has no visibility of forwards or includes.
>> In a more complex application it might not be obvious that a forward /
>> include has occurred. Including the path for the resource that could not
>> be found makes debugging significantly easier.
>>
>> I think this change should be reverted.
>
> I am -1 on this change. I agree that it should be reverted.
>
> I have seen this message from DefaultServlet many times and it is rather useful.
> If a programmer wants to hide the error path, the widely used practice
> is to configure a custom error page for the error code 404.
>
> I cannot comment on other servlets (WebdavServlet, SSIServlet,
> JSPServlet) touched by this commit.
this will also apply to JSP servlet when you ask the request dispatcher
to forward to a JSP page.
I will work on decent messages for DefaultServlet and JSPServlet
tomorrow. The WebDavServlet makes no real sense because the responses
are for automated clients. They will happily ignore the HTML error page.
M
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 7.0.x updated: Remove redundant sole path/URI
from error page message on SC_NOT_FOUND
Posted by Konstantin Kolinko <kn...@gmail.com>.
пн, 20 апр. 2020 г. в 11:05, Mark Thomas <ma...@apache.org>:
>
> On 18/04/2020 20:59, michaelo@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > michaelo pushed a commit to branch 7.0.x
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/7.0.x by this push:
> > new 08879ae Remove redundant sole path/URI from error page message on SC_NOT_FOUND
> > 08879ae is described below
> >
> > commit 08879aeb5e30933bc0a6aaea6c1fa8a9ef4b8a58
> > Author: Michael Osipov <mi...@apache.org>
> > AuthorDate: Sat Apr 18 20:58:40 2020 +0200
> >
> > Remove redundant sole path/URI from error page message on SC_NOT_FOUND
> >
> > When a component issues a SC_NOT_FOUND don't respond with the path/URI only in
> > the error message because it does not offer any more detail about the error,
> > plus the client knows the path/URI already.
>
> Not necessarily. The client has no visibility of forwards or includes.
> In a more complex application it might not be obvious that a forward /
> include has occurred. Including the path for the resource that could not
> be found makes debugging significantly easier.
>
> I think this change should be reverted.
I am -1 on this change. I agree that it should be reverted.
I have seen this message from DefaultServlet many times and it is rather useful.
If a programmer wants to hide the error path, the widely used practice
is to configure a custom error page for the error code 404.
I cannot comment on other servlets (WebdavServlet, SSIServlet,
JSPServlet) touched by this commit.
BTW, see changelog part of this commit:
it adds the same change twice. A remainder from a broken merge?
Best regards,
Konstantin Kolinko
> > ---
> > java/org/apache/catalina/servlets/DefaultServlet.java | 5 ++---
> > java/org/apache/catalina/servlets/WebdavServlet.java | 2 +-
> > java/org/apache/catalina/ssi/SSIServlet.java | 6 +++---
> > java/org/apache/jasper/servlet/JspServlet.java | 3 +--
> > webapps/docs/changelog.xml | 10 +++++++++-
> > 5 files changed, 16 insertions(+), 10 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch 7.0.x updated: Remove redundant sole path/URI
from error page message on SC_NOT_FOUND
Posted by Mark Thomas <ma...@apache.org>.
On 18/04/2020 20:59, michaelo@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> michaelo pushed a commit to branch 7.0.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/7.0.x by this push:
> new 08879ae Remove redundant sole path/URI from error page message on SC_NOT_FOUND
> 08879ae is described below
>
> commit 08879aeb5e30933bc0a6aaea6c1fa8a9ef4b8a58
> Author: Michael Osipov <mi...@apache.org>
> AuthorDate: Sat Apr 18 20:58:40 2020 +0200
>
> Remove redundant sole path/URI from error page message on SC_NOT_FOUND
>
> When a component issues a SC_NOT_FOUND don't respond with the path/URI only in
> the error message because it does not offer any more detail about the error,
> plus the client knows the path/URI already.
Not necessarily. The client has no visibility of forwards or includes.
In a more complex application it might not be obvious that a forward /
include has occurred. Including the path for the resource that could not
be found makes debugging significantly easier.
I think this change should be reverted.
Mark
> ---
> java/org/apache/catalina/servlets/DefaultServlet.java | 5 ++---
> java/org/apache/catalina/servlets/WebdavServlet.java | 2 +-
> java/org/apache/catalina/ssi/SSIServlet.java | 6 +++---
> java/org/apache/jasper/servlet/JspServlet.java | 3 +--
> webapps/docs/changelog.xml | 10 +++++++++-
> 5 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java
> index bb96786..a959844 100644
> --- a/java/org/apache/catalina/servlets/DefaultServlet.java
> +++ b/java/org/apache/catalina/servlets/DefaultServlet.java
> @@ -849,7 +849,7 @@ public class DefaultServlet extends HttpServlet {
> response.sendError(((Integer) request.getAttribute(
> RequestDispatcher.ERROR_STATUS_CODE)).intValue());
> } else {
> - response.sendError(HttpServletResponse.SC_NOT_FOUND, requestUri);
> + response.sendError(HttpServletResponse.SC_NOT_FOUND);
> }
> return;
> }
> @@ -885,8 +885,7 @@ public class DefaultServlet extends HttpServlet {
> // Skip directory listings if we have been configured to
> // suppress them
> if (!listings) {
> - response.sendError(HttpServletResponse.SC_NOT_FOUND,
> - request.getRequestURI());
> + response.sendError(HttpServletResponse.SC_NOT_FOUND);
> return;
> }
> contentType = "text/html;charset=UTF-8";
> diff --git a/java/org/apache/catalina/servlets/WebdavServlet.java b/java/org/apache/catalina/servlets/WebdavServlet.java
> index 8213c0f..4d0f8d4 100644
> --- a/java/org/apache/catalina/servlets/WebdavServlet.java
> +++ b/java/org/apache/catalina/servlets/WebdavServlet.java
> @@ -656,7 +656,7 @@ public class WebdavServlet extends DefaultServlet {
> }
>
> if (!exists) {
> - resp.sendError(HttpServletResponse.SC_NOT_FOUND, path);
> + resp.sendError(HttpServletResponse.SC_NOT_FOUND);
> return;
> }
>
> diff --git a/java/org/apache/catalina/ssi/SSIServlet.java b/java/org/apache/catalina/ssi/SSIServlet.java
> index cee6202..a5a6d64 100644
> --- a/java/org/apache/catalina/ssi/SSIServlet.java
> +++ b/java/org/apache/catalina/ssi/SSIServlet.java
> @@ -156,13 +156,13 @@ public class SSIServlet extends HttpServlet {
> // (the "toUpperCase()" avoids problems on Windows systems)
> if (path == null || path.toUpperCase(Locale.ENGLISH).startsWith("/WEB-INF")
> || path.toUpperCase(Locale.ENGLISH).startsWith("/META-INF")) {
> - res.sendError(HttpServletResponse.SC_NOT_FOUND, path);
> + res.sendError(HttpServletResponse.SC_NOT_FOUND);
> log("Can't serve file: " + path);
> return;
> }
> URL resource = servletContext.getResource(path);
> if (resource == null) {
> - res.sendError(HttpServletResponse.SC_NOT_FOUND, path);
> + res.sendError(HttpServletResponse.SC_NOT_FOUND);
> log("Can't find file: " + path);
> return;
> }
> @@ -222,4 +222,4 @@ public class SSIServlet extends HttpServlet {
> }
> bufferedReader.close();
> }
> -}
> \ No newline at end of file
> +}
> diff --git a/java/org/apache/jasper/servlet/JspServlet.java b/java/org/apache/jasper/servlet/JspServlet.java
> index 79cc710..0db97ed 100644
> --- a/java/org/apache/jasper/servlet/JspServlet.java
> +++ b/java/org/apache/jasper/servlet/JspServlet.java
> @@ -417,8 +417,7 @@ public class JspServlet extends HttpServlet implements PeriodicEventListener {
> throw new ServletException(SecurityUtil.filter(msg));
> } else {
> try {
> - response.sendError(HttpServletResponse.SC_NOT_FOUND,
> - request.getRequestURI());
> + response.sendError(HttpServletResponse.SC_NOT_FOUND);
> } catch (IllegalStateException ise) {
> log.error(Localizer.getMessage("jsp.error.file.not.found",
> jspUri));
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index fbfae60..2d98d99 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -59,7 +59,7 @@
> They eventually become mixed with the numbered issues (i.e., numbered
> issues do not "pop up" wrt. others).
> -->
> -<section name="Tomcat 7.0.104 (violetagg)">
> +<section name="Tomcat 7.0.104 (violetagg)" rtext="in development">
> <subsection name="Catalina">
> <changelog>
> <add>
> @@ -85,6 +85,10 @@
> that use a custom class loader that loads resources from non-standard
> locations. (markt)
> </fix>
> + <update>
> + Remove redundant sole path/URI from error page message on SC_NOT_FOUND.
> + (michaelo)
> + </update>
> </changelog>
> </subsection>
> <subsection name="Coyote">
> @@ -122,6 +126,10 @@
> does not support these values, a warning will be logged and the latest
> supported version will used. (markt)
> </add>
> + <update>
> + Remove redundant sole path/URI from error page message on SC_NOT_FOUND.
> + (michaelo)
> + </update>
> </changelog>
> </subsection>
> <subsection name="Cluster">
>
>
> ---------------------------------------------------------------------
> 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