https://issues.apache.org/bugzilla/show_bug.cgi?id=53411

Konstantin Kolinko <knst.koli...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |---
           Severity|normal                      |enhancement

--- Comment #5 from Konstantin Kolinko <knst.koli...@gmail.com> ---
Reopening this as enhancement.

Looking at AbstractHttp11Processor#process() this error is correctly written to
access log, etc, so I do not see much concern.

Still I think there is a room for improvement, and I noted several minor issues
from my code review.


1) in MapperListener#findDefaultHost()

First, maybe 
s/log.warn(sm.getString("mapperListener.unknownDefaultHost",/log.error/ 

Second, maybe mention something like " Tomcat will not be able to process
HTTP/1.0 requests that do not specify a Host header" in the message.

Third, I am a bit wondering why not to call mapper.setDefaultHost()
unconditionally. What is wrong with passing a name there?

Hosts can be added and removed through JMX calls on
StandardEngine#addChild()/removeChild() and it seems that MapperListener fails
to update defaultHost setting on the Mapper when it happens.

So why not to pass the defaultHost name to the Mapper as is and let it handle
missing matches (like it already does)?


2) in Mapper#map(MB,MB,S,MD)

Fourth,
Maybe just return without mapping here, as if the Host is not found. We already
do if(defaultHostName==null){ return; } in its #internalMap(CC,CC,S,MD) method.

It will need some update to its caller though, which is
CoyoteAdapter#postParseRequest()


3) In CoyoteAdapter#postParseRequest()

This request could be rejected with error 404, instead of 400 that exception
handling in AbstractHttp11Processor#process() does. The postParseRequest()
already has code for handling it as 404, but

Fifth,
access logging needs to be changed a bit. The current code:

[[[
                // Make sure there is a host (might not be during shutdown)
                if (host != null) {
                    host.logAccess(request, response, 0, true);
                }
]]]

I think that it should fallback to CoyoteAdapter#log(..) when host is null.

-- 
You are receiving this mail because:
You are the assignee for the bug.

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

Reply via email to