https://bz.apache.org/bugzilla/show_bug.cgi?id=66627
Bug ID: 66627
Summary: Regression due to MessageBytes refactoring
Product: Tomcat 9
Version: 9.0.71
Hardware: All
OS: All
Status: NEW
Severity: regression
Priority: P2
Component: Util
Assignee: dev@tomcat.apache.org
Reporter: bill-apa...@carpenter.org
Target Milestone: -----
This commit,
https://github.com/apache/tomcat/commit/10a1a6d46d952bab4dfde44c3c0de12b0330da79,
that appeared in 9.0.71 made changes to MessageBytes.toString() that cause a
serious regression under some circumstances. This is preventing us from
upgrading to later Tomcat releases to pick up security fixes.
If the MessageByte object is of type T_BYTES or T_CHARS, it gets converted to
type T_STR. Although there is nothing in the general contract of toString()
that prohibits modifying the object, it's an unexpected side-effect. The
JavaDoc for MessageBytes.getType() explicitly says it returns "the type of the
original content", so the change breaks that contract.
But it's more serious than a mere disagreement with documentation. Some
colleagues of mine were a few days ahead of me in investigating this problem
(they were working with tomcat-embed-core and I was working woth Tomcat
standalone). Most of this explanation is due to their research.
If something calls MessageBytes.toString(), fragile assumptions elsewhere can
fall apart. For example, if you use a Java agent like OpenTelemetry, it
intercepts every request in order to obtain the URL path for logging.
CoyoteAdapter.postParseRequest() then makes a mistake because the MessageBytes
object changed from type T_BYTES to type T_STR, and this assumption is no
longer true:
/*
* The URI is chars or String, and has been sent using an in-memory protocol
handler. The following
* assumptions are made: - req.requestURI() has been set to the 'original'
non-decoded, non-normalized
* URI - req.decodedURI() has been set to the decoded, normalized form of
req.requestURI()
*/
The downstream result of that is a 404 response for every URL path.
Experimentally, I found that simply removing the type reassignment, as seen
here, was enough to resolve the immediate issue. State tracking in the
MessageBytes class is a bit complicated, and I have not looked carefully to see
if this proposed fix has any undesired consequences.
/**
* Compute the string value.
* @return the string
*/
@Override
public String toString() {
switch (type) {
case T_NULL:
case T_STR:
// No conversion required
break;
case T_BYTES:
//type = T_STR;
strValue = byteC.toString();
break;
case T_CHARS:
//type = T_STR;
strValue = charC.toString();
break;
}
return strValue;
}
--
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
