Filip Hanik - Dev Lists wrote: > we're copying bytes to two new byte arrays on every call, just so that > we have info when it fails?
For query string there is (and always has been) one copy and then this patch copies again for the failure info. For POST requests the only copy is for the failure info. > especially only logged with debugEnabled, shouldn't your logic check > that flag then The debug logging includes the exception. There is still warn level logging that provides the information on the failed parameters. One option to reduce the copying would be to log the failure with the corrupted data at warn level along with the info that if you use debug logging you'll see the original data. That would move the copying to debug only although you might see some odd messages depending on how the decoding failed. What do you think? Mark > Filip > > > ma...@apache.org wrote: >> Author: markt >> Date: Fri Jun 19 21:14:32 2009 >> New Revision: 786667 >> >> URL: http://svn.apache.org/viewvc?rev=786667&view=rev >> Log: >> Can't use queryMB as that isn't the only source. >> >> Modified: >> tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java >> >> Modified: tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java?rev=786667&r1=786666&r2=786667&view=diff >> >> ============================================================================== >> >> --- tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java >> (original) >> +++ tomcat/trunk/java/org/apache/tomcat/util/http/Parameters.java Fri >> Jun 19 21:14:32 2009 >> @@ -338,6 +338,8 @@ >> // if needed >> ByteChunk tmpName=new ByteChunk(); >> ByteChunk tmpValue=new ByteChunk(); >> + ByteChunk origName=new ByteChunk(); >> + ByteChunk origValue=new ByteChunk(); >> CharChunk tmpNameC=new CharChunk(1024); >> CharChunk tmpValueC=new CharChunk(1024); >> @@ -396,18 +398,25 @@ >> } >> tmpName.setBytes( bytes, nameStart, nameEnd-nameStart ); >> tmpValue.setBytes( bytes, valStart, valEnd-valStart ); >> - >> + try { >> + // Take copies as if anything goes wrong originals >> will be >> + // corrupted. This means original values can be logged >> + origName.append(bytes, nameStart, nameEnd-nameStart); >> + origValue.append(bytes, valStart, valEnd-valStart); >> + } catch (IOException ioe) { >> + // Should never happen... >> + log.error("Error copying parameters", ioe); >> + } >> + try { >> addParam( urlDecode(tmpName, enc), >> urlDecode(tmpValue, enc) ); >> } catch (IOException e) { >> - // tmpName or tmpValue will be corrupted at this >> point due to >> - // failed decoding. Have to go to queryMB to get >> original data >> StringBuilder msg = >> new StringBuilder("Parameters: Character decoding >> failed."); >> msg.append(" Parameter '"); >> - msg.append(queryMB.toString().substring(nameStart, >> nameEnd)); >> + msg.append(origName.toString()); >> msg.append("' with value '"); >> - msg.append(queryMB.toString().substring(valStart, >> valEnd)); >> + msg.append(origValue.toString()); >> msg.append("' has been ignored."); >> if (log.isDebugEnabled()) { >> log.debug(msg, e); >> @@ -418,7 +427,8 @@ >> >> tmpName.recycle(); >> tmpValue.recycle(); >> - >> + origName.recycle(); >> + origValue.recycle(); >> } while( pos<end ); >> } >> >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> >> >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
