On Wed, May 13, 2020 at 11:30 AM Michael Osipov <micha...@apache.org> wrote:

> Am 2020-05-13 um 11:18 schrieb Rémy Maucherat:
> > On Wed, May 13, 2020 at 11:08 AM Michael Osipov <micha...@apache.org>
> wrote:
> >
> >> Am 2020-05-13 um 09:45 schrieb r...@apache.org:
> >>> This is an automated email from the ASF dual-hosted git repository.
> >>>
> >>> remm pushed a commit to branch master
> >>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >>>
> >>>
> >>> The following commit(s) were added to refs/heads/master by this push:
> >>>        new f34dd07  Add a constant for invalid URI
> >>> f34dd07 is described below
> >>>
> >>> commit f34dd072d189183cbe152135d0a6b88e0a13315b
> >>> Author: remm <r...@apache.org>
> >>> AuthorDate: Wed May 13 09:45:03 2020 +0200
> >>>
> >>>       Add a constant for invalid URI
> >>>
> >>>       Add a space to it to make the code look cleaner.
> >>> ---
> >>>    java/org/apache/catalina/connector/CoyoteAdapter.java | 8 +++++---
> >>>    1 file changed, 5 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java
> >> b/java/org/apache/catalina/connector/CoyoteAdapter.java
> >>> index ab113a7..98a33e8 100644
> >>> --- a/java/org/apache/catalina/connector/CoyoteAdapter.java
> >>> +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
> >>> @@ -72,6 +72,8 @@ public class CoyoteAdapter implements Adapter {
> >>>                System.getProperty("java.vm.vendor") + "/" +
> >>>                System.getProperty("java.runtime.version") + ")";
> >>>
> >>> +    private static final String INVALID_URI = "Invalid URI ";
> >>> +
> >>>        private static final EnumSet<SessionTrackingMode> SSL_ONLY =
> >>>            EnumSet.of(SessionTrackingMode.SSL);
> >>>
> >>> @@ -610,7 +612,7 @@ public class CoyoteAdapter implements Adapter {
> >>>
> >>   connector.getService().getContainer().logAccess(request, response, 0,
> >> true);
> >>>                    return false;
> >>>                } else {
> >>> -                response.sendError(400, "Invalid URI");
> >>> +                response.sendError(400, INVALID_URI);
> >>>                }
> >>>            }
> >>>
> >>> @@ -628,7 +630,7 @@ public class CoyoteAdapter implements Adapter {
> >>>                try {
> >>>
> req.getURLDecoder().convert(decodedURI.getByteChunk(),
> >> connector.getEncodedSolidusHandlingInternal());
> >>>                } catch (IOException ioe) {
> >>> -                response.sendError(400, "Invalid URI: " +
> >> ioe.getMessage());
> >>> +                response.sendError(400, INVALID_URI +
> ioe.getMessage());
> >>>                }
> >>>                // Normalization
> >>>                if (normalize(req.decodedURI(),
> >> connector.getAllowBackslash())) {
> >>> @@ -638,7 +640,7 @@ public class CoyoteAdapter implements Adapter {
> >>>                    // Therefore it is not necessary to check that the
> URI
> >> remains
> >>>                    // normalized after character decoding
> >>>                } else {
> >>> -                response.sendError(400, "Invalid URI");
> >>> +                response.sendError(400, INVALID_URI);
> >>>                }
> >>>            } else {
> >>>                /* The URI is chars or String, and has been sent using
> an
> >> in-memory
> >>
> >> This change is complete: You have have a trailing 0x20 after the message
> >> and in the case of IOE#getMessage() the colon is gone. Please rework.
> >>
> >
> > What is the big issue ? This is just a string that goes in a HTML that
> > nobody ever sees [as the URI is invalid, this is not a real client,
> simply
> > a hack attempt], I preferred to simplify a bit since the extra space is
> not
> > a problem and the colon is cosmetic. I can revert this is you decide to
> > veto it.
>
> If no one ever sees why to write a message at all? Those are just wasted
> bytes, aren't they?
>

It's hard to anticipate which error messages are actually seen, so it's
better to keep them. Although I believe no actual client will see it, I
will not remove this one since the amount saved is zero. Just in case.
I am still trying to find ways to change the Connector situation and am
looking at these classes. If you don't like the change, you should veto and
I will revert no questions asked.

Rémy


>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Reply via email to