Project: Tomcat
Bug 54010 Suggestion for code improvement (avoiding potential bug)
Reported: 2012-10-16 02:20:47 | Status: resolved fixed

In connectors/jk/java/org/apache/jk/common/HandlerRequest.java coyote.Request's schemeMB is assigned in 2 places. 1st place: 400 boolean isSSL = msg.getByte() != 0; 401 if( isSSL ) { 402 // XXX req.setSecure( true ); 403 req.scheme().setString("https"); 404 } 2nd place: 518 case AjpConstants.SC_A_SSL_CERT : 519 req.scheme().setString( "https" ); and similar assignments for SC_A_SSL_CIPHER and SC_A_SSL_SESSION cases below. It seems they do not make sense because the packet's 8-bit field is designated for telling whether it's SSL or not. So the 1st place is enough. Adding the 2nd place may pose potential bug in that a packet with the 8-bit SSL field being 0 and suffixes of SC_A_SSL_* key-value pairs can later incorrect trigger a wrong redirection message pointing to a https location. A simple correction is to honor the 8-bit SSL-field in packet and delete the 3 lines of 2nd place assigning "https". Even though the chances of such spurious packet is low, but it's best we can have threat-free, semantic-correct tomcat code. The same lines of code remain in 6.0 and 7.0. But maybe I misunderstand the code, in which case please kindly point out. Thanks.

Files changed in fix:
java/org/apache/coyote/ajp/AbstractAjpProcessor.java