Bug 42701 - Http11NioProtocol returns wrong data on non-Comet HTTP requests
Summary: Http11NioProtocol returns wrong data on non-Comet HTTP requests
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: unspecified
Hardware: All other
: P2 major (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-20 05:24 UTC by Andrey Subbotin
Modified: 2007-06-20 11:04 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Subbotin 2007-06-20 05:24:19 UTC
When loading pages via Http11NioProtocol about 50% of images and other resources
are randomly replaced with other images/resources from the same page or fail.
The problem usually is noticeable only when communicating with non-localhost
servers.

Upon investigating I found out that NIO protocol reads from request input stream
until it drains the available data, then puts the partially parsed request and
the partially read socket into event queues and waits until more data comes.

However then it mismatches the request/socket pair and the resulting request is
a mix of headers from several different requests. In fact it doesn't even try to
 keep the mapping - it just grabs any request and any socket and starts parsing.

This bug is not noticeable on localhost, since there the entire request is
typically read at once.

This bug does not happen for Comet requests that keep their socket-request
mapping in Http11NioProtocol.Http11ConnectionHandler.connections map.

Changing Http11NioProtocol.Http11ConnectionHandler.process() to the following
seems to fix the problem (I am keeping the socket-request mapping for regular
requests in the same map where Comet requests keep theirs)
===============================================================
        public SocketState process(MyNioChannel socket) {
            MyHttp11NioProcessor processor = connections.get(socket);
            try {
                if (processor == null) {
                    processor = recycledProcessors.poll();
                }
                if (processor == null) {
                    processor = createProcessor();
                }

                if (processor instanceof ActionHook) {
                    ((ActionHook) processor).action(ActionCode.ACTION_START, null);
                }


                processor.setSslSupport(null);


                SocketState state = processor.process(socket);
                if (state != SocketState.CLOSED) {
                    // Associate the connection with the processor. The next request
                    // processed by this thread will use either a new or a recycled
                    // processor.
                    if (log.isDebugEnabled()) log.debug("Not recycling
["+processor+"]
Comet="+((NioEndpoint.KeyAttachment)socket.getAttachment(false)).getComet());
                    connections.put(socket, processor);
                    socket.getPoller().add(socket);
                } else {
                    connections.remove(socket);
                    recycledProcessors.offer(processor);
                }
                return state;

            } catch (java.net.SocketException e) {
                // SocketExceptions are normal
               
log.debug(sm.getString("http11protocol.proto.socketexception.debug"), e);
            } catch (java.io.IOException e) {
                // IOExceptions are normal
               
log.debug(sm.getString("http11protocol.proto.ioexception.debug"), e);
            }
            // Future developers: if you discover any other
            // rare-but-nonfatal exceptions, catch them here, and log as
            // above.
            catch (Throwable e) {
                // any other exception or error is odd. Here we log it
                // with "ERROR" level, so it will show up even on
                // less-than-verbose logs.
                log.error(sm.getString("http11protocol.proto.error"), e);
            }
            connections.remove(socket);
            recycledProcessors.offer(processor);
            return SocketState.CLOSED;
        }
Comment 1 Filip Hanik 2007-06-20 06:07:32 UTC
I believe a different fix has been applied to trunk of 6.0.x and 6.x, are you
able to test that version before we consider this fix.
Comment 2 Filip Hanik 2007-06-20 06:11:30 UTC
here is the 6.0 fix
http://svn.apache.org/viewvc?view=rev&revision=542645

and here is the 6.x fix
http://svn.apache.org/viewvc?view=rev&rev=542666

Comment 3 Andrey Subbotin 2007-06-20 06:57:04 UTC
Yes, I checked and the bug was fixed in the trunk. My apologies for not checking
the latest version.
Comment 4 Filip Hanik 2007-06-20 07:24:28 UTC
no worries, thanks for reporting it.
The two fixes are little different:
6.0.x is blocking read of the request
6.x is non blocking read of request line and headers

would you be able to test both?

Filip